-
Notifications
You must be signed in to change notification settings - Fork 11
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
Sort out generated features for landuse, buildings & POIs #72
Sort out generated features for landuse, buildings & POIs #72
Conversation
…g outlines. Updated building logic to remove 'POI-like' kinds - these will go in the POIs layer instead.
…rt them to addresses if they are numeric.
…e that property set.
… that we can generate landuse labels without having landuse polygons.
feature_layers, zoom, source_layer=None, start_zoom=0): | ||
""" | ||
Generates address points from building polygons where there is an | ||
addr:housenumber tag on the building. Removes those tags from the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this introduce potential backwards incompatibility that we should note in the CHANGELOG?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there are lots of changes from these PRs... is the appropriate thing to include updates to the changelog in the PR, or just flag them for the changelog to be updated when pushing & tagging a new version?
…eful information (the area probably isn't zero, just unspecified).
if len(parts) == 0: | ||
# the only way we can signal an empty geometry, as | ||
# MultiPoint([]) throws an exception. | ||
return GeometryCollection() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MultiPoint()
seems to work though, and the others with no args seem to as well. We'd want to use the appropriate geometry constructor based on keep_dim
right? I'm not sure if that's worth it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch - I didn't even try just MultiPoint()
. I've extracted the constructors in 15ed909 - hopefully that looks better?
…erty getting/dropping logic to be clearer. Dropped non-idiomatic use of pass in favour of continue.
👍 |
Sort out generated features for landuse, buildings & POIs
This PR contains:
Connects to tilezen/vector-datasource#201.
@rmarianski could you review, please?