[patch] Deprecate vending=parcel_pickup

[overflorian]

Hi,
A new tag just ​has been approved: ​Tag:amenity=parcel_locker and describes a parcel locker.
It replaces previously used (and now deprecated) amenity=vending_machine + vending=parcel_pickup

I propose to:

• While a amenity=vending_machine is created, stop proposing the associated preset vending=parcel_pickup
• Create a new dedicated preset for amenity=parcel_locker
• Change the validator accordingly

Thanks

PS: deprecation follow-up (including similar requests to other tools) is available on this wiki page: ​https://wiki.openstreetmap.org/wiki/Tag:vending%3Dparcel_pickup#Deprecation_follow-up

[gaben]

A preset is already available for download, see Presets/ParcelLockers.

[aceman]

Thanks, but that preset offers also undocumented tags like 'parcel_locker=', or mixes name and number into a single tag 'ref='.

Anyway, JOSM should have this built in, because the lockers are very popular in the wild. The real extent in OSM is unknown, as many have been tagged as amenity=vending_machine as that was the method at the time.

[gaben]

The icon from the above preset would do it for JOSM?

[gaben]

Please try out the provided patch.

The preset structure is not appealing for me, but this is the best I could came up with.

Edit: the icon is from the above preset (​here), but slightly modified by me.

[gaben]

initial patch

[gaben]

add missing commas, change mapcss rule

[gaben]

The messages still not working as one would expect, but it's JOSM's limitation. With regex, sublist and probably more, the {*.key} property cannot read out.

One possible solution is duplicating rules:

*[amenity=vending][vending=parcel_mail_in],
*[amenity=vending][vending=parcel_pickup],
*[amenity=vending][vending=parcel_mail_in;parcel_pickup],
*[amenity=vending][vending=parcel_pickup;parcel_mail_in] { ... }

but it isn't elegant ...and doesn't work. I'm out of ideas. Probably a new custom message is needed.

[skyper]

Some remarks:

• I guess the icon needs more colors/contrast. Only one color does not work well, regarding different LaFs and backgrounds.
• The style should support areas and not only nodes.
• The validator rule does not work so far, as parcel_mail_in and/or parcel_pickup need to be preserved as keys. Maybe, we need several rules for different cases.

[gaben]

• Yes, a white bg would do it, like the hunting stand's icon.
• The wiki page was modified since then, allowing areas as well, thanks for noticing.
• What do you mean? It is preserved by the patch, only ignoring vending=parcel_mail_in/parcel_pickup.

[skyper]

• A background might work, but actually I am not convinced about the icon and its recognition.
• The wiki was wrong as prior use and the proposal included areas.
• No, I would expect to add parcel_pickup=yes if the key is not present and vending=parcel_pickup is removed. Same for parcel_mail_in=*.

[kubahaha]

How is it going? Since amenity=parcel_locker is added even to default carto style

[gaben]

I've created the base patch, and looking at the wiki page we got a new icon since then.

@skyper can you work on this or should I proceed with the patch?

[skyper]

@gaben:
Please, go ahead. I might have time to comment but I won't work on it.

[gaben]

Okay, currently investigating why the {0.key} is not working with the ~= selector. To my understanding, it should work.

[gaben]

Please review, I think it's done.

[skyper]

Yes, much better but:

• the warnings should check for amenity=vending_machine
  • not sure if we need to check for existing parcel_pickup=* and parcel_mail_in=* before overwriting
• vending=parcel_mail_in and vending=parcel_pickup should be removed from presets (was in v2 but is missing in v3)
• rendering of the deprecated tags needs to be changed to deprecated.svg for nodes and disabled for areas
  • the old icon should probably be moved to nodist/

Replying to gaben:

Okay, currently investigating why the {0.key} is not working with the ~= selector. To my understanding, it should work.

As workaround you can add an additional selector [key] to get the key and/or its values.

[gaben]

Thanks for the review!

• I was thinking about adding the amenity=vending_machine to the filter, but I left it out intentionally. It does not harm, so readded.
• Good catch! Probably a result of working locally (no git branch or commits) on 4 tickets in parallel. Fixed.
• Hmm, I may miss something. The main tag amenity=vending_machine is still used. I checked all the deprecated.svg usages, and none of them was a combination of a main and subtag, like in our case (amenity=vending_machine + vending=parcel_pickup|parcel_mail_in).

[skyper]

Thanks for re-adding amenity=vending_machine to the filter. It is important as without it other values of amenity=* would be overwritten by the fix.
Sorry, I was off regarding the rendering. Thought we have more individual icons but that is not the case. Anyway, I still think deprecated.svg for the combination is useful as eyes' catcher but not that important.

[gaben]

added amenity=vending_machine filter, remove deprecated vending values, remove amenity=parcel_locker from ignoretags.cfg

[gaben]

Now it looks good, but not sure if ignoretags.cfg needs individual entries for tag values:

K:vending=parcel_pickup
K:vending=parcel_mail_in
K:vending=parcel_pickup;parcel_mail_in

So the question, is it missing K:vending=parcel_mail_in;parcel_pickup (switched order) or the first two is unnecessary?

Anyway, it's minor. Whoever will commit, please have a look at it.

[taylor.smock]

Milestone renamed

[taylor.smock]

Ticket retargeted after milestone closed

[taylor.smock]

Somewhat stupid question:
What is supposed to happen if something is tagged vending=parcel_pickup;parcel_mail_in;stamps?

As an aside, there is a list membership comparator, so instead of [vending="parcel_mail_in;parcel_pickup"] and [vending="parcel_pickup;parcel_mail_in"] you can do [vending~=parcel_mail_in][vending~=parcel_pickup]. If you do that, you will probably want to set .deprecated_parcel or something and use !.deprecated_parcel to avoid having multiple warnings for the same thing.

[gaben]

Yes, there is the list membership comparator, although if I remember correctly, the problem is the fixAdd: "{0.value}=yes"; part is not working as mentioned in comment:6 and comment:13.

Probably it's a corner case, so not a bug, just no one thought about it yet.

[gaben]

Looping in Klumbumbus. What do you think about the provided patch? I couldn't find any other possibility.

[taylor.smock]

I was thinking of something like the following (trimmed for size)

/* #21782 */
*[amenity=vending_machine][vending=parcel_mail_in],
*[amenity=vending_machine][vending=parcel_pickup] {
  set .parcel_checked;
  throwWarning: tr("{0} is deprecated", "{1.tag}");
  group: tr("deprecated tagging");
}
/** It would be nice if we could just say list type only has the following values */
*[amenity=vending_machine][vending~=parcel_mail_in][vending~=parcel_pickup][count(uniq_list(split(";", tag("vending")))) == 2] {
  set .parcel_checked;
  throwWarning: tr("{0} is deprecated", "{1.tag}");
  group: tr("deprecated tagging");
}
/** This is just for cases where we have stamp;parcel_mail_in or stuff like that */
*[amenity=vending_machine][vending~=parcel_mail_in]!.parcel_checked,
*[amenity=vending_machine][vending~=parcel_pickup]!.parcel_checked {
  throwWarning: tr("{0} is deprecated", "{1.tag}");
  group: tr("deprecated tagging");
}

[taylor.smock]

Ticket retargeted after milestone closed

[taylor.smock]

Ticket retargeted after milestone closed