Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Geometry labels #8

Merged
merged 5 commits into from Dec 16, 2021
Merged

Geometry labels #8

merged 5 commits into from Dec 16, 2021

Conversation

Sybrand
Copy link
Member

@Sybrand Sybrand commented Dec 14, 2021

Create a separate table for each layer, that contains only the centroid of the source layer. This allows for a separate layer that only contains labels, not entire geometries and thus avoiding duplication of labels that may occur when serving up geometries that span multiple tiles.

Copy link
Collaborator

@andrea-williams andrea-williams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No obvious concerns from me, but I'm out of the loop on what's going on with the tileserver...

@@ -30,6 +33,28 @@ def get_column_type(value):
raise Exception(f'unknown type {type(value)}')


def create_point_table_schema(meta_data: MetaData, data: dict, table_name: str, srid: int) -> Table:
"""
"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes. The cheater's way of adding code "documentation" so linting won't complain.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That method was dumb anyway. I've removed it.

Copy link
Collaborator

@micheal-w-wells micheal-w-wells left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also just catching on this, but interested to see how it works. I've only tried to sync data from DataBC and not ESRI, and also not in python.

More a general question/comment than feedback relevant to the PR, but here I am anyway: Is there a search feature being passed to ESRI or are we grabbing the whole object list for the whole layer in fetch_object_list ? If it works it works it's not like a human is doing all those http calls to grab each object, but I'm curious if it's possible to search by a feature or if it's planned.

I can imagine the object list for some has to be insane (insane like the features with so many duplicate vertices that help create these issues to begin with).

@Sybrand
Copy link
Member Author

Sybrand commented Dec 15, 2021

I'm also just catching on this, but interested to see how it works. I've only tried to sync data from DataBC and not ESRI, and also not in python.

More a general question/comment than feedback relevant to the PR, but here I am anyway: Is there a search feature being passed to ESRI or are we grabbing the whole object list for the whole layer in fetch_object_list ? If it works it works it's not like a human is doing all those http calls to grab each object, but I'm curious if it's possible to search by a feature or if it's planned.

Right now it's grabbing the entire layer. For our use case at least, that's the desired functionality.
I don't have any intention to implement any kind of search right now - but if it's something someone needs there's no reason it can't be added. Right now I'm only focusing on our teams needs, but I'd love to make this repo be a general solution that multiple teams can use.

I can imagine the object list for some has to be insane (insane like the features with so many duplicate vertices that help create these issues to begin with).

The "duplication" issue I mentioned relates to how vector tiles are served up and rendered. A geometry may span multiple tiles, so it ends up with a label for each tile. For the layers I'm pulling right now, the data is nice and clean.

@Sybrand Sybrand merged commit a2aa3f3 into main Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants