Skip to content

add scheme to replace parking:lane:_side_=parallel/diagonal/perpendicular#918

Closed
tiptoptom wants to merge 0 commit intoopenstreetmap:mainfrom
tiptoptom:main
Closed

add scheme to replace parking:lane:_side_=parallel/diagonal/perpendicular#918
tiptoptom wants to merge 0 commit intoopenstreetmap:mainfrom
tiptoptom:main

Conversation

@tiptoptom
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 8, 2023

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

@tyrasd
Copy link
Copy Markdown
Member

tyrasd commented Jun 11, 2023

@tordans could you take a look at this PR perhaps? Is the assosciation of parking:lane:both=yes to parking:both=lane fine (I saw that in #879 (comment) you suggested to also use the value yes for the new tag)?

@tyrasd
Copy link
Copy Markdown
Member

tyrasd commented Jul 18, 2023

(small ping @tordans)

Copy link
Copy Markdown
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.

Sorry for taking so long…

I suggest to being a bit more conservative with the updates.

I talked to @SupaplexOSM he was asking about auto updating

parking:lane:<side>:<orientation>=<position> to parking:<side>=lane

That would soften the change I suggest here, since most of the time there is an update path for both parking and parking orientation.

The disadvantage of adding parking:lane:<side>:<orientation>=<position> would be, that those are quite a few combinations of tags (due to the very unfortunate side-orientation-key…) so we would increase the deprecation list quite a bit. However, looking at the usage, only one set is really relevant. Everything below 3k can be be ignored IMO

parking:lane:both:parallel; 20,170
parking:lane:both:parallel=on_street; 15,311

parking:lane:left:parallel; 13,569
parking:lane:left:parallel=on_street; 7.350

parking:lane:right:parallel; 17,331
parking:lane:right:parallel=on_street; 9,602

parking:lane:both:diagonal; 264
parking:lane:both:marked; 99
parking:lane:both:perpendicular; 1,038
parking:lane:left:diagonal; 1,329
parking:lane:left:marked; 182
parking:lane:left:perpendicular; 2,018
parking:lane:right:diagonal; 1,525
parking:lane:right:marked; 185
parking:lane:right:perpendicular; 2,001

That would mean also adding the deprecation rules …

 {
    "old": {"parking:lane:both:parallel": "on_street"},
    "replace": {"parking:both": "lane"}
  },
 {
    "old": {"parking:lane:left:parallel": "on_street"},
    "replace": {"parking:left": "lane"}
  },
 {
    "old": {"parking:lane:right:parallel": "on_street"},
    "replace": {"parking:right": "lane"}
  },

Update: I thought about changing this to use the "update tag but keep value" version of the deprecation.json (see https://github.com/ideditor/schema-builder/pull/105/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R710-R717), but that would also preserve values like "marked" which we do not want in the new schema. So better to keep the specific version above.

Comment thread data/deprecated.json Outdated
Comment on lines +1209 to +1210
"old": {"parking:lane:both": "yes"},
"replace": {"parking:both": "lane"}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"old": {"parking:lane:both": "yes"},
"replace": {"parking:both": "lane"}
"old": {"parking:lane:both": "yes"},
"replace": {"parking:both": "yes"}

When we build https://osmberlin.github.io/osm-tag-updater/#/manual , we made it that
INPUT: parking:lane:both=yes
OUTPUT: parking:both=yes

Our idea being…

  • it is likely lane, but we cannot be sure
  • its better to have it a non standard yes so people get nudged to update it, that to auto update to something that might be wrong

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But isn't it safe with parkin:lane:both that it is lane?

Comment thread data/deprecated.json Outdated
},
{
"old": {"parking:lane:both": "fire_lane"},
"replace": {"parking:both": "no", "parking:both:restriction": "no_stopping", "parking:both:restriction:reason": "fire_lane"}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Side node: We do not have a preset for the *:reason tag, so this will become "invisible" to users that do not look at the "Tags" area. That is OK IMO. Its just a +1 to some day have a even more advanced preset system that would allow to have something like the reason more visible.

Comment thread data/deprecated.json Outdated
Comment on lines +1236 to +1247
{
"old": {"parking:lane:both": "diagonal"},
"replace": {"parking:both": "lane", "parking:both:orientation": "diagonal"}
},
{
"old": {"parking:lane:both": "parallel"},
"replace": {"parking:both": "lane", "parking:both:orientation": "parallel"}
},
{
"old": {"parking:lane:both": "perpendicular"},
"replace": {"parking:both": "lane", "parking:both:orientation": "perpendicular"}
},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{
"old": {"parking:lane:both": "diagonal"},
"replace": {"parking:both": "lane", "parking:both:orientation": "diagonal"}
},
{
"old": {"parking:lane:both": "parallel"},
"replace": {"parking:both": "lane", "parking:both:orientation": "parallel"}
},
{
"old": {"parking:lane:both": "perpendicular"},
"replace": {"parking:both": "lane", "parking:both:orientation": "perpendicular"}
},
{
"old": {"parking:lane:both": "diagonal"},
"replace": {"parking:both:orientation": "diagonal"}
},
{
"old": {"parking:lane:both": "parallel"},
"replace": {"parking:both:orientation": "parallel"}
},
{
"old": {"parking:lane:both": "perpendicular"},
"replace": {"parking:both:orientation": "perpendicular"}
},

Same as with my comment above: I would think we should rather be more conservative when updating the tags that auto-fill data that might not be true. When you add parking:lane:both= diagonal to https://osmberlin.github.io/osm-tag-updater/#/manual we only transform the given key but show a yellow info to add the missing key manually.
Unfortunately, the way iD works is that the orientation field will become visible, but the "parking" field does not.
It would be better to have it show like …
image

However I tried that but we are missing the capabilities to show fields conditionally depending on other tags.

Since the deprecations happen automatically (as in, the user is not made aware that things change), I would still vote to only update what we are 100% sure about.

Comment thread data/deprecated.json Outdated
Comment on lines +1276 to +1287
{
"old": {"parking:lane:left": "diagonal"},
"replace": {"parking:left": "lane", "parking:left:orientation": "diagonal"}
},
{
"old": {"parking:lane:left": "parallel"},
"replace": {"parking:left": "lane", "parking:left:orientation": "parallel"}
},
{
"old": {"parking:lane:left": "perpendicular"},
"replace": {"parking:left": "lane", "parking:left:orientation": "perpendicular"}
},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same as above

Suggested change
{
"old": {"parking:lane:left": "diagonal"},
"replace": {"parking:left": "lane", "parking:left:orientation": "diagonal"}
},
{
"old": {"parking:lane:left": "parallel"},
"replace": {"parking:left": "lane", "parking:left:orientation": "parallel"}
},
{
"old": {"parking:lane:left": "perpendicular"},
"replace": {"parking:left": "lane", "parking:left:orientation": "perpendicular"}
},
{
"old": {"parking:lane:left": "diagonal"},
"replace": {"parking:left:orientation": "diagonal"}
},
{
"old": {"parking:lane:left": "parallel"},
"replace": {"parking:left:orientation": "parallel"}
},
{
"old": {"parking:lane:left": "perpendicular"},
"replace": {"parking:left:orientation": "perpendicular"}
},

Comment thread data/deprecated.json Outdated
Comment on lines +1316 to +1327
{
"old": {"parking:lane:right": "diagonal"},
"replace": {"parking:right": "lane", "parking:right:orientation": "diagonal"}
},
{
"old": {"parking:lane:right": "parallel"},
"replace": {"parking:right": "lane", "parking:right:orientation": "parallel"}
},
{
"old": {"parking:lane:right": "perpendicular"},
"replace": {"parking:right": "lane", "parking:right:orientation": "perpendicular"}
},
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same as above

Suggested change
{
"old": {"parking:lane:right": "diagonal"},
"replace": {"parking:right": "lane", "parking:right:orientation": "diagonal"}
},
{
"old": {"parking:lane:right": "parallel"},
"replace": {"parking:right": "lane", "parking:right:orientation": "parallel"}
},
{
"old": {"parking:lane:right": "perpendicular"},
"replace": {"parking:right": "lane", "parking:right:orientation": "perpendicular"}
},
{
"old": {"parking:lane:right": "diagonal"},
"replace": {"parking:right:orientation": "diagonal"}
},
{
"old": {"parking:lane:right": "parallel"},
"replace": {"parking:right:orientation": "parallel"}
},
{
"old": {"parking:lane:right": "perpendicular"},
"replace": {"parking:right:orientation": "perpendicular"}
},

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.

3 participants