Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parcel locker category change #339

Merged
merged 10 commits into from
Jan 10, 2022

Conversation

ttomasz
Copy link
Contributor

@ttomasz ttomasz commented Jan 7, 2022

Implements proposal changing parcel lockers tagging which was recently approved: https://wiki.openstreetmap.org/wiki/Proposed_features/amenity%3Dparcel_locker

Essentially just moves presets that had vending_machine=parcel_pickup to amenity=parcel_locker and adds old tags to deprecated.json

Also added some fields (description, ref, payment) and some terms

tyrasd
tyrasd previously requested changes Jan 7, 2022
Copy link
Member

@tyrasd tyrasd left a comment

Choose a reason for hiding this comment

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

Strictly speaking this is missing a preset for "drop off only" parcel locker, right? I guess those are pretty uncommon, but they would be misclassified as a "Parcel Pickup/Dropoff Locker" with the proposed solution, which is not great. Previously they didn't match any of the presets.

Why not consolidate the two presets into a single "Parcel Locker" one with two new checkbox-fields for the tags parcel_pickup (placeholder could be "assumed to yes") and parcel_mail_in respectively?

data/deprecated.json Outdated Show resolved Hide resolved
@tyrasd
Copy link
Member

tyrasd commented Jan 7, 2022

PS: It would also be good if there was a proper (non-proposal) wiki page before we include the preset in iD.

@matkoniecz
Copy link
Contributor

Why not consolidate the two presets into a single "Parcel Locker" one with two new checkbox-fields for the tags parcel_mail_in (placeholder could be "assumed to yes") and parcel_mail_in respectively?

This way tagging it would require more clicks (instead of selecting it from the list -> select basic packstation, select both checkboxes)

@ttomasz
Copy link
Contributor Author

ttomasz commented Jan 7, 2022

Strictly speaking this is missing a preset for "drop off only" parcel locker, right? I guess those are pretty uncommon, but they would be misclassified as a "Parcel Pickup/Dropoff Locker" with the proposed solution, which is not great. Previously they didn't match any of the presets.

You're right. This was a mostly theoretical possibility for me since I have never seen such locker but as it's allowed it's good to have a preset for it. I added it.

Why not consolidate the two presets into a single "Parcel Locker" one with two new checkbox-fields for the tags parcel_mail_in (placeholder could be "assumed to yes") and parcel_mail_in respectively?

I guess one preset might be better than three. On the other hand during RFC some people were quite adamant about marking parcel_pickup as not required at treating it as assumed to be "yes". With simple checkboxes it could lead to people adding these fields when not necessary. I don't know what's better... I'll try to merge these into single preset in the next commit.

@tyrasd tyrasd dismissed their stale review January 7, 2022 19:21

requested change was performed

@tyrasd
Copy link
Member

tyrasd commented Jan 7, 2022

Thanks for the update! But I still see one (or two) additional potential issue(s) here: If a mapper only knows that a parcel locker exists at a given location, but doesn't know whether is also accepts dropping off of parcels or not, they would be forced to choose one of the three presets. The correct one to choose would be "Parcel Pickup Locker", but that's not very intuitive. The second (somewhat related) issue is that the current "Parcel Pickup Locker" preset doesn't allow to explicitly specify that the dropping off of parcels is actually not possible: since parcel_mail_in is not set, it could be the case that it's value was just unknown when the feature was mapped.

With simple checkboxes it could lead to people adding these fields when not necessary.

I think this is not really the case when the description of the "default" case is well chosen. For example, the oneway field displays "assumed to be no" for the unset case, and I think most mappers actually don't explicitly set the checkbox to no on regular non-oneway streets. This should work here just fine as well.

This way tagging it would require more clicks

I think two clicks is just fine for such a feature. I think we should go for it.

@ttomasz
Copy link
Contributor Author

ttomasz commented Jan 9, 2022

Didn't know that there were such options 😅 I updated the preset, let me know if this is correct or needs some fixes :)

PS. The wiki page for amenity=parcel_locker has been created: https://wiki.openstreetmap.org/wiki/Tag:amenity%3Dparcel_locker

reason: the wiki/poposal doesn't mention the tag explicitly, and most current uses just seem to contain or repeat ref numbers, brand names, or similar information.
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