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

Render power plants area #3405

Closed
dktue opened this issue Sep 20, 2018 · 22 comments · Fixed by #4088
Closed

Render power plants area #3405

dktue opened this issue Sep 20, 2018 · 22 comments · Fixed by #4088

Comments

@dktue
Copy link

dktue commented Sep 20, 2018

Example where rendering the area would be helpful for power=plant
https://www.openstreetmap.org/way/164454445
image

@kocio-pl
Copy link
Collaborator

Hm, 42% of them have landuse=industrial included:

https://taginfo.openstreetmap.org/tags/power=plant#combinations

According to wiki using it is "recommended if not already inside a bigger landuse=industrial, else optional":

https://wiki.openstreetmap.org/wiki/Tag%3Apower%3Dplant

@kocio-pl kocio-pl changed the title Render power plants Render power plants area Sep 20, 2018
@kocio-pl
Copy link
Collaborator

kocio-pl commented Sep 20, 2018

I guess if landuse for this exact object is optional, then we don't have to expect people will add it (no need to act as a quality assurance), so we can safely add such rendering - even safer than with #3376, which was just a reality check.

Would you like to try?

@matkoniecz
Copy link
Contributor

I guess if landuse for this exact object is optional

Why? I think in this case landuse is missing so not rendering landuse is OK.

From what I remember that was reasoning behind showing just name - so power plants with larger landuse=industrial will have their names rendered, and power plants not within landuse=industrial will be visible as data issue.

@kocio-pl
Copy link
Collaborator

But there's no data issue - it is explicitly allowed as a valid tagging on wiki.

@dktue
Copy link
Author

dktue commented Sep 20, 2018

@kocio-pl That's the point: Either we declare it a data issue and correct it in the wiki or we render it implicitly.

@kocio-pl
Copy link
Collaborator

You can discuss it on a Tagging list to make sure.

@Adamant36
Copy link
Contributor

It seems weird and inconsistent to have substations rendered as areas but not power plants. I would expect them to be extremely similar in mapping philosophy and only a small amount of them are tagged with landuse. So its clearly not required in that case. I don't see why it wouldn't transfer over to power plants.

@matkoniecz
Copy link
Contributor

matkoniecz commented Sep 21, 2018

But there's no data issue - it is explicitly allowed as a valid tagging on wiki.

Are you sure? My reading of

recommended if not already inside a bigger landuse=industrial, else optional

is that

  • inside larger landuse=industrial adding landuse=industrial is optional (our current rendering is OK in this case)
  • if not inside landuse=industrial then landuse=industrial is recommended (our current behavior of not rendering area and encouraging to add landuse=industrial is also OK in this case)

@kocio-pl
Copy link
Collaborator

Yes, I see no data issue here, because "recommended" is not "required".

@matkoniecz
Copy link
Contributor

Wiki typically describes things this way, to avoid confusion in rare examples. There may be a rare example of power plant that is not in industrial area (because it is left operating as a tourism attraction or is a military installation).

Also, many people complain when wiki dictates tagging rather than recommends.

I will change wiki a bit to clarify (if someone disagress - please start tagging discussion on tagging mailing list or Wiki).

@kocio-pl
Copy link
Collaborator

Thanks Mateusz! But that means that current rendering is proper and the ticket can be closed.

@matkoniecz
Copy link
Contributor

I am closing, but feel free to start tagging dicussion if someone thinks that wiki is wrong or that encouraging to add landuse=industrial is a wrong idea.

@dieterdreist
Copy link

dieterdreist commented Sep 21, 2018

@matkoniecz I don’t understand why it was closed, the discussion about landuse has nothing to do with rendering this feature for areas.

Would we remove the rendering of area features from military airports because there will be landuse=military?

@HolgerJeromin
Copy link
Contributor

Just as a reference: #1836 was the PR with the decision to only render a name and not a fill or outline to suggest adding a landuse=industrial.

@kocio-pl
Copy link
Collaborator

Thanks for providing link.

I would expect that power=plant would be anyway tagged as landuse=industrial.
Rendering area (like landuse=industrial) hides that some data is missing

That was the same assumption that it's incomplete without landuse, but was not backed by Wiki definition, so this time we're more scrupulous.

@matkoniecz
Copy link
Contributor

Would we remove the rendering of area features from military airports because there will be landuse=military?

No, but military airports (tagged with for example military=airfield) should not be rendered as landuse=military without that landuse specified.

@dieterdreist
Copy link

dieterdreist commented Sep 22, 2018 via email

@matkoniecz
Copy link
Contributor

Because it may make less obvious that land use tagging is missing.

@Tomasz-W
Copy link

If smaller power=substation areas are filled, why bigger power=plant areas aren't in any way?

@Adamant36
Copy link
Contributor

@Tomasz-W, I brought that up also. It doesn't make any sense to render substations but not power plants. There's no good reason not to render them either.

@jeisenbe
Copy link
Collaborator

Reopened: it doesn't make sense to render power=substation areas (and currently power=station, a synonym of power=plant) but not power=plant.

Also, we should not render the text label based on the size of the area without any representation of the area. (Larger power=plant areas get larger labels, at earlier zoom levels). This is fixed by rendering the area.

@jeisenbe jeisenbe reopened this Mar 21, 2020
@Adamant36
Copy link
Contributor

Slightly off topic, but rendering for power=station should still be removed IMO. Rendering power=plant would be great though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants