[PATCH] Preset proposes deprecated key parking:orientation

[Famlam]

What steps will reproduce the problem?

1. Add an area tagged with amenity=parking
2. Open the preset link
3. See (at the bottom) the option to add parking:orientation

What is the expected result?

The option adds the tag orientation

What happens instead?

The option adds the tag parking:orientation

Please provide any additional information below. Attach a screenshot if possible.

parking:orientation on amenity=parking has been deprecated in favor of orientation.
Based on the graphical history, it will also soon take over in terms of number of objects in the database.
Hence, I think it would be good if the preset of JOSM uses the new key instead.

Wiki old key: ​https://wiki.openstreetmap.org/wiki/Key:parking:orientation
Wiki new key: ​https://wiki.openstreetmap.org/wiki/Key:orientation
Proposal: ​https://wiki.openstreetmap.org/wiki/Proposal:Street_parking_revision
Tag history plot: ​https://taghistory.raifer.tech/#***/orientation/&***/parking%3Aorientation/

Revision:18721
Build-Date:2023-05-03 12:38:30

Identification: JOSM/1.5 (18721 nl) Windows 10 64-Bit
OS Build number: Windows 10 Home 2009 (19045)
Memory Usage: 380 MB / 2012 MB (57 MB allocated, but free)
Java version: 17.0.7+7-LTS, Azul Systems, Inc., OpenJDK 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920×1080 (scaling 1.10×1.10)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: Cp1252
System property sun.jnu.encoding: Cp1252
Locale info: nl_NL
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Djpackage.app-version=1.5.18721, --add-modules=java.scripting,java.sql,javafx.controls,javafx.media,javafx.swing,javafx.web, --add-exports=java.base/sun.security.action=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-opens=java.base/java.nio=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED, --add-opens=java.desktop/javax.imageio.spi=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, -Djpackage.app-path=%UserProfile%\AppData\Local\JOSM\JOSM.exe]
Dataset consistency test: No problems found

Plugins:
+ OpeningHoursEditor (35924)
+ SimplifyArea (35978)
+ imagery_offset_db (35978)
+ pbf (36034)
+ pt_assistant (2.1.15-19-g9aeec3c-SNAPSHOT)
+ reverter (36043)
+ tageditor (36011)
+ turnlanes-tagging (v0.0.5)
+ undelete (36011)
+ utilsplugin2 (36011)

Tagging presets:
+ http://mijndev.openstreetmap.nl/~allroads/JOSM/Presets/NL-Fiets.zip

Map paint styles:
- https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransport&zip=1
+ %UserProfile%\Documents\tijdelijke bestanden\josm-eigen.mappaint.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Styles/Sidewalks&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/ParkingLanes&zip=1

Validator rules:
+ %UserProfile%\Documents\tijdelijke bestanden\josm-eigen.validator.mapcss
+ https://josm.openstreetmap.de/josmfile?page=Rules/SuspiciousSwimming_Pool&zip=1
+ https://raw.githubusercontent.com/Famlam/OsmMapcssValidationNL/main/netherlands.validator.mapcss
+ https://raw.githubusercontent.com/osm-fr/osmose-backend/master/plugins/TagFix_Destination.validator.mapcss
+ https://raw.githubusercontent.com/osm-fr/osmose-backend/master/plugins/Colour.validator.mapcss
+ https://raw.githubusercontent.com/osm-fr/osmose-backend/master/plugins/notprefix.validator.mapcss
+ https://raw.githubusercontent.com/osm-fr/osmose-backend/master/plugins/TagFix_MultipleTag2.validator.mapcss

Last errors/warnings:
- 00000.592 W: extended font config - overriding 'filename.Myanmar_Text=mmrtext.ttf' with 'MMRTEXT.TTF'
- 00000.594 W: extended font config - overriding 'filename.Mongolian_Baiti=monbaiti.ttf' with 'MONBAITI.TTF'
- 00001.594 E: java.security.KeyStoreException: Windows-ROOT not found. Oorzaak: java.security.NoSuchAlgorithmException: Windows-ROOT KeyStore not available

[taylor.smock]

