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

Rendering rare shop values #2415

Merged
merged 1 commit into from
Oct 31, 2016
Merged

Rendering rare shop values #2415

merged 1 commit into from
Oct 31, 2016

Conversation

kocio-pl
Copy link
Collaborator

@kocio-pl kocio-pl commented Oct 19, 2016

Resolves #2099.

All the white lists for other shops:

WHEN shop IN ( ... ) THEN 'other' ELSE NULL END,

have been replaced with just basic checking if it's really a shop:

WHEN shop = 'no' OR shop = 'disused' OR shop IS NULL THEN NULL ELSE 'other' END,

I'm not familiar with SQL, but testing shows that it works. Does it make any sense to check also if shop is not null to exclude all the shops having their own icons?

@pnorman
Copy link
Collaborator

pnorman commented Oct 19, 2016

The decision to render specific shops was a conscious decision, we wanted to avoid rendering mistakes like shop=supermarkte. We can revisit it, but #2099 is about shop=yes, not about all shop values that don't have an individual rendering.

At the time we made the decision I would have preferred rendering shop is not null and shop not in ('vacant',...) and not specially rendered as a dot. Something which wasn't originally considered is the maintenance burden of maintaining two lists of shops, one with special renderings, another with a generic rendering.

@kocio-pl
Copy link
Collaborator Author

#2099 has proposed solution in the title, but the problem description:

Rendering shop=yes and not rendering rare shop values encourages to omit detail during tagging

shows that the title is not precise, because not showing rare shops is the real source and shop=yes is just a common workaround - a valid general value which is probably misused. BTW - the misuse is marginal comparing to building=yes (3,44% vs 82.12% of total uses with given key).

Therefore I think we could start with fixing the source and wait some time to see if any other action is needed, as proposed here. I like this solution, that's why I didn't exclude shop=yes. Is it OK for you or you want to decide also this question now?

@kocio-pl
Copy link
Collaborator Author

Thanks for bringing up the vacant value! I was sure there is something like this, but took disused instead by mistake (it's good to filter it out too)... Now the line is:

WHEN shop IN ('no', 'vacant', 'disused') OR shop IS NULL THEN NULL ELSE 'other' END,

@kocio-pl
Copy link
Collaborator Author

I've added some more shop values indicating that shop is non-existing - I took them from shop_values.rb and removed this script.

@pnorman
Copy link
Collaborator

pnorman commented Oct 31, 2016

I'm going to go ahead and merge this. There are arguments either way on if it's better to have a whitelist of minor shop values from a tagging and cartographic viewpoint.

What settles it for me is the maintenance issues. We haven't been maintaining the lists, the same large list is supposed to be identical in four places, and the queries shops are part of are already some of larger ones in the style.

@pnorman pnorman merged commit 3e35540 into gravitystorm:master Oct 31, 2016
@kocio-pl kocio-pl deleted the shops_all branch October 31, 2016 02:54
@gravitystorm
Copy link
Owner

For the record, I disagree with this approach. It was a major improvement to avoid catch-all rendering, which this PR re-enables. I understand the lack of maintenance on the shops list but it would be better to maintain the list rather than re-introduce a catch-all.

The proliferation of shop values is outside the scope of the repo, but the trouble we are having here highlights that this is in itself a problem that I would prefer tackled too. This would most likely be by introducing categories and subcategories of shops, rather than having hundreds of artificially discreet values.

I'm not suggesting that this to be reverted, I just want to make my point here.

@kocio-pl
Copy link
Collaborator Author

The point is we were not able to maintain this list and I haven't find a better way. But if somebody find the right tool to control it (here or on the data level), I would be fine with it.

Categories may be a good thing, not only for shops, but also for other objects. I even tried to introduce it once, but nobody followed and I didn't know how the implementation should look like. Do you have any ideas about it?

@aceman444
Copy link

You still have a whitelist of shop types that have a specific icon. All others should get just the dot. In that way, you still see typos, like the mentioned 'supermarkte', because it will only get a dot, instead of the mapper's expected icon. But it is still a shop despite the typo, so it should appear on the map.

@HolgerJeromin
Copy link
Contributor

@aceman444
But you will not find the shop=beds typo (or unusual/unwanted tagging) which would be shop=bed.

@dieterdreist
Copy link

sent from a phone

Il giorno 16 nov 2016, alle ore 14:51, Andy Allan [email protected] ha scritto:

The proliferation of shop values is outside the scope of the repo, but the trouble we are having here highlights that this is in itself a problem that I would prefer tackled too. This would most likely be by introducing categories and subcategories of shops, rather than having hundreds of artificially discreet values.

in the end you won't end up with less tags for distinct shop types, it would just be easier to find them - maybe. This is also something the editors can provide without having it hardcoded into the tags, josm and others are already doing it.

+1 to giving the shops list more care. I suggest to give more importance to an existing wiki definition rather than just usage numbers, and lower the threshold to a few hundred.

@nebulon42
Copy link
Contributor

I'm not too fond of this change too. We are not the map of shops, but it sometimes seems we are. They have gotten far too much attention and we should render much less than more.

@kocio-pl
Copy link
Collaborator Author

I've created a ticket to sort the things out, because the problem is not easy to define (let alone to solve). The #2444 is a better place to discuss it than a PR closed a month ago.

@aceman444
Copy link

@HolgerJeromin why not? If one is in the whitelist with specific icon, you will see if it didn't get it. If none has a specific icon, it means the value is not important yet (few usages worldwide), so then it is not even sure which value is the correct one.

@HolgerJeromin
Copy link
Contributor

My example was clear: beds 2700 vs bed 1.

@aceman444
Copy link

So does 'beds' have a special icon?

@HolgerJeromin
Copy link
Contributor

No and thats the whole point

@aceman444
Copy link

So in that case, how did you determine 'beds' is the proper value and not 'bed'? Once it is important enough and gets a special icon, the typos may show up (with dot instead of the expected icon).
Or was it important enough till now to be in the whitelist to get the dot, but not an icon?

@HolgerJeromin
Copy link
Contributor

Ok, take shop=baby_goods (which was in the dot white liste, is approved by the wiki and has 2090 elements) and shop=baby_good (which has no wiki page and is not used in the database).

One propose of this style is "feedback mechanism for mappers to validate their edits". I think most people does not call shop=baby_good a valid or at least useful/wanted entry in our database.

@aceman444
Copy link

So was baby_goods only with dot and no icon?

I think the feedback can then be solved with having 2 lists:

  1. list with shop values that have each its own special icon
  2. list of other accepted values (e.g. >1000 uses and a wiki page). These may be rendered by a common shop icon.
  3. all the other unknown shop values will be rendered with a dot.

matkoniecz added a commit to matkoniecz/openstreetmap-carto that referenced this pull request Mar 15, 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 cases 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 deisrable to encourage more specific shop tagging.

It 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 , some shop like this really exist),
so data consumers need anyway some startegy for handling rare shop types.

 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 added a commit to matkoniecz/openstreetmap-carto that referenced this pull request Mar 15, 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 cases 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 added a commit to matkoniecz/openstreetmap-carto that referenced this pull request May 11, 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
imagico pushed a commit that referenced this pull request Aug 22, 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 tags with mistakes 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
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.

7 participants