[WIP] Add healthcare to list of polygons in lua transformations

[jeisenbe]

Fixes #3639

Changes proposed in this pull request:

• Add 'healthcare' to list of polygon objects in openstreetmap-carto.lua

Explanation:

• This will allow closed ways ("areas") that are tagged with "healthcare=*" to be imported as polygons, so that they can be rendered properly
• Currently imported objects will not be immediately affected, but newly added and edited objects should render properly now, and all should render properly after database reload.

Test rendering:

Before

After

• Rendered after reloading database. Before database reload the rendering was unchanged.
  

[jeisenbe]

I see that 'amenity_hospital' is rendered with a polygon fill in societal_amenities color, but healthcare_hospital was not added to landcover.mss

Was this intentional, or an oversight in the PR that added support for healthcare= ?

[jeisenbe]

Hmm... adding healthcare_hospital and healthcare_clinic to landcover.mss does not work to render polygon fill and outlines for these features, even though there are already two lines in project.mml.

This is what I tried:

+++ b/landcover.mss
@@ -570,7 +570,9 @@
   }
 
   [feature = 'amenity_hospital'],
+  [feature = 'healthcare_hospital'],
   [feature = 'amenity_clinic'],
+  [feature = 'healthcare_clinic'],
   [feature = 'amenity_university'],

And in project.mml the landcover layer has (lines 124 and 150)

              ('healthcare_' || (CASE WHEN tags->'healthcare' IN ('clinic' ,'hospital') THEN tags->'healthcare' ELSE NULL END)) AS healthcare,

(FROM planet_osm_polygon)
WHERE ...

             OR tags->'healthcare' IN ('clinic', 'hospital')

I've also added 'healthcare' to openstreetmap-carto.lua already, as in the first commit in this PR.

Why are these features still not rendering with this code?

Is this due to "healthcare" being in the hstore column rather than in a standard column?

[imagico]

I am not sure if you want to

• initiate a database reload on the OSMF servers
• roll out this change without a database reload (leading to inconsistencies in rendering between pre-existing and newly added or modified features)
• implement this change in OSM-Carto without rolling it out on OSMF servers (leading to rendering differences between osm.org and test environments)

I would be against the second and third options for the mentioned reasons.

[kocio-pl]

Is this due to "healthcare" being in the hstore column rather than in a standard column?

Yes. Adding column in lua file means you have to change all the hstore references in SQL to the standard column references.

[matthijsmelissen]

I agree we cannot merge this now because there is no database coming up, and I wouldn't like our code to divert from the code on production. Let's add it to #3611 and take it into consideration during the next reload. For now I'm going to reject this PR.

[jeisenbe]

The problem is, this feature is already rendered, but only on nodes or on polygons tagged with area=yes, so the current situation is also not good for tagging feedback. Unless we make the more extreme choice of reverting the PR that added healthcare rendering, this would be better than the other options. For mapper feedback, new and edited features are most important. So it may nit nene a large problem if old polygons only tagged with “healthcare” are not yet rendered for a few months.

…

On Wed, Jan 16, 2019 at 7:57 PM Christoph Hormann ***@***.***> wrote: I am not sure if you want to - initiate a database reload on the OSMF servers - roll out this change without a database reload (leading to inconsistencies in rendering between pre-existing and newly added or modified features) - implement this change in OSM-Carto without rolling it out on OSMF servers (leading to rendering differences between osm.org and test environments) I would be against the second and third options for the mentioned reasons. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3644 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AoxshL6PcCdGSJXKA-0SQKTGnnuFT1zJks5vDwWOgaJpZM4aCU0N> .

[jeisenbe]

Would you like to stop rendering healthcare on all polygons, or entirely, for the time being? On Wed, Jan 16, 2019 at 9:56 PM Joseph Eisenberg <joseph.eisenberg@gmail.com> wrote:

…

The problem is, this feature is already rendered, but only on nodes or on polygons tagged with area=yes, so the current situation is also not good for tagging feedback. Unless we make the more extreme choice of reverting the PR that added healthcare rendering, this would be better than the other options. For mapper feedback, new and edited features are most important. So it may nit nene a large problem if old polygons only tagged with “healthcare” are not yet rendered for a few months. On Wed, Jan 16, 2019 at 7:57 PM Christoph Hormann < ***@***.***> wrote: > I am not sure if you want to > > - initiate a database reload on the OSMF servers > - roll out this change without a database reload (leading to > inconsistencies in rendering between pre-existing and newly added or > modified features) > - implement this change in OSM-Carto without rolling it out on OSMF > servers (leading to rendering differences between osm.org and test > environments) > > I would be against the second and third options for the mentioned reasons. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#3644 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AoxshL6PcCdGSJXKA-0SQKTGnnuFT1zJks5vDwWOgaJpZM4aCU0N> > . >

[matthijsmelissen]

What do you think is the best way forward, @jeisenbe?

[kocio-pl]

I would prefer to talk with OWG first and ask when they would be ready to reimport database. If we know it, we can plan our action, hopefully changing all the current hstore objects into plain columns.

[UPDATE] Question asked: openstreetmap/chef#211

[jeisenbe]

Sorry, my first response was sent before last couple of hours of emails loaded, so I didn’t realize the PR was rejected. @imagico, do you think this feature should be removed until the next database reload? Or would it be acceptable to only render “healthcare=“ tags on nodes for now?

…

On Wed, Jan 16, 2019 at 10:00 PM kocio-pl ***@***.***> wrote: I would prefer to talk with OWG first and ask when they would be ready to reimport database. If we know it, we can plan our action, hopefully changing all the current hstore objects into plain columns. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3644 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AoxshHbR0pUQeVWk6-abtFejyU0C2cQ6ks5vDyJ8gaJpZM4aCU0N> .

[imagico]

I have not reviewed #3498 so i cannot give you a full answer to the pros and cons to revert it (i see other problems with it as well) but yes, a change that would depend on this PR to be consistently rendered would IMO need to wait for a database reload.

[mboeringa]

Do we actually need this?

It seems that e.g. iD already automatically double tags with additional "healthcare" tag for the most important amenities:

Also see this:
openstreetmap/iD@bfda2c5

In addition, healthcare tagging is a bit of a mess, with two competing schemes:
https://wiki.openstreetmap.org/wiki/Key:healthcare
https://wiki.openstreetmap.org/wiki/Proposed_features/Healthcare_2.0

While the first one seems used in iD, the second seems at least partly used by HOT:
https://github.com/hotosm/presets/blob/master/HDM.xml

health_facility:type is a key from the "officially" "abandoned" Healthcare 2.0 proposal...

This essentially means you would need to select:
amenity IN ('hospital') OR healthcare IN ('hospital') OR health_facility:type IN ('hospital')
to be reasonably sure to have most hospital facilities selected.

By introducing the secondary "healthcare" tag in rendering, there is a slight risk people will no longer double tag with amenity=, meaning all styles would need to start employ the above query if they want to catch all facilities of a certain type, although looking at the HOT preset, it also seems to promote or use double tagging with at least amenity=, so that this issue may not be to big in reality.