The parking:orientation tag was added in r17936 (see #20192).

We are going to need to add a deprecation warning for parking:orientation on amenity=parking as well.

[Famlam]

We are going to need to add a deprecation warning for parking:orientation on amenity=parking as well.

The validation rule fortunately isn't difficult, either:

way[parking:orientation][!orientation][amenity=parking] {
  throwWarning: tr("{0} is deprecated", "{0.tag}");
  group: tr("deprecated tagging");
  suggestAlternative: "{1.key}={0.value}";
  fixChangeKey: "{0.key} => {1.key}";
}
way[parking:orientation][orientation][amenity=parking] {
  throwWarning: tr("{0} is deprecated", "{0.tag}");
  group: tr("deprecated tagging");
  fixRemove: "{0.key}";
}

or:

way[parking:orientation][amenity=parking] {
  throwWarning: tr("{0} is deprecated", "{0.tag}");
  group: tr("deprecated tagging");
  suggestAlternative: "orientation={0.value}";
  fixChangeKey: "{0.key} => orientation";
}

[JeroenHoek]

This should fix it.

[JeroenHoek]

Thanks for the deprecation warning, that saves me a bunch of effort.

[JeroenHoek]

@taylor.smock: would you happen to know off-hand which line (if any) should be added to resources/data/validator/ignoretags.cfg?

[taylor.smock]

For way[parking:orientation][orientation][amenity=parking], I'd be a lot more comfortable if the fixRemove only occurred when the values in parking:orientation and orientation are the same.

For ignoretags.cfg, it should be between 412 and 743. I really need to sort that list, but I think I need to sort it based off of the key, not the prefix.

[JeroenHoek]

Patch updated with feedback.

[Famlam]

For way[parking:orientation][orientation][amenity=parking], I'd be a lot more comfortable if the fixRemove only occurred when the values in parking:orientation and orientation are the same.

Ok, then it would be:

--- removed because outdated

EDIT: Jeroen was just faster. May I however propose to use my suggestion instead if you don't mind? The =* operator should be more safe with i.e. misspellings in the values (or if more values are added later). And it's simpler ;)

If not, I would recommend to change the patch by Jeroenhoek to have [parking:orientation=X][orientation][orientation!=X], rather than explicitly listing the 3 official values hardcoded in every line. Also, the 1, 2 and 3 from the warning text should be removed, probably leftovers from debugging? Lastly, good to suggest what to use instead in the message with number 3 (note: you can use multiple suggestAlternative, as you can see in my suggestion).

[taylor.smock]

As a somewhat stupid question, is there any particular reason why way was used instead of area? The wiki page seems to indicate that it should be on areas or points.

Instead of setting hasFixParkingOrientation, did you consider using ["parking:orientation"!=*orientation]?

[Famlam]

Good point.
Regarding the set: I thought it would maybe be slightly faster as the set is a boolean, while !=* is another comparison to be made, but probably the effect is minimal.

Updated rules below:

area[parking:orientation][!orientation][amenity=parking] {
  throwWarning: tr("{0} is deprecated", "{0.tag}");
  group: tr("deprecated tagging");
  suggestAlternative: "{1.key}={0.value}";
  fixChangeKey: "{0.key} => {1.key}";
}
area[parking:orientation][orientation]["parking:orientation"=*orientation][amenity=parking] {
  throwWarning: tr("{0} is deprecated", "{0.tag}");
  group: tr("deprecated tagging");
  fixRemove: "{0.key}";
}
area[parking:orientation][orientation]["parking:orientation"!=*orientation][amenity=parking] {
  throwWarning: tr("{0} is deprecated", "{0.tag}");
  group: tr("deprecated tagging");
  suggestAlternative: "{1.key}={0.value}";
  suggestAlternative: "{1.key}={1.value}";
}

