Moving ford to a new tagging scheme #2743

kocio-pl · 2017-08-13T06:07:36Z

Resolves #267.

Simply removing support for deprecated tagging scheme and adding support for the new one (much more popular already).

I need some help, since it works for points, but not for ways. Another thing is deciding on rendering names - 3% have them, but I suspect most of them are the names of the highways.

matthijsmelissen · 2017-08-20T22:04:48Z

Related to #484.

I think we would need to add an amenity_line layer to accomplish this.

kocio-pl · 2017-08-30T22:38:39Z

Looks like amenity-line layer works, since data inspector in Kosmtik shows feature highway_ford for a way tagged with ford=*, but it still doesn't show the icon. Maybe something should be changed in amenity-points.mss?

matthijsmelissen · 2017-08-30T22:42:38Z

Did you add a rendering rule for #amenity-line?

kocio-pl · 2017-08-30T22:47:27Z

I haven't touched amenity-points.mss at all yet. There's a block:

openstreetmap-carto/amenity-points.mss

Lines 141 to 146 in 89ef164

    
           [feature = 'highway_ford'][zoom >= 16] { 
        
             marker-file: url('symbols/ford.svg'); 
        
             marker-fill: @transportation-icon; 
        
             marker-placement: interior; 
        
             marker-clip: false; 
        
           }

but I don't know how to add this rule.

kocio-pl · 2017-08-30T22:59:37Z

OK, now it works - the icon is visible. The question is if this whole block have to be duplicated to work with points and lines or there's a way to combine it?

matthijsmelissen · 2017-08-30T23:01:55Z

You can add class: points to your new layer, so the .class selector in the mss will select it.

