Skip to content

include surface at mid-zooms#1334

Merged
nvkelso merged 5 commits intotilezen:masterfrom
matkoniecz:surface
Jul 26, 2017
Merged

include surface at mid-zooms#1334
nvkelso merged 5 commits intotilezen:masterfrom
matkoniecz:surface

Conversation

@matkoniecz
Copy link
Copy Markdown
Contributor

@matkoniecz matkoniecz commented Jul 25, 2017

enables surface tag on z8 to z14 (as bicycle routes appear on z8)
fixes #1252

  • Update tests
  • Update data migrations (not needed as doesn't impact min_zoom)
  • Update docs

WIP, I created PR to run tests independently from my local instance (after reload of testing database test failed with claim that 11/1091/673 has no road layer (and that tile at http://tile.mapzen.com/mapzen/vector/v1/roads/11/1091/673.json has road layer).

enables surface tag on z8 to z14 (as bicycle routes appear on z8)
fixes #1252
Comment thread queries.yaml Outdated
(kind == 'path' and zoom < 15) or
(kind in ['minor_road', 'major_road', 'highway', 'rail'] and zoom < 13)
# drop certain road properties at lower zooms
- fn: vectordatasource.transform.drop_properties
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section isn't needed because we don't source from OpenStreetMap at the low zooms.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(You'd just leave out surface from the list of drop_properties.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread integration-test/1252-roads-surface.py Outdated

test.assert_has_feature(
10, 545, 336, 'roads',
{ 'id': 58691615, 'kind_detail': 'track', 'surface': 'concrete:lanes'})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strangely enough concrete:lanes is a valid surface value in OSM:

But in Tilezen we generally avoid : and - in property names or values in favor of _.

That's controlled in the YAML config. So update where surface is added to include a mapping:

More like how we trap aerialway values to prevent j-bar from being exported in with the case statement:

https://github.com/tilezen/vector-datasource/blob/master/yaml/roads.yaml#L633

  - filter:
      aerialway:
        - drag_lift
        - platter
        - t-bar
        - goods
        - magic_carpet
        - rope_tow
        - "yes"
        - zip_line
        - j-bar
        - unknown
        - mixed_lift
        - canopy
        - cableway
    min_zoom: 15
    table: osm
    output:
      <<: *osm_highway_properties
      <<: *osm_network
      kind: aerialway
      kind_detail:
        case:
          - { when: { aerialway: 't-bar' }, then: 't_bar' }
          - { when: { aerialway: 'j-bar' }, then: 'j_bar' }
          - { when: { aerialway: 'yes' }, then: null }
          - else: { col: 'aerialway' }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll want to do this for a few values, TagInfo has the full details:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"But in Tilezen we generally avoid : and - in property names or values in favor of _."

There is also concrete:plates (other are not worth handling).

I will add it to the transform and documentation of surface value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

OpenStreetMap data is not used at this zoom levels
@matkoniecz
Copy link
Copy Markdown
Contributor Author

I added transformation (on request). Now there are rare/malformed values that have untransformed colon.

Surface values except quite short list are malformed/really rare.

Maybe it would be a good idea to add also filtering and exclude very rare/malformed values?

I would probably reuse surface list from https://github.com/gravitystorm/openstreetmap-carto/blob/2be3a11ac240c82fb81200fbcd8be3aef16a187a/project.mml#L466

@rmarianski
Copy link
Copy Markdown
Member

Maybe it would be a good idea to add also filtering and exclude very rare/malformed values?

I would probably reuse surface list from https://github.com/gravitystorm/openstreetmap-carto/blob/2be3a11ac240c82fb81200fbcd8be3aef16a187a/project.mml#L466

👍 to whitelists where feasible

@nvkelso
Copy link
Copy Markdown
Member

nvkelso commented Jul 26, 2017

Filed #1335 to follow-up about is_paved boolean, which I think should also take care of any more white listing / value transforms.

Comment thread docs/layers.md Outdated
Mapzen calculates the `landuse_kind` value by intercutting `roads` with the `landuse` layer to determine if a road segment is over a parks, hospitals, universities or other landuse features. Use this property to modify the visual appearance of roads over these features. For instance, light grey minor roads look great in general, but aren't legible over most landuse colors unless they are darkened.

To improve performance, some road segments are merged at low and mid-zooms. To facilitate this, certain properties are dropped at those zooms. Examples include `is_bridge` and `is_tunnel`, `name`, `network`, `oneway`, `ref`, and `surface`. The exact zoom varies per feature class (major roads keep this properties over a wider range, minor roads drop them starting at zoom 14). When roads are merged, the original OSM `id` values are dropped.
To improve performance, some road segments are merged at low and mid-zooms. To facilitate this, certain properties are dropped at those zooms. Examples include `is_bridge` and `is_tunnel`, `name`, `network`, `oneway` and `ref`. The exact zoom varies per feature class (major roads keep this properties over a wider range, minor roads drop them starting at zoom 14). When roads are merged, the original OSM `id` values are dropped.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our practice is to use comma before the and in a list:

Examples include is_bridge and is_tunnel, name, network, oneway, and ref.

@nvkelso nvkelso changed the title [wip] include surface at mid-zooms include surface at mid-zooms Jul 26, 2017
@nvkelso nvkelso merged commit 0aacf36 into tilezen:master Jul 26, 2017
@nvkelso nvkelso removed the in review label Jul 26, 2017
@nvkelso
Copy link
Copy Markdown
Member

nvkelso commented Jul 26, 2017

Great contribution, thank you!

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.

surface property missing at mid-zooms

3 participants