Skip to content
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 fell kind to landuse #1932

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Add fell kind to landuse #1932

wants to merge 8 commits into from

Conversation

rwrx
Copy link
Contributor

@rwrx rwrx commented Sep 30, 2019

Add fell kind to landuse

@@ -373,6 +373,7 @@ post_process:
scree: barren
shingle: barren
stone: barren
fell: barren
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, thanks for taking care of the remapping!

Copy link
Member

@nvkelso nvkelso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to set a sort_rank for the new fell kind, too.

Otherwise LGTM

@nvkelso
Copy link
Member

nvkelso commented Sep 30, 2019

Are there also names for these, if so we need to add collision_rank.

@rwrx
Copy link
Contributor Author

rwrx commented Oct 1, 2019

I am not sure which number for fell should I set. I have looked at bare_rock, desert, grassland, heath, sand, shingle and they does not have set sort_rank. I haven't found a fell with a name. Is there some online tool to do osm query?

@nvkelso
Copy link
Member

nvkelso commented Oct 7, 2019

Looks like a few fell features do have names:

sort_order:

collision_order:

Copy link
Member

@nvkelso nvkelso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a test, here's an example feature:

I found it using Overpass-Turbo for (in northern Europe in this case):

Which should be in this tile:

  • 16, 35072, 18481

With tags:

  • area: yes
  • name: Fulufjellet
  • natural: fell
  • wikidata: Q34852081

@rwrx
Copy link
Contributor Author

rwrx commented Oct 9, 2019

I have updated sort and collision ranks, but now Circle is complaining that there is no 265 sort rank, because I have incremented it to 266:

dam,LineString;MultiLineString,266

I am not sure how to change this Circle test.

@rwrx
Copy link
Contributor Author

rwrx commented Oct 9, 2019

I have added test, but I am not sure into which file I should have put it in and I am not sure whether it is correctly written.

@nvkelso nvkelso added this to the v1.9.0 milestone Nov 11, 2019
@rwrx rwrx mentioned this pull request Mar 21, 2022
dam,*,224
land,*,223
wetland,*,221
mud,*,220
Copy link
Member

@nvkelso nvkelso Mar 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mud, wetland, land and above don't need to increment their numbers, as there is a discontinuity buffer between mud and footway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, should I remove dam, land, wetland and mud? Will they sort properly when they will be removed from this list?

@nvkelso nvkelso modified the milestones: v1.9.0, v2.0.0 Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants