-
Notifications
You must be signed in to change notification settings - Fork 576
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 natural=valley #1589
base: master
Are you sure you want to change the base?
Add natural=valley #1589
Conversation
At the time, `natural=valley` ways were excluded from openmaptiles#1202 Following the discussion there, it looks like it would be fine to add them.
Results evaluating commit 5bf2ca1 (merged with base e61442c as fbe99ef). See run details. PostgreSQL DB size in MB: 4939 ⇒ 4939 (0.0% change)
expand for details...
|
Thanks! Maybe one improvement for
|
LGTM |
Adding the Could you please update the PR? Thanks! |
On second thought, I prefer not to expand this PR. The intention here is for a minimal addition to the layer where Each of the two recommendations require a non-trivial enhancement of the layer:
I prefer that these enhancements will be put in a separate issue/s that would be handled by a different person. |
Cliffs, ridges, and arêtes are all commonly rendered on maps as a line feature without labelling (though I would love to have labelled ridges in OpenMapTiles). Valleys on the other hand are rarely rendered on maps as a line feature only - they are usually rendered only as a label feature. So the primary cartographic reason for the linestring for valleys are as placement for a label. So I'm not sure what benefit we really gain from adding valleys if we don't also include sufficient attributes ( |
#274 requested the addition of several
natural
features.#1202 addressed most of them.
However,
natural=valley
ways were excluded from the PR.Following the discussion about this exclusion, it looks like it would be fine to add them and resolve #274