-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
Expose min_size
in configurable schema
#421
Conversation
https://github.com/onthegomap/planetiler/actions/runs/5585967125 ℹ️ Base Logs 4c1503e
ℹ️ This Branch Logs 391f01d
|
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.
Thanks for getting this started! I think there's a bit more we'd like to do with min_size
later (let it vary by zoom, and specify in different units, see #349 (comment)) but I think just letting it be specified in pixels for all zoom levels is a good starting point and we can add those other things in a followup review.
@@ -55,6 +55,7 @@ public record OutputFeature( | |||
GeometryType geometry, | |||
@JsonProperty("min_zoom") Integer minZoom, | |||
@JsonProperty("max_zoom") Integer maxZoom, | |||
@JsonProperty("min_pixel_size") Double minPixelSize, |
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.
can you also add a test to SchemaValidatorTest
to make sure this is being compared in the tests?
@@ -84,6 +84,7 @@ public ConfiguredFeature(String layer, TagValueProducer tagValueProducer, Featur | |||
} | |||
processors.add(makeFeatureProcessor(feature.minZoom(), Integer.class, Feature::setMinZoom)); | |||
processors.add(makeFeatureProcessor(feature.maxZoom(), Integer.class, Feature::setMaxZoom)); | |||
processors.add(makeFeatureProcessor(feature.minPixelSize(), Double.class, Feature::setMinPixelSize)); |
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.
I think we want some kind of test for this in ConfiguredFeatureTest
to make sure it's wired-up properly.
min_pixel_size
in configurable schemamin_size
in configurable schema
Kudos, SonarCloud Quality Gate passed! |
@wipfli want to dust this PR off and get it merged? We'll need it for linestring merging to work in #626. Or did you want me to take it over? From my perspective the only things left to do before merging are:
|
Hi @msbarry!
If you can take it over it would be ideal. Thank you so much! |
Kudos, SonarCloud Quality Gate passed! |
Exposes the
Feature::setMinPixelSize
function in the configurable schema asmin_size
.This follows the implementation of
min_zoom
.Related to #349