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

Add first AreaMetadata with config option and tests. #42

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lehmann-4178656ch
Copy link
Member

This PR adds a switch to add the number of outer polygons for areas.

This information was partially requested by @jogoe222 and allows SPARQL queries selecting complex areas without parsing the WKT representation.

@patrickbr
Copy link
Member

patrickbr commented Jan 11, 2023

Thanks, Axel! I am a bit confused by the relation name multipolygon_outer_polygon_count though - why is "outer polygon" mentioned there explicitly? This seems redundant, as a "normal" polygon can only have a single outer polygon. I think something simpler like outer_polygon_count or even polygon_count or num_polygons would be better here :)

Also, is this really limited to multipolygons like the name suggests? Shouldn't it also be written for "normal" polygons to ensure that a user can filter simple polygons without parsing the WKT?

@lehmann-4178656ch
Copy link
Member Author

I'm not good at naming things, as we only have a global meta prefix I thought that mentioning multipolygons could help the understanding of what this value is about without looking at the subject. But this can and should be improved, currently I like outer_polygon_count as we could add inner_polygon_count in the future. I'll think about this and ask which additional values could be useful -> I'll mark this PR as draft for the meantime.

@lehmann-4178656ch lehmann-4178656ch marked this pull request as draft January 11, 2023 17:06
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.

2 participants