-
-
Notifications
You must be signed in to change notification settings - Fork 82
Migrate POI minZoom assigment from plain Java to MultiExpression rules #539
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
Migrate POI minZoom assigment from plain Java to MultiExpression rules #539
Conversation
…ined failure with aerodrome/iata tags
…tags for zoom checks
|
Good work, thanks for opening this pull request @migurski. Some thoughts from my side: |
In this case, we’ll generally always need them for the unbounded right-hand side of a range. I’ll update this PR with an
I thought this was the case and even had it passing unit tests, but ran into a bunch of errors when I actually tried doing it in a real processing run: The unsupported operation is why I switched to using a |
|
|
|
Above is how an additional _country tag is added to a source feature in Places.java |
I saw this pattern elsewhere and used it, so I’m unsure why my own usage resulted in the |
| * Creates an {@link Expression} that matches when a numeric tag value is within a specified range. | ||
| * | ||
| * <p> | ||
| * The lower bound is inclusive. The upper bound, if provided, is exclusive. |
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.
| * The lower bound is inclusive. The upper bound, if provided, is exclusive. | |
| * The lower bound is inclusive. The upper bound is exclusive. |
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.
Added this in #541
wipfli
left a comment
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.
Looks good to me. Maybe we should use the same class for the extra tags also in the Places.java file



Follow up to #537.
Migrate from plain Java conditionals to MultiExpression rules when applying POI
minZoom. Removes most dense logic fromprocessOsm(), passes all existing tests with no changes required.To recreate
wayAreaandheightspan logic, introduce range expressions for rules:withinRange(attr, lower, upper)– true when named attribute is between lower (inclusive) and upper (exclusive) boundsatLeast(attr, lower)– true when named attribute is greater than or equal to lower (inclusive)To separate into multiple rules passes to for readability (kinds followed by zooms for either points or polyons), introduce
SourceFeatureWithComputedTagswrapper for source features with a mutable tags map.