[PATCH] Support new conditional parking lane tagging in validator

[riiga_92@…]

What steps will reproduce the problem?

1. Add, for example, the tag "parking:condition:both:conditional=no_parking @ (Mo-Fr 08:00-18:00)" to a road.
2. Run validator (or attempt to upload changes).

What is the expected result?

Tagging is valid and no warnings are raised.

What happens instead?

Warnings are given for valid tagging.

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

I've made a possible patch, but I lack the ability to test it as I don't have a development environment for JOSM set up.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2022-01-02 21:24:43 +0100 (Sun, 02 Jan 2022)
Revision:18360
Build-Date:2022-01-02 20:26:19
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (18360 en) Linux Manjaro Linux
Memory Usage: 938 MB / 4002 MB (203 MB allocated, but free)
Java version: 17.0.3+3, N/A, OpenJDK 64-Bit Server VM
Look and Feel: javax.swing.plaf.metal.MetalLookAndFeel
Screen: :0.0 1920×1200 (scaling 1.00×1.00)
Maximum Screen Size: 1920×1200
Best cursor sizes: 16×16→16×16, 32×32→32×32
Environment variable LANG: sv_SE.UTF-8
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_SE
Numbers with default locale: 1234567890 -> 1234567890
Desktop environment: KDE
VM arguments: [--add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, -Djosm.restart=true, -Dawt.useSystemAAFontSettings=gasp]

Plugins:
+ AddrInterpolation (35893)
+ FastDraw (35893)
+ ImportImagePlugin (35893)
+ OpeningHoursEditor (35893)
+ PicLayer (1.0.1)
+ apache-commons (35893)
+ apache-http (35893)
+ areaselector (368)
+ austriaaddresshelper (1597341117)
+ buildings_tools (35893)
+ ejml (35893)
+ geotools (35893)
+ imagery_offset_db (35893)
+ jaxb (35893)
+ jna (35893)
+ jogl (1.2.3)
+ jts (35893)
+ kendzi3d-resources (0.0.2)
+ log4j (35893)
+ measurement (35893)
+ opendata (35893)
+ pt_assistant (1ff2e15)
+ reverter (35893)
+ tageditor (35893)
+ terracer (35893)
+ turnlanes-tagging (v0.0.5)
+ turnrestrictions (35893)
+ utilsplugin2 (35893)

Tagging presets:
+ https://josm.openstreetmap.de/josmfile?page=Presets/PublicTransportOneClick&zip=1
+ ${HOME}/Dokument/QuickSets.zip
+ https://github.com/kendzi/Simple3dBuildingsPreset/releases/download/0.9_2018-05-08/s3db-preset.zip
+ https://josm.openstreetmap.de/josmfile?page=Presets/ParkingLanes&zip=1

Map paint styles:
- https://josm.openstreetmap.de/josmfile?page=Styles/Fixme&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/LayerChecker&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Streets&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Maxspeed&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Surface-DataEntry&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_buildings_sv&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Enhanced_Lane_and_Road_Attributes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransport&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/PriorityRoad&zip=1
- ${HOME}/sidewalks-mod.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/SimpleBuildingTags&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/SimpleRoofTags&zip=1
- https://raw.githubusercontent.com/yopaseopor/indoormap/master/indoormap-style.mapcss
- https://github.com/GlassOceanos/indoor-JOSM-style/archive/master.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/SidewalksAndFootways&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_Postcode&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Admin_Boundaries&zip=1
- https://raw.githubusercontent.com/species/josm-preset-traffic_sign_direction/master/direction.mapcss
- ${HOME}/Dokument/OSM/Styles_ParkingLanes-style.mapcss

Last errors/warnings:
- 04250.842 E: Failed to locate image 'images/warning-flipped.svg'
- 04250.843 E: Failed to locate image 'images/warning.svg'
- 04250.843 E: Failed to locate image 'images/warning.svg'
- 04250.892 E: Failed to locate image 'images/warning-flipped.svg'
- 04250.893 E: Failed to locate image 'images/warning.svg'
- 04250.893 E: Failed to locate image 'images/warning.svg'
- 04252.495 E: Failed to locate image 'images/warning-flipped.svg'
- 04252.496 E: Failed to locate image 'images/warning.svg'
- 04252.784 E: Failed to locate image 'images/warning-flipped.svg'
- 04252.784 E: Failed to locate image 'images/warning.svg'

[riiga_92@…]

Possible patch

[riiga_92@…]

JOSM validator warning

[riiga_92@…]

Seems like I made the patch the wrong around. What's being deleted should be added and vice versa.

[riiga_92@…]

Correct diff (hopefully)

[skyper]

With all these new values only valid for parking:lane:* or parking:condition:* a separate test with own arrays might be better. Additionally, I think that there are keys missing like parking:lane:both:conditional.

[riiga_92@…]

You're right that a separate test might be better. I will see if I can try and write one in the coming weeks.

parking:lane:both:conditional is not a valid key, a physical parking lane cannot be conditional.

[skyper]

Replying to riiga_92@…:

parking:lane:both:conditional is not a valid key, a physical parking lane cannot be conditional.

