#22957 closed defect (fixed)
[PATCH] Preset proposes deprecated key parking:orientation
| Reported by: | Famlam | Owned by: | team |
|---|---|---|---|
| Priority: | normal | Milestone: | 23.05 |
| Component: | Internal preset | Version: | |
| Keywords: | template_report parking orientation | Cc: | JeroenHoek |
Description
What steps will reproduce the problem?
- Add an area tagged with
amenity=parking - Open the preset link
- 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
Attachments (2)
Change History (27)
comment:1 by , 3 years ago
| Cc: | added |
|---|
comment:2 by , 3 years ago
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"; }
comment:3 by , 3 years ago
| Summary: | Preset proposes deprecated key parking:orientation → [PATCH] Preset proposes deprecated key parking:orientation |
|---|
comment:5 by , 3 years ago
Thanks for the deprecation warning, that saves me a bunch of effort. I've used the first suggestion in the patch. It works as expected.
comment:6 by , 3 years ago
@taylor.smock: would you happen to know off-hand which line (if any) should be added to resources/data/validator/ignoretags.cfg?
comment:7 by , 3 years ago
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.
comment:9 by , 3 years ago
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).
comment:10 by , 3 years ago
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]?
follow-up: 13 comment:11 by , 3 years ago
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
comment:12 by , 3 years ago
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.
by , 3 years ago
| Attachment: | 0001-Deprecated-tag-parking-orientation.patch added |
|---|
Patch adressing this issue
comment:13 by , 3 years ago
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.
follow-up: 22 comment:14 by , 3 years ago
I'd prefer the second one as it is much safer to warn about conflicts in values.
I do not think Currently combinations are not supported.parking:orientation should be ignored as it is only deprecated as combination of amenity=parking but not in general.
comment:15 by , 3 years ago
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)
comment:16 by , 3 years ago
No, parking:orientation was specifically introduced for street-side parking (by the proposal authors, which includes me). Deprecating it fully is correct.
comment:17 by , 3 years ago
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).
comment:18 by , 3 years ago
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.
comment:19 by , 3 years ago
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.
comment:20 by , 3 years ago
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;
by , 3 years ago
| Attachment: | 0002-Deprecated-tag-parking-orientation_JeroenHoek_Famlam.patch added |
|---|
comment:21 by , 3 years ago
Fair point! New patch attached (the easy way: manually modifying JeroenHoeks file :) )
comment:23 by , 2 years ago
| Milestone: | → 23.05 |
|---|



The
parking:orientationtag was added in r17936 (see #20192).We are going to need to add a deprecation warning for
parking:orientationonamenity=parkingas well.