-
Notifications
You must be signed in to change notification settings - Fork 199
Add preset for climbing=route_bottom
#1590
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
Conversation
🍱 Your pull request preview is ready Please use this preview to check your changes. Ideally use the test documentation template and document your test results by commenting on the PR. This will speed up the review process for everyone. FYI, once this PR is merged, you can use the iD Editor Preview to test your changes in interaction with all other changes. |
First Test-Documentation before the code reviewTest-DocumentationPreview links & Sidebar Screenshots
SearchInfo-
|
Haha, great minds think alike. I just published a draft PR doing some of the same work you have done here: #1591 I suggest we try to merge yours first, but I'd really like to get the |
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 breaking down the big PR. Its much easier to review this way.
I left some comments on improvements.
In general, it would be great if you could take this opportunity and check if the Wiki pages are beginner friendly already. I thing they need to be improved a bit.
When I looked at https://wiki.openstreetmap.org/wiki/Tag:sport%3Dclimbing I did not find any mention of the fields here. The fact that there is https://wiki.openstreetmap.org/wiki/Climbing is only visible if users actually read line by line. And only on https://wiki.openstreetmap.org/wiki/Climbing are there more details on what we are doing here.
Adding some of the fields here as redirects will help. But also adding some more links and callouts to the pages will help.
The German page is also bigger than the english page which I always find weird for something that is international https://wiki.openstreetmap.org/wiki/DE:Tag:sport%3Dclimbing (and not scoped to one country in the presets here).
data/fields/climbing/sport.json
Outdated
{ | ||
"key": "climbing:sport", | ||
"type": "check", | ||
"label": "Sport climbing" |
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.
Do users really understand that this is "with bolts" where the other is "bring your own"? Can we make this part of the label without making it wrong in some cases…?
Eg. https://de.wikipedia.org/wiki/Sportklettern says "bouldern" is also sport climbing?
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.
Users will understand. Bouldering occasionally gets lumped under sport climbing for political reasons related to "sport climbing" being an Olympic sport. There may be some linguistic differences too, but most Germans I know also make a distinction between the two.
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 explored this topic little bit more, and usually the hypernym Rock climbing, consists of Sport climbing (with bolts+rope), Bouldering (shorter, without bolts), etc. The wikipedia states it clearly.
It seems the Olympics really added some confusion here.
To make it clearer, I added a bracket: Sport climbing (bolted climbing)
– specifically because we don't have the bouldering option (yet), so begginers could try to fit it wrongly. What do you think?
"fields": [ | ||
"name" | ||
], | ||
"moreFields": [ | ||
"climbing/grade/uiaa", | ||
"climbing/grade/french", | ||
"climbing/grade/saxon", | ||
"climbing/grade/yds_class", | ||
"website", | ||
"climbing/length", | ||
"climbing/sport", | ||
"climbing/trad", | ||
"climbing/bolts", | ||
"description" | ||
], |
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.
"fields": [ | |
"name" | |
], | |
"moreFields": [ | |
"climbing/grade/uiaa", | |
"climbing/grade/french", | |
"climbing/grade/saxon", | |
"climbing/grade/yds_class", | |
"website", | |
"climbing/length", | |
"climbing/sport", | |
"climbing/trad", | |
"climbing/bolts", | |
"description" | |
], | |
"fields": [ | |
"name", | |
"climbing/sport", | |
"climbing/trad", | |
"climbing/bolts", | |
], | |
"moreFields": [ | |
"climbing/grade/uiaa", | |
"climbing/grade/french", | |
"climbing/grade/saxon", | |
"climbing/grade/yds_class", | |
"website", | |
"climbing/length", | |
"description" | |
], |
Does every route have a name? Having the name field that prominent will result in users adding it as a free form field. What is the recommended name for a route?
Is there some way to promote a source in this schema? Like "From the signs at the wall" or "From the book XY"?
I think some fields should be fields
. Aren't sport and trad fields we always want to be filled? (With bolts being conditional, see above)
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.
About the name
: Looking at a few examples https://overpass-turbo.eu/s/25II the data looks good ATM. That might change, however, once we promote it to less experienced users.
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.
Most routes have a name. Rarely there is a sign. More often books, online guides, word of mouth.
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.
Yes, usually routes have names - I have never heard about a route without name. But when is someone mapping in the field – they could probably see there is a bolted route, but the name is not usually written there. It is maintained by the local climbing organization, usually appears in printed guides, and sometimes there is a signboard nearby with the names, difficulties and a map (topo).
I added the suggested fields to fields
property.
New route looks like this, which is probably fine - the info buttons shows the additional clarification:
@@ -0,0 +1,6 @@ | |||
{ | |||
"key": "climbing:grade:french", | |||
"type": "text", |
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 wonder if we should try to improve the data validation aspects for those grade fields.
type:text
is the most open field which provides no validation.
However, the values are well defined, eg. https://taginfo.openstreetmap.org/keys/climbing%3Agrade%3Afrench#values
What are our options?
- We could make it an
combo
field withstrings
and either list all or rely on taginfo to fill the missing (seeautosuggest
) - If there where a good website that we can link to, we could make it an
identifier
field and use thepattern
to validate the input via Regex. The website should be something like a generic page that handles the input as search param, maybe something that shows the value in different grades?
This applies to all 3 grade fields.
I think we should at least do the combo
so users are guided to well structures data.
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.
Grades are very diverse. French grades might benefit from a combo, but other grades get more complicated.
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 took the values from the grading table on wiki, and made them into a combo
field. I took only the basic ones (without slash), i think it is a good start for the users to select, and of course it allows users to enter their own value, if they prefer.
You have to add a description in the data item table, eg https://wiki.openstreetmap.org/wiki/Item:Q8221 |
@tordans - thanks for a comprehensive review. I agree that this is a good oportunity to update the wiki. I will work on it this week. Once it is visible on wiki, i will respond to the comments with links. ad link to Q230238) i dont understand your comment - this item has the label specified, but iD still ignores the dataitem altogether. It seems to only fetch a wiki page, not the item. Maybe it gets fixed after i create the route_bottom page? or do i have to link it to the item? ![]() |
I don't know. I asked about it in https://osmus.slack.com/archives/CBK3JLUJU/p1749630713467699 just now. |
I made a few changes to the item that should help, though I haven’t tested them in iD yet:
You can find more information on the expected format in this help article. It’s kind of dense and easy to miss important steps there, but hopefully you can use this item as an example in the future. This case is also a little more awkward than usual because there isn’t a dedicated article about this tag yet, only a redirect to a different article. I’d encourage you write a full-fledged article about the tag at some point for discoverability. |
I changed this on the wiki + created correct wikitiems to all of these pages:
|
@tordans - I made new screenshots here: Test-documentation screenshots after codereview changes no.1Test-DocumentationPreview links & Sidebar Screenshots
SearchInfo-
|
1 | 2 |
---|---|
![]() |
![]() |
Wording
- American English - should be the same
-
name
,aliases
(if present) use Title Case -
terms
(if present) use lower case, sorted A-Z
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 this is ready to be merged now.
I changed the combo fields a bit with my last two commits. The Grade (Saxon) requires the caseSensitive=true
flag. And with explicit autosuggest=off
the options are sorted just as they are specified in the file and also no network request is done.
To me, this looks good. Shall we merge?
Perfect! I also noticed the slightly unordered lists by the taginfo and didn't know it is possible to turn it off 👍 Lets roll it 🙂 |
Description, Motivation & Context
Hi, here is my another take on adding climbing features to iD. This time I focused only on what is already prevalent in OSM. Last year I proposed a PR (#1113) which stalled - perhaps it was too large, or perhaps I add too much of unused stuff from OSM wiki.
In this first PR, I would like to add only
climbing=route_bottom
node preset + its fields.Preset: Climbing Route Start (
climbing=route_bottom
)way
withclimbing=route
, but it is not as usual in OSM DB yet.Linked fields from the preset
I wrote a script which measured all the tag combinations - see results here. I propose to add only fields which are used more often and are already established on the OSM wiki.
name=*
climbing:grade:uiaa=*
climbing:grade:french=*
climbing:grade:saxon=*
climbing:grade:yds_class=*
website=*
climbing:length=#
climbing:sport=yes
climbing:trad=yes
climbing:bolts=#
description=*
Related issues
Links and data
Relevant OSM Wiki links:
Relevant tag usage stats:
Checklist and Test-Documentation Template
Read on to get your PR merged faster…
Follow these steps to test your PR yourself and make it a lot easier and faster for maintainers to check and approve it.
This is how it works:
After you submit your PR, the system will create a preview and comment on your PR:
Once the preview is ready, use it to test your changes.
Now copy the snippet below into a new comment and fill out the blanks.
Now your PR is ready to be reviewed.