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 presets for cycleway=traffic_island #1068

Merged
merged 1 commit into from
Dec 25, 2024
Merged

Conversation

k-yle
Copy link
Collaborator

@k-yle k-yle commented Dec 7, 2023

Closes #950

No one commented on that issue so I assume this is non-controversial...

Here's the updated table, the 3 entries marked as 🆕 are the ones added in this PR:

Foot Cycle Cycle & Foot (International) Cycle & Foot (EU)
Roadside Path ✅ Sidewalk ✅ Cycle Path ✅ Cycle & Foot Path ✅ Cycle & Foot Path
Generic Crossing ✅ Pedestrian Crossing ✅ Cycle Crossing ✅ Cycle & Foot Crossing ✅ Cycle & Foot Crossing
Specific Crossings ✅ Unmarked Crossing
✅ Marked Crossing
✅ Uncontrolled Crossing
✅ Zebra Crossing
✅ Crossing With Traffic Signals
✅ Unmarked Cycle Crossing
✅ Marked Cycle Crossing
✅ Uncontrolled Cycle Crossing
Zebra Crossing
✅ Cycle Crossing With Traffic Signals
Unmarked Crossing
Marked Crossing
Uncontrolled Crossing
Toucan Crossing
Crossing With Traffic Signals
✅ Unmarked Cycle & Foot Crossing
✅ Marked Crossing
✅ Marked Cycle & Foot Crossing
Toucan Crossing
✅ Cycle & Foot Crossing With Pedestrian Signals
Traffic Island ✅ Refuge Island 🆕 Cycle Refuge Island 🆕 Cycle & Foot Refuge Island 🆕 Cycle & Foot Refuge Island

Copy link

github-actions bot commented Dec 7, 2023

🍱 Preview the tagging presets of this pull request here: https://pr-1068--ideditor-presets-preview.netlify.app/id/dist/#locale=en.

@tyrasd tyrasd added this to the v6.6 milestone Dec 15, 2023
@tyrasd tyrasd modified the milestones: v6.6, v6.7 Jan 24, 2024
@tyrasd tyrasd modified the milestones: v6.7, v6.8 Mar 13, 2024
Copy link
Collaborator

@tordans tordans left a comment

Choose a reason for hiding this comment

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

I think we should also add a (unsearchable) preset for path=traffic_island https://wiki.openstreetmap.org/wiki/Tag:path=traffic_island which has low usage ATM but it is a pain to hot have all those crossing presents on all common highway classes and for some tagging the hw=path+bicycle=designated+foot=designated is very common.

data/presets/highway/cycleway/traffic_island_shared.json Outdated Show resolved Hide resolved
@tordans tordans mentioned this pull request Apr 26, 2024
18 tasks
@tordans
Copy link
Collaborator

tordans commented Dec 19, 2024

@k-yle I think now that #1201 is merged it would be worth restarting this to add the missing presets. Do you want to look into it?

@k-yle
Copy link
Collaborator Author

k-yle commented Dec 20, 2024

@tordans done, and I've updated the table in the PR description.

@Dimitar5555
Copy link
Contributor

Dimitar5555 commented Dec 20, 2024

Having both foot=designated and bicycle=designated seems to break the preset for Cycle & Foot Traffic Island.

https://www.openstreetmap.org/way/1237536836

Both foot=designated bicycle=designated
image image image

Copy link
Collaborator

@tordans tordans left a comment

Choose a reason for hiding this comment

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

Thanks for this, I think it is great to have all those presets for alle the way types ready.

I left a few inline comments.

In addition to those:

a. I think we should update the footway preset to follow the same "rythmn" that these new files have and add the changes from here there as well https://github.com/openstreetmap/id-tagging-schema/blob/main/data/presets/highway/footway/traffic_island.json#L9-L12

b. Did anyone check the wiki data entries already?

c. I think we schon start cleaning up the wiki pages. Ideally, we would have a good wiki page setup, but I am not sure if we should make this a requirement for this PR.

The think that bothers me is, that most of the "how to" on those pages is the same, but we don't have a good way to write this into wiki pages that don't share a primary key. We could just make up a wiki page "Tag:*=traffic_sign" and redirect the other 3 there…
Or we could write the cycleway+path pages with a strong reference to the footway page…

Anyone up to tackling this?


Btw, I was amazed to see how many of those tags are used in Berlin already https://overpass-turbo.eu/s/1VY0

data/presets/highway/path/traffic_island_shared.json Outdated Show resolved Hide resolved
data/presets/highway/cycleway/traffic_island_shared.json Outdated Show resolved Hide resolved
data/presets/highway/path/traffic_island_shared.json Outdated Show resolved Hide resolved
Comment on lines 19 to 21
"addTags": {
"highway": "cycleway",
"cycleway": "traffic_island"
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

addTags
I don't think that we need addTags because all that we need can be part of tags.
Docs: https://github.com/ideditor/schema-builder?tab=readme-ov-file#addtags

The cycleway/bicycle_foot preset uses addTags to force the bicycle=designated but I am not sure we need to force this, see #1411.

removeTags
The docs at https://wiki.openstreetmap.org/wiki/Tag%3Acycleway%3Dtraffic_island and https://wiki.openstreetmap.org/wiki/Tag:footway%3Dtraffic_island go into details on the crossing:island. My understand is, that it is likely that a way has a crossing:island when it is migrated to a *=traffic_island and in this case, we want to remove all crossing:island because those only belong on crossing=* ways.

Suggested change
"addTags": {
"highway": "cycleway",
"cycleway": "traffic_island"
},
"removeTags": {
"crossing:island": "*",
},

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

[please ignore previous deleted comment 🤦]

I think these removeTags belong on the crossing presets, not the traffic island presets right? because we want crossing:* to be removed when changing from crossing to traffic_island

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

regarding bicycle=designated should we keep it for consistency and change it later if #1411 is accepted?

Otherwise it might be confusing if only the refuge island preset matches, but not the connected paths. Example here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@k-yle I agree, lets keep the presets in sync and update them later, if needed.

This comment was marked as resolved.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these removeTags belong on the crossing presets, not the traffic island presets right? because we want crossing:* to be removed when changing from crossing to traffic_island

Let's leave this out of V1 of this addition and maybe add it later… It makes this PR more complex that it has to be.

The use case I was thinking of is… Users use crossing:island=yes on ways when they don't split of the traffic_island part of the way. Then, at some point, users want to add details and split off the traffic_island part. Which means that a ways that already has crossing:island=yes gets transformed into a …=traffic_island which would be when this removeTags gets triggered (right?).

But again, this should be done separately…

data/presets/highway/cycleway/traffic_island.json Outdated Show resolved Hide resolved
Copy link
Collaborator

@tordans tordans left a comment

Choose a reason for hiding this comment

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

I think this looks great now.

@tordans tordans merged commit 874cea8 into openstreetmap:main Dec 25, 2024
5 checks passed
@k-yle k-yle deleted the traffic branch December 25, 2024 23:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add 2 more presets for refuge islands
4 participants