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

Add rendering for power=plant #1836

Merged
merged 2 commits into from
Sep 16, 2015
Merged

Conversation

kreed
Copy link
Contributor

@kreed kreed commented Sep 15, 2015

This will render names for power plants.

Closes #1835, supersedes #1771

@matkoniecz
Copy link
Contributor

I would expect that power=plant would be anyway tagged as landuse=industrial. Are there cases of correctly tagged places where rendering power=plant like landuse=industrial would improve situation?

Also, #1835 mentions that power=station should not be rendered, maybe it should be removed from rendering?

@HolgerJeromin
Copy link
Contributor

right now we have about four times more stations that plants. So perhaps it is too early to drop station rendering. But as this tagging has no documentation this could be a valid thing.

But you are right, most power=plant should have landuse=industral or for small things building tag. So perhaps drop power=plant and station for promoting landuse/building.

@kreed
Copy link
Contributor Author

kreed commented Sep 15, 2015

I would expect that power=plant would be anyway tagged as landuse=industrial. Are there cases of correctly tagged places where rendering power=plant like landuse=industrial would improve situation?

It would be helpful in cases where the power plants are located inside of larger industrial areas. Adding a landuse=industrial tag to such plants would be redundant.

About one-third of all power plants are currently tagged with landuse=industrial (I'm analyzing the extract from here, since the tag isn't common enough for taginfo statistics).

power=# SELECT count(*) FROM planet_osm_polygon WHERE power='plant';                                                                                                                
 count 
-------
  2592

power=# SELECT count(*) FROM planet_osm_polygon WHERE power='plant' AND landuse='industrial';                                                                                       
 count 
-------
   867

Also, #1835 mentions that power=station should not be rendered, maybe it should be removed from rendering?

Yes, power=station is problematic because of ambiguous tagging. Both power plants and power substations are tagged with it. I would prefer to deal with it in a separate PR though.

@HolgerJeromin
Copy link
Contributor

thanks i did not thought about a plant within an industrial complex.

i just fixed a few substations which were tagged as power=plant (while searching power=plant and landuse!=industrial with overpass). 8-/
I cannot think of a rendering (aka an icon which is plant, but no substation) to prevent this.

@matkoniecz
Copy link
Contributor

plant within an industrial complex

In that case rendering only name would be better. My test found many mistagged substations and individual power plants without tagged landuse=industrial.

Rendering area (like landuse=industrial) hides that some data is missing - for example see https://cloud.githubusercontent.com/assets/899988/9902546/92f733ee-5c6f-11e5-8e17-32d9056488b6.png

@kreed
Copy link
Contributor Author

kreed commented Sep 16, 2015

Good point. That sounds like a good solution. I'll update the code.

@kreed
Copy link
Contributor Author

kreed commented Sep 16, 2015

Updated. I also reordered the coalesce statement so that the power color is used over the industrial color when both tags are present.

matkoniecz added a commit that referenced this pull request Sep 16, 2015
Add rendering for power=plant
@matkoniecz matkoniecz merged commit 3fcb935 into gravitystorm:master Sep 16, 2015
@matkoniecz
Copy link
Contributor

If you are interested in making more pull requests - #887 is very similar.

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.

3 participants