Closes #204 and adds POIs for landuse=village_green, laduse=allotment… #212
Conversation
| @Test | ||
| void playground() { | ||
| assertFeatures(15, | ||
| List.of(Map.of("pmap:kind", "playground"), Map.of("pmap:kind", "playground", "pmap:min_zoom", 13, "name", "Spielwiese")), |
There was a problem hiding this comment.
I am a bit unsure about why I see two elements here - maybe just have a closer look whetehr the tests are what you would expect :-)
There was a problem hiding this comment.
@bdon can you comment on this question about test construction, please?
There was a problem hiding this comment.
The tests run against the entire Profile, not just a single layer class, so you're also getting the landuse element in the result set.
There was a problem hiding this comment.
Okay, understood, but that means I'm basically not unit-testing the part I'm interested in. I don't necessarily want to refactor it so it is independent - I think it's rather up to you how you want this part to evolve in the future. I'll add a comment about what is happening.
|
Will fix the issues monday or tuesday, won't find time beforehand |
| @@ -355,7 +356,9 @@ public void processFeature(SourceFeature sf, FeatureCollector features) { | |||
| } | |||
| } | |||
| } else if (kind.equals("cemetery") || | |||
| kind.equals("school")) { | |||
| kind.equals("school") || | |||
| kind.equals("allotments") || | |||
There was a problem hiding this comment.
Allotment POIs should appear later than schools, more like zoom 15 or 16
(see also how Tilezen sets the value: https://github.com/tilezen/vector-datasource/blob/master/yaml/pois.yaml#L1258-L1266)
There was a problem hiding this comment.
Then I'll add it such that allotments larger than 250m² get shown from zoom level 15, and smaller ones from 16 (basically last part of this branch).
| kind.equals("school")) { | ||
| kind.equals("school") || | ||
| kind.equals("allotments") || | ||
| kind.equals("playground")) { |
There was a problem hiding this comment.
Playground POIs should only appear at later zooms, like zoom 16.
(see also how Tilezen sets the value: https://github.com/tilezen/vector-datasource/blob/master/yaml/pois.yaml#L1315-L1324)
There was a problem hiding this comment.
Okay, I'll add a separate branch showing them from zoom level 16 onwards.
| @@ -323,7 +323,8 @@ public void processFeature(SourceFeature sf, FeatureCollector features) { | |||
| } else if (kind.equals("forest") || | |||
| kind.equals("park") || | |||
| kind.equals("protected_area") || | |||
| kind.equals("nature_reserve")) { | |||
| kind.equals("nature_reserve") || | |||
| kind.equals("village_green")) { | |||
There was a problem hiding this comment.
This could work out... but might need some adjustments. Generally village greens are pretty small, and the other features in this grouping can get much larger.
Most similar Tilezen section is here https://github.com/tilezen/vector-datasource/blob/master/yaml/pois.yaml#L143-L152, but there's some related clamping for commons.
There was a problem hiding this comment.
What adjustments are you thinking about?
…allotments, leisure=playground
|
|
@lenalebt thanks for adding these - can you suggest a representative area of the world so I can try out the PR on it? The nature of tileset changes is that they will affect the visual output. One of the things I'm working on is a test suite that lets us regression compare visually any two states in the past. Example of a test for bridges: https://maps.protomaps.com/visualtests/?tag=bridge Ideally a visual test example for this would have one or more playgrounds and village greens / allotments |
|
(Nevermind, I see your link for https://www.openstreetmap.org/#map=18/51.50497/7.31888 , perfect example!) |
|
Merging for now, will follow up with style changes later, thanks! |
|
(visual diff) |
…s, leisure=playground