From 3d90278909c362b9b598f771bc56e3bff2088445 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 13:04:34 +0100 Subject: [PATCH 01/10] Don't rely on 'network' for bicycle networks, use 'bicycle_network' instead. --- integration-test/647-cycle-route.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/integration-test/647-cycle-route.py b/integration-test/647-cycle-route.py index 4b81d139f..cc8fb7f31 100644 --- a/integration-test/647-cycle-route.py +++ b/integration-test/647-cycle-route.py @@ -3,17 +3,17 @@ # http://www.openstreetmap.org/relation/32386 assert_has_feature( 16, 10487, 25327, 'roads', - { 'kind': 'major_road', 'cycleway': 'lane', 'network': 'lcn', 'bicycle_network': 'lcn' }) + { 'kind': 'major_road', 'cycleway': 'lane', 'bicycle_network': 'lcn' }) # Way: King Street (8920394) http://www.openstreetmap.org/way/8920394 assert_has_feature( 16, 10487, 25329, 'roads', - { 'kind': 'major_road', 'cycleway_left': 'lane', 'network': 'lcn', 'bicycle_network': 'lcn'}) + { 'kind': 'major_road', 'cycleway_left': 'lane', 'bicycle_network': 'lcn'}) # Way: King Street (397270776) http://www.openstreetmap.org/way/397270776 assert_has_feature( 16, 10487, 25329, 'roads', - { 'kind': 'major_road', 'cycleway_right': 'lane', 'network': 'lcn', 'bicycle_network': 'lcn'}) + { 'kind': 'major_road', 'cycleway_right': 'lane', 'bicycle_network': 'lcn'}) # Way: Clara-Immerwahr-Straße (287167007) http://www.openstreetmap.org/way/287167007 assert_has_feature( From 945273f3f4e809dc38d79a02cdfe1bbca13c8eda Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 13:05:45 +0100 Subject: [PATCH 02/10] Fetch all networks for a road, and sort them to find the most important. Use the 'ref' from that relation to form the 'shield_text' output property. --- data/functions.sql | 31 +++++++++++------ queries.yaml | 1 + vectordatasource/transform.py | 65 +++++++++++++++++++++++++++++++++++ yaml/roads.yaml | 2 +- 4 files changed, 87 insertions(+), 12 deletions(-) diff --git a/data/functions.sql b/data/functions.sql index 2f46d0295..14bd42db4 100644 --- a/data/functions.sql +++ b/data/functions.sql @@ -83,24 +83,33 @@ BEGIN END; $$ LANGUAGE plpgsql IMMUTABLE; --- mz_get_rel_network returns a network tag, or NULL, for a --- given way ID. +-- mz_get_rel_networks returns a list of triples of route type, +-- network and ref tags, or NULL, for a given way ID. -- -- it does this by joining onto the relations slim table, so it -- won't work if you dropped the slim tables, or didn't use slim -- mode in osm2pgsql. -- -CREATE OR REPLACE FUNCTION mz_get_rel_network( +CREATE OR REPLACE FUNCTION mz_get_rel_networks( way_id bigint) RETURNS text AS $$ -BEGIN - RETURN mz_first_dedup(ARRAY( - SELECT mz_rel_get_tag(tags, 'network') - FROM planet_osm_rels - WHERE parts && ARRAY[way_id] - AND parts[way_off+1:rel_off] && ARRAY[way_id])); -END; -$$ LANGUAGE plpgsql STABLE; +SELECT + string_agg(unnested, '|') +FROM ( + SELECT + unnest(tags) AS unnested + FROM ( + SELECT + hstore(tags)->ARRAY['route','network','ref'] AS tags + FROM + planet_osm_rels + WHERE + parts && ARRAY[way_id] AND + parts[way_off+1:rel_off] && ARRAY[way_id] AND + hstore(tags) ?& ARRAY['route','network','ref'] + ) inner1 + ) inner2; +$$ LANGUAGE sql STABLE; -- adds the prefix onto every key in an hstore value CREATE OR REPLACE FUNCTION mz_hstore_add_prefix( diff --git a/queries.yaml b/queries.yaml index 19e6ac0c3..6a021822c 100644 --- a/queries.yaml +++ b/queries.yaml @@ -89,6 +89,7 @@ layers: - vectordatasource.transform.normalize_aerialways - vectordatasource.transform.normalize_cycleway - vectordatasource.transform.add_is_bicycle_related + - vectordatasource.transform.choose_most_important_network - vectordatasource.transform.road_trim_properties - vectordatasource.transform.remove_feature_id - vectordatasource.transform.tags_remove diff --git a/vectordatasource/transform.py b/vectordatasource/transform.py index 04148dbea..0d51605eb 100644 --- a/vectordatasource/transform.py +++ b/vectordatasource/transform.py @@ -3676,3 +3676,68 @@ def normalize_operator_values(shape, properties, fid, zoom): return (shape, properties, fid) return (shape, properties, fid) + + +def network_importance(route_type, network, ref): + """ + Returns an integer representing the numeric importance of the network, + where lower numbers are more important. + + This is to handle roads which are part of many networks, and ensuring + that the most important one is displayed. For example, in the USA many + roads can be part of both interstate (US:I) and "US" (US:US) highways, + and possibly state ones as well (e.g: US:NY:xxx). In addition, there + are international conventions around the use of "CC:national" and + "CC:regional:*" where "CC" is an ISO 2-letter country code. + + Here we treat national-level roads as more important than regional or + lower, and assume that the deeper the network is in the hierarchy, the + less important the road. Roads with lower "ref" numbers are considered + more important than higher "ref" numbers, if they are part of the same + network. + """ + + if network == 'US:I' or ':national' in network: + network_code = 1 + elif network == 'US:US' or ':regional' in network: + network_code = 2 + else: + network_code = len(network.split(':')) + 3 + + try: + ref = max(int(ref), 0) + except ValueError: + ref = 0 + + return network_code * 10000 + min(ref, 9999) + + +def choose_most_important_network(shape, properties, fid, zoom): + """ + Use the `network_importance` function to select any road networks from + `mz_networks` and take the most important one. + """ + + networks = properties.pop('mz_networks', None) + + if networks is not None: + networks = networks.split('|') + + # take the list and make triples out of it + itr = iter(networks) + triples = zip(itr, itr, itr) + + def is_road_network(t): + return t[0] == 'road' + + triples = filter(is_road_network, triples) + + if len(triples) > 0: + def network_key(t): + return network_importance(*t) + + route_type, network, ref = sorted(triples, key=network_key)[0] + properties['network'] = network + properties['shield_text'] = ref + + return (shape, properties, fid) diff --git a/yaml/roads.yaml b/yaml/roads.yaml index 1715b9ffa..bd2ac8e39 100644 --- a/yaml/roads.yaml +++ b/yaml/roads.yaml @@ -51,7 +51,7 @@ global: CASE WHEN tags->'crossing' <> 'no' THEN tags->'crossing' END sidewalk: {col: tags->sidewalk} - &osm_network_from_relation - network: {expr: "mz_get_rel_network(osm_id)"} + mz_networks: {expr: "mz_get_rel_networks(osm_id)"} - &osm_network_from_tags network: {col: tags->network} - &osm_piste_properties From 594b0e28d840de323fd98a5a2f3acd543d94fec2 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 13:10:40 +0100 Subject: [PATCH 03/10] Add test case for networks and network sorting. --- integration-test/192-shield-text-ref.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 integration-test/192-shield-text-ref.py diff --git a/integration-test/192-shield-text-ref.py b/integration-test/192-shield-text-ref.py new file mode 100644 index 000000000..ea1b30451 --- /dev/null +++ b/integration-test/192-shield-text-ref.py @@ -0,0 +1,19 @@ +# US 101, "James Lick Freeway" +# http://www.openstreetmap.org/way/27183379 +# http://www.openstreetmap.org/relation/108619 +assert_has_feature( + 16, 10484, 25334, 'roads', + { 'kind': 'highway', 'network': 'US:US', 'id': 27183379, + 'shield_text': '101' }) + +# I-77, I-81, US-11 & US-52 all in one road West Virginia. +# +# http://www.openstreetmap.org/way/51388984 +# http://www.openstreetmap.org/relation/2309416 +# http://www.openstreetmap.org/relation/2301037 +# http://www.openstreetmap.org/relation/2297359 +# http://www.openstreetmap.org/relation/1027748 +assert_has_feature( + 16, 18022, 25522, 'roads', + { 'kind': 'highway', 'network': 'US:I', 'id': 51388984, + 'shield_text': '77' }) From 3686b08965308a6033810821630b0459fe10b33a Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 13:13:26 +0100 Subject: [PATCH 04/10] If we're dropping network, then shield_text is useless - the network determines which shield to use. --- queries.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/queries.yaml b/queries.yaml index 6a021822c..9a2149e5c 100644 --- a/queries.yaml +++ b/queries.yaml @@ -568,7 +568,7 @@ post_process: source_layer: roads start_zoom: 0 end_zoom: 14 - properties: [name, ref, network] + properties: [name, ref, network, shield_text] where: >- (kind == 'rail' and zoom < 15) or (kind == 'minor_road' and zoom < 14) or @@ -581,7 +581,7 @@ post_process: source_layer: roads start_zoom: 7 end_zoom: 10 - properties: [name, network] + properties: [name, network, shield_text] where: >- kind == 'major_road' # this is a patch to get rid of name, but keep ref & network, for highways From 8a5e1321f44334f57bceeaf2b1317f7a304543bb Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 13:17:51 +0100 Subject: [PATCH 05/10] Add documentation for new 'shield_text' property. Clarify other properties in relation to it. --- docs/layers.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/layers.md b/docs/layers.md index 8eab3e34d..20269ef29 100644 --- a/docs/layers.md +++ b/docs/layers.md @@ -837,7 +837,7 @@ To improve performance, some road segments are merged at low and mid-zooms. To f * `source`: `openstreetmap` or `naturalearthdata.com` * `kind`: one of High Road's values for `highway`, `major_road`, `minor_road`, `rail`, `path`, `ferry`, `piste`, `aerialway`, `aeroway`, `racetrack`, `portage_way` if `whitewater=portage_way`; or Natural Earth's `featurecla` value. You'll want to look at other tags like `highway` and `railway` for raw OpenStreetMap values. At low zooms, Natural Earth `featurecla` kinds of `Road` and `Ferry` are used. Look to `type` for more fidelity. * `landuse_kind`: See description above, values match values in the `landuse` layer. -* `ref`: Used for road shields. Related, see `symbol` for pistes. +* `ref`: Commonly-used reference for roads, for example "I 90" for Interstate 90. To use with shields, see the common optional properties `network` and `shield_text`. Related, see `symbol` for pistes. * `sort_key`: a suggestion for which order to draw features. The value is an integer where smaller numbers suggest that features should be "behind" features with larger numbers. At zooms >= 15, the `sort_key` is adjusted to realistically model bridge, tunnel, and layer ordering. #### Road properties (common optional): @@ -857,13 +857,14 @@ To improve performance, some road segments are merged at low and mid-zooms. To f * `is_tunnel`: `true` if the road is part of a tunnel. The property will not be present if the road is not part of a tunnel. * `leisure`: See kind list below. * `man_made`: See kind list below. -* `network`: eg: `US:I` for the United States Interstate network, useful for shields and road selections. +* `network`: eg: `US:I` for the United States Interstate network, useful for shields and road selections. This only contains _road_ network types. Please see `bicycle_network` and `walking_network` for bicycle and walking networks, respectively. * `oneway_bicycle`: `oneway:bicycle` tag from feature * `oneway`: `yes` or `no` * `piste_type`: See kind list below. * `railway`: the original OSM railway tag value * `segregated`: Set to `true` when a path allows both pedestrian and bicycle traffic, but when pedestrian traffic is segregated from bicycle traffic. * `service`: See value list below, provided for `railway` and `highway=service` roads. +* `shield_text`: Contains text to display on a shield. For example, I 90 would have a `network` of `US:I` and a `shield_text` of `90`. The `ref`, `I 90`, is less useful for shield display without further processing. * `type`: Natural Earth roads and ferry * `walking_network`: Present if the feature is part of a hiking network. If so, the value will be one of `iwn` for International Walking Network, `nwn` for National Walking Network, `rwn` for Regional Walking Network, `lwn` for Local Walking Network. * `kind_detail`: normalized values describing the kind value, see below. From 09be38e9b9cf24caf34eeca98777f08169b65b7f Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 13:25:45 +0100 Subject: [PATCH 06/10] Add cleanup for old, replaced version of the function. --- data/migrations/v1.0.0-cleanup.sql | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 data/migrations/v1.0.0-cleanup.sql diff --git a/data/migrations/v1.0.0-cleanup.sql b/data/migrations/v1.0.0-cleanup.sql new file mode 100644 index 000000000..3b1df1f4a --- /dev/null +++ b/data/migrations/v1.0.0-cleanup.sql @@ -0,0 +1,2 @@ +-- drop no longer used single-network version of this function +DROP FUNCTION mz_get_rel_network(bigint); From 861ac6d9340038094fc7005d048e6de698b02e1f Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 16:24:53 +0100 Subject: [PATCH 07/10] Don't stringify - keep as array through JSON. --- data/functions.sql | 4 ++-- vectordatasource/transform.py | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/data/functions.sql b/data/functions.sql index 14bd42db4..90d90ae00 100644 --- a/data/functions.sql +++ b/data/functions.sql @@ -92,9 +92,9 @@ $$ LANGUAGE plpgsql IMMUTABLE; -- CREATE OR REPLACE FUNCTION mz_get_rel_networks( way_id bigint) -RETURNS text AS $$ +RETURNS text[] AS $$ SELECT - string_agg(unnested, '|') + array_agg(unnested) FROM ( SELECT unnest(tags) AS unnested diff --git a/vectordatasource/transform.py b/vectordatasource/transform.py index 0d51605eb..78362d4d6 100644 --- a/vectordatasource/transform.py +++ b/vectordatasource/transform.py @@ -3721,8 +3721,6 @@ def choose_most_important_network(shape, properties, fid, zoom): networks = properties.pop('mz_networks', None) if networks is not None: - networks = networks.split('|') - # take the list and make triples out of it itr = iter(networks) triples = zip(itr, itr, itr) From 99ea8027ead10696b4729379f7f84e9ab9e8174b Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 16:37:34 +0100 Subject: [PATCH 08/10] Simplify code by rewriting as a list comprehension. --- vectordatasource/transform.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/vectordatasource/transform.py b/vectordatasource/transform.py index 78362d4d6..8718c6a5e 100644 --- a/vectordatasource/transform.py +++ b/vectordatasource/transform.py @@ -3724,11 +3724,7 @@ def choose_most_important_network(shape, properties, fid, zoom): # take the list and make triples out of it itr = iter(networks) triples = zip(itr, itr, itr) - - def is_road_network(t): - return t[0] == 'road' - - triples = filter(is_road_network, triples) + triples = [t for t in triples if t[0] == 'road'] if len(triples) > 0: def network_key(t): From 62375dadc817b97cf549ae8eb76500b7774bf817 Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 16:44:14 +0100 Subject: [PATCH 09/10] Expose all networks & shield texts for use with multiple shield capable renderers. --- docs/layers.md | 1 + integration-test/192-shield-text-ref.py | 4 +++- vectordatasource/transform.py | 9 ++++++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/docs/layers.md b/docs/layers.md index 20269ef29..d3de12ce3 100644 --- a/docs/layers.md +++ b/docs/layers.md @@ -844,6 +844,7 @@ To improve performance, some road segments are merged at low and mid-zooms. To f * `aerialway`: See kind list below. * `aeroway`: See kind list below. +* `all_networks` and `all_shield_texts`: All the networks of which this road is a part, and all of the shield texts. See `network` and `shield_text` below. **Note** that these properties will not be present on MVT format tiles, as we cannot currently encode lists as values. * `bicycle_network`: Present if the feature is part of a cycling network. If so, the value will be one of `icn` for International Cycling Network, `ncn` for National Cycling Network, `rcn` for Regional Cycling Network, `lcn` for Local Cycling Network. * `cycleway`: `cycleway` tag from feature. If no `cycleway` tag is present but `cycleway:both` exists, we source from that tag instead. * `cycleway_left`: `cycleway_left` tag from feature diff --git a/integration-test/192-shield-text-ref.py b/integration-test/192-shield-text-ref.py index ea1b30451..36211f455 100644 --- a/integration-test/192-shield-text-ref.py +++ b/integration-test/192-shield-text-ref.py @@ -16,4 +16,6 @@ assert_has_feature( 16, 18022, 25522, 'roads', { 'kind': 'highway', 'network': 'US:I', 'id': 51388984, - 'shield_text': '77' }) + 'shield_text': '77', + 'all_networks': ['US:I', 'US:I', 'US:US', 'US:US'], + 'all_shield_texts': ['77', '81', '11', '52'] }) diff --git a/vectordatasource/transform.py b/vectordatasource/transform.py index 8718c6a5e..39c6f562d 100644 --- a/vectordatasource/transform.py +++ b/vectordatasource/transform.py @@ -3730,8 +3730,15 @@ def choose_most_important_network(shape, properties, fid, zoom): def network_key(t): return network_importance(*t) - route_type, network, ref = sorted(triples, key=network_key)[0] + networks = sorted(triples, key=network_key) + + # expose first network as network/shield_text + route_type, network, ref = networks[0] properties['network'] = network properties['shield_text'] = ref + # expose all networks as well. + properties['all_networks'] = [n[1] for n in networks] + properties['all_shield_texts'] = [n[2] for n in networks] + return (shape, properties, fid) From bba8a952a30890e41869b968d89db98fba20c8ce Mon Sep 17 00:00:00 2001 From: Matt Amos Date: Fri, 12 Aug 2016 17:34:40 +0100 Subject: [PATCH 10/10] Be deeper about freezing properties dictionaries for road merging. --- integration-test/358-merge-same-roads.py | 12 ++++++++- vectordatasource/transform.py | 32 ++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/integration-test/358-merge-same-roads.py b/integration-test/358-merge-same-roads.py index cde36249e..e5eb3123e 100644 --- a/integration-test/358-merge-same-roads.py +++ b/integration-test/358-merge-same-roads.py @@ -8,11 +8,21 @@ # RAW QUERY: way(36.563,-122.377,37.732,-120.844)[highway=primary];>; # RAW QUERY: way(36.563,-122.377,37.732,-120.844)[highway=trunk];>; # + +def _freeze(thing): + if isinstance(thing, dict): + return frozenset([(_freeze(k), _freeze(v)) for k, v in thing.items()]) + + elif isinstance(thing, list): + return tuple([_freeze(i) for i in thing]) + + return thing + with features_in_tile_layer(8, 41, 99, 'roads') as roads: features = set() for road in roads: - props = frozenset(road['properties'].items()) + props = frozenset(_freeze(road['properties'])) if props in features: raise Exception("Duplicate properties %r in roads layer, but " "properties should be unique." diff --git a/vectordatasource/transform.py b/vectordatasource/transform.py index 39c6f562d..714f29390 100644 --- a/vectordatasource/transform.py +++ b/vectordatasource/transform.py @@ -2764,6 +2764,34 @@ def add_uic_ref(shape, properties, fid, zoom): return shape, properties, fid +def _freeze(thing): + """ + Freezes something to a hashable item. + """ + + if isinstance(thing, dict): + return frozenset([(_freeze(k), _freeze(v)) for k, v in thing.items()]) + + elif isinstance(thing, list): + return tuple([_freeze(i) for i in thing]) + + return thing + + +def _thaw(thing): + """ + Reverse of the freeze operation. + """ + + if isinstance(thing, frozenset): + return dict([_thaw(i) for i in thing]) + + elif isinstance(thing, tuple): + return list([_thaw(i) for i in thing]) + + return thing + + def merge_features(ctx): """ Merge (linear) features with the same properties together, attempting to @@ -2814,7 +2842,7 @@ def merge_features(ctx): # because dicts are mutable and therefore not hashable, we have to # transform their items into a frozenset instead. - frozen_props = frozenset(props.items()) + frozen_props = _freeze(props) if frozen_props in features_by_property: features_by_property[frozen_props][2].append(shape) @@ -2833,7 +2861,7 @@ def merge_features(ctx): multi = MultiLineString(list_of_linestrings) # thaw the frozen properties to use in the new feature. - props = dict(frozen_props) + props = _thaw(frozen_props) # restore any 'id' property. if p_id is not None: