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

stop rendering shop=yes [ready for review] #3718

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

matkoniecz
Copy link
Contributor

@matkoniecz matkoniecz commented Mar 15, 2019

fixes #3697

Originally only shop with their own icons were rendered.

Later popular shop values were also rendered - as dots without icons. This was deliberate decision to not render cases like shop=supermarkte

Since #2415 all shop values except 'no', 'vacant', 'closed', 'disused', 'empty' are rendered.

Given that any, even never used shop value is rendered and shop=yes is unreasonably popular it is desirable to encourage more specific shop tagging.

Dropping shop=yes may encourage using some new shop values and in some cases people will mistakenly create new tags duplicating existing ones,
but standard shop types are still encouraged by using icons.

shop tag currently has long tail of unusual shop types (with most tagged correctly - for example shop=maps, some shop like this really exist),
so data consumers need anyway some strategy for handling rare shop types.

shop=yes is currenly reported as a tagging mistake by decent validators (for example JOSM and Osmose).

Note that this is not deprecation of a tag but stopping to render a deprecated tag.
This tag was never considered as a sufficient for tagging shop type

Test rendering with links to the example places:

https://www.openstreetmap.org/way/237537359 and other now fixed objects, found using http://overpass-turbo.eu/s/H0Q

Before (this place has both node and area with shop=yes)
Selection_002

After
Selection_003

@matkoniecz
Copy link
Contributor Author

Note that all and each shop value except blacklisted 'no', 'vacant', 'closed', 'disused', 'empty', 'yes' will continue to be rendered in this style.

Dropping rendering of shop=yes will stop rendering of about 140-150k objects (note that some shop=yes are not rendered due to their presence on amenity=fuel).

@imagico
Copy link
Collaborator

imagico commented Mar 25, 2019

I think with #2415 taken as given this change makes sense but i would be in favor of revisiting the decision of #2415 to render a catchall for shops.

We currently have 105k different values for shop=* but only ~850 with more than ten uses - which means that there are nearly as many special choice values for shop=* out there as there are cases of shop=yes and many of them are just typos or pointless synonyms for existing established values.

I am kind of inclined to suggest rendering neither shop=yes nor any special choice values - but that would require creating and maintaining a fairly long whitelist of values. While this could be scripted based on taginfo data this is still a maintenance problem.

@matkoniecz
Copy link
Contributor Author

matkoniecz commented Mar 25, 2019

Note that script already existed - see https://github.com/gravitystorm/openstreetmap-carto/blob/c17cc64cbf37b96710439b1ca6c64ca73e514883/scripts/shop_values.rb (at that time it whitelisted everything above 100 usages except some explicitly blacklisted entries).

I would also support revisiting this decision (AKA reverting #2415) in addition to stopping rendering shop=yes combined with lowering minimal count to 50 (or maybe some lower value?) - but to do in addition and after dropping shop=yes rendering.

@imagico
Copy link
Collaborator

imagico commented Mar 25, 2019

My idea was more of a combined rule - including values that are not on blacklist and either

  • sufficient use numbers to indicate it is not just a typo or unqualified choice
  • a tag page on the wiki
  • support for the specific value in any taginfo registered project

But the question is not really how the rules should look like exactly but if we want to have a dynamic list of values that needs to be updated on a regular basis.

I see this also in relation to #3635 - if a generic shop dot ceases to indicate any shop value and starts indicating a meaningful shop value that would probably somewhat reduce the incentive to add special individual symbols for all kind of shops to separate them from all the meaningless dots.

@matkoniecz
Copy link
Contributor Author

matkoniecz commented Mar 25, 2019

a tag page on the wiki

Not something that should be blindly used. I created many tag pages on wiki to document specific tag as a horrible mistake.

But a good candidate for reviewing page for potential inclusion.

@matkoniecz
Copy link
Contributor Author

Though note that potential catch-all removal is a separate project and it is unnecessary to discuss it here.

@kocio-pl
Copy link
Collaborator

I don't believe such list makes sense if there is no person dedicated to "gardening" this particular problem (not a big task, but needed to be done in a regular fashion for a long time). We had a short list of shops once, when there was much more activity in this project, and even then it was abandoned and I'm afraid we would get back to this fiction.

At this moment I support removing shop=yes only, so we take no additional obligation and people still have clearly defined and quite large task (~150.000 uses) to take care of.

@matkoniecz
Copy link
Contributor Author

Yes, certainly dropping shop=yes is not implying dropping also catch-all that would be a separate discussion.

BTW, I looked at old script that resulted in some updates (of script, OSM Wiki and mailing list). If someone is interested in shop values it is a good idea to look at them.

@imagico
Copy link
Collaborator

imagico commented Mar 25, 2019

Note since #2415 the same logic as used for shop=* was also added for office=*. While it is certainly not necessary to look at all of this (catch-alls and yes-values, shop and office) at once it could help deliberating on these things and trying to find consensus together.

@matkoniecz
Copy link
Contributor Author

I opened #3718 (certainly not ready, but posted as PR as I made some updates to an old script).

@kocio-pl
Copy link
Collaborator

Another similar tag to look at is building=* (I mean especially building=yes). Once in a while we can check if something can be changed in rendering them without causing too much stir in a bowl.

@matkoniecz matkoniecz changed the title stop rendering shop=yes stop rendering shop=yes [requires rewrite] Apr 8, 2019
fixes gravitystorm#3697

Originally only shop with their own icons were rendered.

Later popular shop values were also rendered - as dots without icons. This was deliberate decision to not render tags with mistakes like shop=supermarkte

Since gravitystorm#2415 all shop values except 'no', 'vacant', 'closed', 'disused', 'empty' are rendered.

Given that any, even never used shop value is rendered and `shop=yes` is unreasonably popular it is desirable to encourage more specific shop tagging.

Dropping `shop=yes` may encourage using some new shop values and in some cases people will mistakenly create new tags duplicating existing ones,
but standard shop types are still encouraged by using icons.

shop tag currently has long tail of unusual shop types (with most tagged correctly - for example `shop=maps`, some shop like this really exist),
so data consumers need anyway some strategy for handling rare shop types.

`shop=yes` is currenly reported as a tagging mistake by decent validators (for example JOSM and Osmose).

Note that this is not deprecation of a tag but stopping to render a deprecated tag.
This tag was never considered as a sufficient for tagging shop type
@matkoniecz matkoniecz changed the title stop rendering shop=yes [requires rewrite] stop rendering shop=yes [requires retesting] May 11, 2019
Copy link
Collaborator

@imagico imagico left a comment

Choose a reason for hiding this comment

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

Works fine.

@imagico imagico changed the title stop rendering shop=yes [requires retesting] stop rendering shop=yes May 21, 2019
@matkoniecz matkoniecz changed the title stop rendering shop=yes stop rendering shop=yes [ready for review] Jun 20, 2019
@imagico imagico merged commit 952d13d into gravitystorm:master Aug 22, 2019
@matkoniecz matkoniecz deleted the stop_shop=yes branch August 22, 2019 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stop rendering shop=yes
4 participants