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.

Sign up for GitHub

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

Jump to bottom

tourism=attraction not rendered when object also has unrendered amenity tag #1029

Closed
1ec5 opened this issue Oct 8, 2014 · 9 comments · Fixed by #1349
Closed

tourism=attraction not rendered when object also has unrendered amenity tag #1029

1ec5 opened this issue Oct 8, 2014 · 9 comments · Fixed by #1349
Labels
amenity-points
Milestone
Bugs and improvem...

Comments

@1ec5
Copy link

1ec5 commented Oct 8, 2014

This area is tagged amenity=casino and tourism=attraction. Normally tourism=attraction gets a red label (example), but amenity=casino seems to override that rule, removing the label at all zoom levels. (The fuschia-ish fill color remains.) A label should appear based on the area’s area (like #941).
@matkoniecz matkoniecz mentioned this issue Oct 8, 2014
Closed
@matthijsmelissen matthijsmelissen changed the title amenity=casino area should be labeled tourism=attraction not rendered when object also has unrendered amenity tag Oct 8, 2014
@matthijsmelissen
Copy link
Collaborator

I made the title more general so it also encompasses #1030. The cause is the same, so the solution will be the same as well.

@matthijsmelissen matthijsmelissen added this to the Bugs and improvements milestone Oct 9, 2014
matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this issue Oct 10, 2014
@matthijsmelissen
Rewrite SQL queries so the COALESCE part works correctly
5dd84e9 
* Resolves gravitystorm#985
* Resolves gravitystorm#1029
@matthijsmelissen matthijsmelissen mentioned this issue Oct 10, 2014
Closed
@1ec5
Copy link
Author

1ec5 commented Jan 2, 2015

714aff3 makes this worse because tourism=attraction simply isn’t rendered at all when the area has an amenity tag.

@matthijsmelissen
Copy link
Collaborator

Yes, this really needs a solution. It's not very clear how, though.

@pnorman
Copy link
Collaborator

pnorman commented Jan 2, 2015

Is this another one of those cases arising out of combinatorial explosion (mapbox/carto#20) workarounds?

@matthijsmelissen
Copy link
Collaborator

Yes it is.

@malenki
Copy link

malenki commented Jan 3, 2015

IMHO: when there are better explaining tags than tourism=attraction on the object – like in this case amenity=casino – the tourism=attraction should not be rendered.
If you want to render casinos: do so. But don't encourage people to stick the generic tourism=attraction on more objects then needed.
After all I don't think the OPs example is a good one since the casino tag explains that it is a casino. For my part I wouldn't consider a casino an attraction for tourists.

@1ec5
Copy link
Author

1ec5 commented Jan 3, 2015

For my part I wouldn't consider a casino an attraction for tourists.

It depends on the case. In the case I linked to above, it is considered a regional tourist attraction. Other casinos may not be. In the same vein, a landmark fountain may be worth tourism=attraction for various reasons, whereas the one across the street may not be. But perhaps I’m misunderstanding tourism=attraction? It seems open to interpretation.

@matkoniecz
Copy link
Contributor

I noticed other cases of missing tourism=attraction labels:

https://www.openstreetmap.org/way/321318572
https://www.openstreetmap.org/node/2500686532

In the first case it is blocked by unrendered natural, in the second case I am not sure what is happening.

as problem is caused by

COALESCE(aeroway, amenity, landuse, leisure, military, "natural", power, tourism, highway, railway) AS feature

unrendered aeroway, amenity, landuse, leisure, military, natural or power will also cause this problem.

@matkoniecz matkoniecz mentioned this issue Jan 10, 2015
Merged
@davidgumberg
Copy link

Same issue with having for example amenity=pharmacy and shop= any unrendered shop type.

matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this issue Mar 4, 2015
@matthijsmelissen
Resolve bugs related to COALESCE problems
aa53e41 
* Make sure conditions are checked in both WHERE and COALESCE
  Checking the condition in the WHERE class is necessary for performance.
  Checking the condition in the COALESCE is necessary to prevent unrendered tags
  blocking rendering of tags further down in the COALESCE.
  See also the discussion [here](gravitystorm#1038 (comment)).

  This is an improvement of gravitystorm#1038.

  This resolves gravitystorm#985, resolves gravitystorm#1029, resolves gravitystorm#1336.

* Give amenity priority over shop.

  This resolves gravitystorm#963.

* Give tourism priority over amenity, shop, leisure, landuse, man_made, natural,
  and place.

  This resolves gravitystorm#1269, resolves first issue of gravitystorm#1232.
@matthijsmelissen matthijsmelissen mentioned this issue Mar 4, 2015
Merged
matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this issue Mar 4, 2015
@matthijsmelissen
Resolve bugs related to COALESCE problems
4a3dfaf 
* Make sure conditions are checked in both WHERE and COALESCE
  Checking the condition in the WHERE class is necessary for performance.
  Checking the condition in the COALESCE is necessary to prevent unrendered tags
  blocking rendering of tags further down in the COALESCE.
  See also the discussion [here](gravitystorm#1038 (comment)).

  This is an improvement of gravitystorm#1038.

  This resolves gravitystorm#985, resolves gravitystorm#1029, resolves gravitystorm#1336.

* Give amenity priority over shop.

  This resolves gravitystorm#963.

* Give tourism priority over amenity, shop, leisure, landuse, man_made, natural,
  and place.

  This resolves gravitystorm#1269, resolves first issue of gravitystorm#1232.
matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this issue Mar 4, 2015
@matthijsmelissen
Resolve bugs related to COALESCE problems
4933cdb 
* Make sure conditions are checked in both WHERE and COALESCE
  Checking the condition in the WHERE class is necessary for performance.
  Checking the condition in the COALESCE is necessary to prevent unrendered tags
  blocking rendering of tags further down in the COALESCE.
  See also the discussion [here](gravitystorm#1038 (comment)).

  This is an improvement of gravitystorm#1038.

  This resolves gravitystorm#985, resolves gravitystorm#1029, resolves gravitystorm#1336.

* Give amenity priority over shop.

  This resolves gravitystorm#963.

* Give tourism priority over amenity, shop, leisure, landuse, man_made, natural,
  and place.

  This resolves gravitystorm#1269, resolves first issue of gravitystorm#1232.
matthijsmelissen added a commit to matthijsmelissen/openstreetmap-carto that referenced this issue Mar 9, 2015
@matthijsmelissen
Resolve bugs related to COALESCE problems
f93b782 
* Make sure conditions are checked in both WHERE and COALESCE
  Checking the condition in the WHERE class is necessary for performance.
  Checking the condition in the COALESCE is necessary to prevent unrendered tags
  blocking rendering of tags further down in the COALESCE.
  See also the discussion [here](gravitystorm#1038 (comment)).

  This is an improvement of gravitystorm#1038.

  This resolves gravitystorm#985, resolves gravitystorm#1029, resolves gravitystorm#1336.

* Give amenity priority over shop.

  This resolves gravitystorm#963.

* Give tourism priority over amenity, shop, leisure, landuse, man_made, natural,
  and place.

  This resolves gravitystorm#1269, resolves first issue of gravitystorm#1232.
sommerluk pushed a commit to sommerluk/openstreetmap-carto that referenced this issue Mar 16, 2015
@matthijsmelissen @sommerluk
Resolve bugs related to COALESCE problems
fa1128f 
* Make sure conditions are checked in both WHERE and COALESCE
  Checking the condition in the WHERE class is necessary for performance.
  Checking the condition in the COALESCE is necessary to prevent unrendered tags
  blocking rendering of tags further down in the COALESCE.
  See also the discussion [here](gravitystorm#1038 (comment)).

  This is an improvement of gravitystorm#1038.

  This resolves gravitystorm#985, resolves gravitystorm#1029, resolves gravitystorm#1336.

* Give amenity priority over shop.

  This resolves gravitystorm#963.

* Give tourism priority over amenity, shop, leisure, landuse, man_made, natural,
  and place.

  This resolves gravitystorm#1269, resolves first issue of gravitystorm#1232.
@jeisenbe jeisenbe mentioned this issue Dec 4, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
amenity-points
Projects
None yet
Milestone
Bugs and improvements
6 participants
@malenki @matkoniecz @pnorman @1ec5 @davidgumberg @matthijsmelissen