(Not sure if you want to warn for nodes, they're relatively rare (2k vs 83k). If so, the 3x area can become 3x *)

@JeroenHoek would you mind updating the patch please? I can only make Github patches, haven't figured out the magic trick of trac yet

[JeroenHoek]

Yeah, SVN and Trac are a bit of a barrier to entry. The easiest way to do this is to use git with the JOSM git mirror, do your work in a local branch and commit (don't do git commit -a, manually select the changed files because the .gitignore misses a few files), and use git-format-patch HEAD~ to generate the patch file.

Don't bother with SVN; it's obsolete.

[JeroenHoek]

Patch adressing this issue

[taylor.smock]

Replying to Famlam:

@JeroenHoek would you mind updating the patch please? I can only make Github patches, haven't figured out the magic trick of trac yet

There is an Attachments section right below the ticket description. There you can attach files.

Anyway, I do look at GitHub PRs (the repo is located at ​https://github.com/JOSM/josm ) when someone links to them on Trac.

Regarding the set: I thought it would maybe be slightly faster as the set is a boolean, while !=* is another comparison to be made, but probably the effect is minimal.

Considering that there are 85k uses, I think that it will have a fairly minimal effect for most users.
The first two checks (does it have orientation and parking:orientation tags) should do the bulk of the filtering.
I'll try to profile it next week to see which is better. But I don't think it will be a significant difference.

[skyper]

I'd prefer the second one as it is much safer to warn about conflicts in values.
I do not think parking:orientation should be ignored as it is only deprecated as combination of amenity=parking but not in general. Currently combinations are not supported.

[Famlam]

Taylor: I'll try to profile it next week to see which is better. But I don't think it will be a significant difference.

I suspect you're right, milliseconds at most probably :)

Skyper: I'd prefer the second one as it is much safer to warn about conflicts in values.

Not sure what this is referring to, there's only one patch?

Skyper: I do not think parking:orientation should be ignored as it is only deprecated as combination of amenity=parking but not in general. Currently combinations are not supported.

I don't think parking:orientation is used anywhere else, right?
(The keys parking:left:orientation, parking:right:orientation and parking:both:orientation for use on highways are not affected by this patch)

[JeroenHoek]

No, parking:orientation was specifically introduced for street-side parking (by the proposal authors, which includes me). Deprecating it fully is correct.

[taylor.smock]

There are 71,065 objects with amenity=parking and parking:orientation. There are 85,888 objects with parking:orientation. Of the remaining 14,823 objects, 14,189 have amenity=parking_space, 446 have amenity=motorcycle_parking, followed by a few other tags that are probably specialized parking tags. And a typo. (amenity=7).

[taylor.smock]

In my two runs (one each), it looks like the =* was actually a bit more efficient, which was surprising. But it might just be some jitter.

With that said, I am now looking at a function to see if it can be optimized.

[Famlam]

Of the remaining 14,823 objects, 14,189 have amenity=parking_space, 446 have amenity=motorcycle_parking, followed by a few other tags that are probably specialized parking tags. And a typo. (amenity=7).

So do you prefer to have [amenity=~/^parking(_space)?$/] instead of [amenity=parking]? The numbers are high enough to make a warning for it interesting. (Probably not worth bothering about the 446 motorcycle_parkings)
It's undocumented for amenity=parking_space (but it was added to JOSM with the intention to be a street parking tag, meaning it's now deprecated as I can't think of secondary purposes), and also the usage numbers of orientation (27k) vs. parking:orientation (14k) in combination with that tag suggest that deprecating it is intended.

[taylor.smock]

Since the vast majority of uses are amenity=parking and amenity=parking_space (99.26%), I think we can safely drop the [amenity=parking] entirely. If we account for motorcycle_parking, that is 99.78% (188 objects remaining).

For the record, I used the following overpass query:

[out:xml][timeout:90];
(
  nwr["parking:orientation"];
);
(._;>;);
out meta;

[Famlam]

Fair point! New patch attached (the easy way: manually modifying JeroenHoeks file :) )

[skyper]

Please forget my previous comment 14.

[taylor.smock]

In 18736/josm:

Fix #22957: parking:orientation is deprecated (patch by Famlam and JeroenHoek)

parking:orientation was introduced by a proposal on 2020-11-28 (JeroenHoek was
one of the authors). parking:orientation was deprecated by a new proposal on
2022-12-08.

[stoecker]

Ticket #22981 has been marked as a duplicate of this ticket.