In CartoCSS you can select either based on the class (with .) or based on the id (with #). The difference is that id's specify a unique layer, but multiple layers can have the same class.

If you need different rules for points and lines you can do something like this:

[feature = 'highway_ford'][zoom >= 16] {
  marker-file: url('symbols/ford.svg');
  marker-fill: @transportation-icon;
  .amenity-points {
    // These are specific point rules
    marker-placement: interior;
    marker-clip: false;
  }
  .amenity-line {
    // Specific line rules here
  }
}

kocio-pl · 2017-08-30T23:03:50Z

I want exactly the same rules for both.

matthijsmelissen · 2017-08-30T23:08:16Z

I want exactly the same rules for both.

Then adding the points class should be enough.

However, you might find you still need different rules though, for example the 'interior' property doesn't really makes sense for lines. But maybe it will just get ignored.

kocio-pl · 2017-08-30T23:36:42Z

Adding points class made me adding a lot of columns to amenity-line layer, so it makes more sense to just define additional rules for lines. Thanks for help!

kocio-pl · 2017-09-04T17:07:01Z

Any comments or reviews? From my point of view it's ready and it blocks resolving #484 (I guess both need separate PRs to decide).

pnorman

Some small fixes are needed, aside from that the code looks okay. Haven't loaded it up yet.

Because the amenity-line layer only has one type of feature, we could simplify its SQL significantly. On the other hand, having it mirror the other queries in form makes it easier to expand later, so the current way works too.

I support having the amenity-line stuff outside of the big amenities class selector as there's not substantial overlap.

pnorman · 2017-09-10T07:53:54Z

project.mml

+          COALESCE(
+           'highway_' || CASE WHEN tags->'ford' IN ('yes', 'stepping_stones') THEN 'ford' ELSE NULL END
+            ) AS feature
+          FROM planet_osm_line


This needs a WHERE clause.

pnorman · 2017-09-10T07:55:12Z

project.mml

@@ -1525,7 +1541,8 @@ Layer:
            OR "natural" IN ('peak', 'volcano', 'saddle', 'spring', 'cave_entrance')
            OR historic IN ('memorial', 'monument', 'archaeological_site', 'wayside_cross')
            OR tags->'memorial' IN ('plaque')
-            OR highway IN ('bus_stop', 'elevator', 'traffic_signals', 'ford')
+            OR highway IN ('bus_stop', 'elevator', 'traffic_signals')
+            OR tags->'ford' IN ('yes', 'stepping_stones')


OR tags @> 'ford=>yes' OR tags @> 'ford=>stepping_stones'

pnorman · 2017-09-10T07:55:45Z

project.mml

@@ -1474,7 +1489,8 @@ Layer:
              'historic_' || CASE WHEN historic IN ('memorial', 'monument', 'archaeological_site') 
                             THEN concat_ws('_', historic, CASE WHEN tags->'memorial' IN ('plaque') THEN tags->'memorial' ELSE NULL END) 
                             ELSE NULL END,
-              'highway_'|| CASE WHEN highway IN ('bus_stop', 'elevator', 'traffic_signals', 'ford') THEN highway ELSE NULL END,
+              'highway_'|| CASE WHEN highway IN ('bus_stop', 'elevator', 'traffic_signals') THEN highway 
+                                WHEN tags->'ford' IN ('yes', 'stepping_stones') THEN 'ford' ELSE NULL END,


WHEN tags @> 'ford=>yes' OR tags @> 'ford=>stepping_stones' THEN ...

pnorman · 2017-09-10T08:10:45Z

This will be adding another layer which has to retrieve all the objects for an area from disk, even if it doesn't send them. This is best avoided, but makes sense here. Any partial indexes would have to involve the tags column, which I'd like to avoid, and they would be a bit unnatural to construct.

kocio-pl · 2017-09-10T19:33:58Z

project.mml

+        (SELECT
+          way,
+          COALESCE(
+           'highway_' || CASE WHEN tags->'ford' IN ('yes', 'stepping_stones') THEN 'ford' ELSE NULL END


What about notation in this line - maybe it should be:

'highway_' || CASE WHEN tags @> 'ford=>yes' OR tags @> 'ford=>stepping_stones' THEN 'ford' ELSE NULL END

It's all too database related, so I'd like to have some kind of a rule of thumb when to use this notation and when it should be tags->'xxx'.

Within the output expressions it's purely a style issue. Our guidelines currently call for @>. The style disadvantage to @> is that you have to repeat tags @> for each option instead of something like && for arrays

I understand that this is not an error, because it was working also with the first notation and I tested that it still works. I can follow this style just to have clean and consistent code, I just don't know when should we use it and when not.

So what about this line?

@> to be consistent with our style guide.

pnorman · 2017-09-11T00:52:42Z

Unrelated to these changes, the ford symbol looks a little blurry

kocio-pl · 2017-09-11T01:10:28Z

Strange - it's a vector image after all...

pnorman · 2017-09-11T01:18:57Z

Strange - it's a vector image after all...

That's got nothing to do with being blurry, it's probably not pixel aligned.

matthijsmelissen mentioned this pull request Aug 16, 2017

Project governing status #2436

Closed

kocio-pl force-pushed the ford branch from 6e870f1 to a6523ec Compare August 30, 2017 22:33

kocio-pl force-pushed the ford branch from a6523ec to 3e8fe6a Compare August 30, 2017 22:58

kocio-pl force-pushed the ford branch from 3e8fe6a to 2411b99 Compare August 30, 2017 23:32

kocio-pl force-pushed the ford branch from 2411b99 to c8e8636 Compare August 31, 2017 00:03

kocio-pl changed the title ~~Moving ford to a new tagging scheme [WIP]~~ Moving ford to a new tagging scheme Aug 31, 2017

kocio-pl force-pushed the ford branch from c8e8636 to ff687b8 Compare August 31, 2017 00:34

kocio-pl added the input needed label Sep 10, 2017

pnorman requested changes Sep 10, 2017

View reviewed changes

kocio-pl force-pushed the ford branch from ff687b8 to 1ef4144 Compare September 10, 2017 19:28

kocio-pl commented Sep 10, 2017

View reviewed changes

Moving ford to a new tagging scheme

6419000

kocio-pl force-pushed the ford branch from 1ef4144 to 6419000 Compare September 10, 2017 23:28

pnorman approved these changes Sep 11, 2017

View reviewed changes

pnorman merged commit d212d8b into gravitystorm:master Sep 11, 2017

kocio-pl deleted the ford branch September 11, 2017 00:53

pnorman mentioned this pull request Sep 11, 2017

Ford symbol is blurry #2822

Closed

imagico mentioned this pull request May 9, 2024

Render railway:preserved=yes as preserved railway #4965

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving ford to a new tagging scheme #2743

Moving ford to a new tagging scheme #2743

kocio-pl commented Aug 13, 2017

matthijsmelissen commented Aug 20, 2017

kocio-pl commented Aug 30, 2017

matthijsmelissen commented Aug 30, 2017

kocio-pl commented Aug 30, 2017

kocio-pl commented Aug 30, 2017

matthijsmelissen commented Aug 30, 2017 •

edited

Loading

kocio-pl commented Aug 30, 2017

matthijsmelissen commented Aug 30, 2017 •

edited

Loading

kocio-pl commented Aug 30, 2017

kocio-pl commented Sep 4, 2017 •

edited

Loading

pnorman left a comment

pnorman Sep 10, 2017

kocio-pl Sep 10, 2017

pnorman Sep 10, 2017

pnorman Sep 10, 2017

pnorman commented Sep 10, 2017

kocio-pl Sep 10, 2017 •

edited

Loading

pnorman Sep 10, 2017

kocio-pl Sep 10, 2017

pnorman Sep 10, 2017

kocio-pl Sep 10, 2017

pnorman commented Sep 11, 2017

kocio-pl commented Sep 11, 2017

pnorman commented Sep 11, 2017

Moving ford to a new tagging scheme #2743

Moving ford to a new tagging scheme #2743

Conversation

kocio-pl commented Aug 13, 2017

matthijsmelissen commented Aug 20, 2017

kocio-pl commented Aug 30, 2017

matthijsmelissen commented Aug 30, 2017

kocio-pl commented Aug 30, 2017

kocio-pl commented Aug 30, 2017

matthijsmelissen commented Aug 30, 2017 • edited Loading

kocio-pl commented Aug 30, 2017

matthijsmelissen commented Aug 30, 2017 • edited Loading

kocio-pl commented Aug 30, 2017

kocio-pl commented Sep 4, 2017 • edited Loading

pnorman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnorman commented Sep 10, 2017

kocio-pl Sep 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnorman commented Sep 11, 2017

kocio-pl commented Sep 11, 2017

pnorman commented Sep 11, 2017

matthijsmelissen commented Aug 30, 2017 •

edited

Loading

matthijsmelissen commented Aug 30, 2017 •

edited

Loading

kocio-pl commented Sep 4, 2017 •

edited

Loading

kocio-pl Sep 10, 2017 •

edited

Loading