Oh sorry, after the last changes it would be parking:lane:[SIDE]:[TYPE]:conditional if the position on the street changes.

[taylor.smock]

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

[taylor.smock]

@riiga_92: Are you still working on this patch?

Also, I kind of feel like the parking:lane:[SIDE]:[TYPE]:conditional should be done in a for loop, e.g.

private static void addSubKeyConditionalRestrictions() {
    for (String side : Arrays.asList("left", "right", "both")) {
        RESTRICTION_TYPES.add("parking:condition:" + side);
        RESTRICTION_TYPES.add("parking:condition:" + side + ":maxstay");
    }
}

but having explicit types is OK right now. If it expands to have more subkey types (i.e., "lanes:conditional"), then I'll have a stronger opinion on it.

As a clarification, you said parking:lane:both:conditional is not a valid key, a physical parking lane cannot be conditional. Are you certain about that? Example:
highway=residential + parking:lane:both=parallel + parking:lane:both:parallel=on_street (on_street is defined as Parking on the street, which could be easily converted to a travel lane. ) + parking:lane:both:conditional=no @ 08:00-17:00 + lanes=2 + lanes:conditional=4 @ 08:00-17:00.

I probably borked the lanes:conditional, but I didn't see anything on the wiki for it.

[riiga_92@…]

Not at the moment, I had some other stuff occupy my free time, but I might revisit this patch in the future. I'll add my progress so far. There are still some FIXMEs in the code.

Regarding :conditional on a physical parking lane, I am pretty certain since I was the one who wrote the proposal that was approved, though theoretically you could use that syntax, there's nothing that forbids it, but I didn't have that in mind. Does that type of parking lane exist in real life? If so, perhaps it would make sense to support it.

[riiga_92@…]

Progress in existing files

[riiga_92@…]

New test

[riiga_92@…]

New unit test

[taylor.smock]

Replying to riiga_92@…:

Regarding :conditional on a physical parking lane, I am pretty certain since I was the one who wrote the proposal that was approved, though theoretically you could use that syntax, there's nothing that forbids it, but I didn't have that in mind. Does that type of parking lane exist in real life? If so, perhaps it would make sense to support it.

I think I have seen that somewhere, but for the life of me I cannot recall where.

[skyper]

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

[skyper]

Replying to skyper:

Replying to riiga_92@…:

parking:lane:both:conditional is not a valid key, a physical parking lane cannot be conditional.

Oh sorry, after the last changes it would be parking:lane:[SIDE]:[TYPE]:conditional if the position on the street changes.

With the last accepted proposal :lane was dropped and it is now parking:[SIDE]:[TYPE]:conditional

[pixunil]

Patch for validating parking:[SIDE]:[RESTRICTION]:conditional

[pixunil]

Hello,
I ran into the false warnings of for parking:[SIDE]:[TYPE]:conditional and added those tags to ConditionalKeys.java. Some remarks:

• I do not know what the normal policy in JOSM for deprecated tagging is, but I excluded parking:lane:* and parking:condition:* from the patch and only included the accepted ​Street parking.
• There were modes with a colon which effectively don't get matched (like ski:alpine). I added those as tests that keys like ski:alpine:conditional are deemed valid.
• Some of the parking restriction keys also contain a colon (like parking:[SIDE]:restriction:reason:conditional. As this made parsing really hard by operating on chunks of key parts splitted by colon, I used a map to store all known keys to be valid. Normally I try to make changes to be small, but I couldn't find another way.
• I added the check for access values for parking:[SIDE]:[MODE]:conditional, like currently for [MODE]:conditional. However, I noticed that when validating [MODE] (not conditional) with an invalid value, only a INFO is raised (Value ''some invalid value'' for key ''bicycle'' not in presets.), so maybe it would be better in future to delegate the value validation to TagChecker.
• For the ease of my development, I changed the unit tests to be @ParameterizedTests. Later I noticed that the overhead per test is quite high (~ 300ms, which quickly adds up to 10s). As I do not understand the test set up, I need either information how to pay the overhead only once or else I could revert to methods full of assertions.
• The patch does not include *:lanes:[DIRECTION]:conditional, but I know that this is also a false negative. However, I wanted to keep the patch small, because the change could be a bit controversial.

As this is my first patch contribution, please let me know if anything is missing / unclear.

[skyper]

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

[skyper]

As the current warnings are wrong, this is a defect.
See also ​https://community.openstreetmap.org/t/111690

[gaben]

I've just tested the proposed patch​, and I'm getting a warning to a way tagged with parking:condition:both:conditional=no_parking @ (Mo-Fr 08:00-18:00) on r19283.

[gaben]

...which is fine, because that kind of tagging was deprecated in 2022, by the ticket author.

Old ​Key:parking:condition, new ​Street_parking.

@riiga_92, could you please actualize the ticket description?

[riiga_92@…]

@gaben: How? I can't find any way to edit the description field.

[gaben]

Just above the text input field where you type the comment, there is an openable menu, you'll se everything what is needed.

[francians]

Hello,

please ensure that also:

parking:right:restriction:conditional=no_stopping @ (Fr[2] 05:00-08:00)

is considered correct from the validator.
For reference it is the second Friday of every month.
Thanks
Best Regards
francians