Merge pull request #1452 from tilezen/zerebubuth/1062-road-shield-cle…

…anup

Allow some kinds of non-numeric shield_text

docs/layers.md

@@ -1031,7 +1031,7 @@ To improve performance, some road segments are merged at low and mid-zooms. To f
 * `ref`: Commonly-used reference for roads, for example "I 90" for Interstate 90. To use with shields, see `network` and `shield_text`. Related, see `symbol` for pistes.
 * `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.
 * `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.
-* `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. _See planned bug fix in [#1062](https://github.com/tilezen/vector-datasource/issues/1062)._
+* `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. For some roads, this can include non-numeric characters, for example the M1 motorway in the UK will have a `shield_text` of `M1`, rather than just `1`.
 
 #### Road properties (common optional):
 

integration-test/1062-road-shield-cleanup.py

@@ -0,0 +1,35 @@
+from . import FixtureTest
+
+
+class RoadShieldCleanup(FixtureTest):
+    def _check_network_relation(
+            self, way_id, rel_id, tile, expected_shield_text):
+        self.load_fixtures([
+            'https://www.openstreetmap.org/way/%d' % (way_id,),
+            'https://www.openstreetmap.org/relation/%d' % (rel_id,),
+        ], clip=self.tile_bbox(*tile))
+
+        z, x, y = tile
+        self.assert_has_feature(
+            z, x, y, 'roads',
+            {'id': way_id, 'shield_text': expected_shield_text})
+
+    def test_A151(self):
+        self._check_network_relation(
+            way_id=208288552, rel_id=1159812, tile=(16, 32949, 22362),
+            expected_shield_text='A151')
+
+    def test_E402(self):
+        self._check_network_relation(
+            way_id=121496753, rel_id=88503, tile=(16, 32975, 22371),
+            expected_shield_text='E402')
+
+    def test_A52(self):
+        self._check_network_relation(
+            way_id=358261897, rel_id=5715176, tile=(16, 32416, 21339),
+            expected_shield_text='A52')
+
+    def test_M1(self):
+        self._check_network_relation(
+            way_id=3109799, rel_id=2332838, tile=(16, 32531, 21377),
+            expected_shield_text='M1')

integration-test/1211-fix-null-network.py

@@ -5,11 +5,11 @@ class FixNullNetwork(FixtureTest):
     def test_routes_with_no_network(self):
         # ref="N 4", route=road, but no network=*
         # so we should get something that has no network, but a shield text of
-        # '4'
+        # 'N4' (see #1062 regarding why it's 'N4' rather than '4').
         self.load_fixtures(
             ['http://www.openstreetmap.org/relation/2307408'],
             clip=self.tile_bbox(11, 1038, 705))
 
         self.assert_has_feature(
             11, 1038, 705, 'roads',
-            {'kind': 'major_road', 'shield_text': '4', 'network': type(None)})
+            {'kind': 'major_road', 'shield_text': 'N4', 'network': type(None)})

test/test_transform.py

@@ -560,14 +560,15 @@ def _assert_shield_text(self, network, ref, expected_shield_text):
         self.assertEquals([expected_shield_text],
                           properties['all_shield_texts'])
 
+    def test_just_a_number(self):
+        self._assert_shield_text("whatever", "101", "101")
+
     def test_a_road(self):
         # based on http://www.openstreetmap.org/relation/2592
-        # simple pattern, should be just the number.
-        self._assert_shield_text("BAB", "A 66", "66")
+        self._assert_shield_text("BAB", "A 66", "A66")
 
         # based on http://www.openstreetmap.org/relation/446270
-        # simple pattern, should be just the number
-        self._assert_shield_text("FR:A-road", "A 66", "66")
+        self._assert_shield_text("FR:A-road", "A 66", "A66")
 
     def test_sr70var1(self):
         # based on http://www.openstreetmap.org/relation/449595

vectordatasource/transform.py

@@ -1,3 +1,4 @@
+# -*- encoding: utf-8 -*-
 # transformation functions to apply to features
 
 from collections import defaultdict, namedtuple
@@ -3932,6 +3933,58 @@ def _guess_type_from_network(network):
         return 'road'
 
 
+# a mapping of operator tag values to the networks that they are (probably)
+# part of. this would be better specified directly on the data, but sometimes
+# it's just not available.
+#
+# this is a list of the operators with >=100 uses on ways tagged as motorways,
+# which should hopefully allow us to catch most of the important ones. they're
+# mapped to the country they're in, which should be enough in most cases to
+# render the appropriate shield.
+_NETWORK_OPERATORS = {
+    'Highways England': 'GB',
+    'ASF': 'FR',
+    'Autopista Litoral Sul': 'BR',
+    'DNIT': 'BR',
+    'Εγνατία Οδός': 'GR',
+    'Αυτοκινητόδρομος Αιγαίου': 'GR',
+    'Transport Scotland': 'GB',
+    'The Danish Road Directorate': 'DK',
+    "Autostrade per l' Italia S.P.A.": 'IT',
+    'Νέα Οδός': 'GR',
+    'Autostrada dei Fiori S.P.A.': 'IT',
+    'S.A.L.T.': 'IT',
+    'Welsh Government': 'GB',
+    'Euroscut': 'PT',
+    'DIRIF': 'FR',
+    'Administración central': 'ES',
+    'Αττική Οδός': 'GR',
+    'Autocamionale della Cisa S.P.A.': 'IT',
+    'Κεντρική Οδός': 'GR',
+    'Bundesrepublik Deutschland': 'DE',
+    'Ecovias': 'BR',
+    '東日本高速道路': 'JP',
+    'NovaDutra': 'BR',
+    'APRR': 'FR',
+    'Via Solutions Südwest': 'DE',
+    'Autoroutes du Sud de la France': 'FR',
+    'Transport for Scotland': 'GB',
+    'Departamento de Infraestructuras Viarias y Movilidad': 'ES',
+    'ViaRondon': 'BR',
+    'DIRNO': 'FR',
+    'SATAP': 'IT',
+    'Ολυμπία Οδός': 'GR',
+    'Midland Expressway Ltd': 'GB',
+    'autobahnplus A8 GmbH': 'DE',
+    'Cart': 'BR',
+    'Μορέας': 'GR',
+    'Hyderabad Metropolitan Development Authority': 'PK',
+    'Viapar': 'BR',
+    'Autostrade Centropadane': 'IT',
+    'Triângulo do Sol': 'BR',
+}
+
+
 def merge_networks_from_tags(shape, props, fid, zoom):
     """
     Take the network and ref tags from the feature and, if they both exist, add
@@ -3943,15 +3996,29 @@ def merge_networks_from_tags(shape, props, fid, zoom):
     ref = props.get('ref')
     mz_networks = props.get('mz_networks', [])
 
+    # if there's no network, but the operator indicates a network, then we can
+    # back-fill an approximate network tag from the operator. this can mean
+    # that extra refs are available for road networks.
+    if network is None:
+        operator = props.get('operator')
+        backfill_network = _NETWORK_OPERATORS.get(operator)
+        if backfill_network:
+            network = backfill_network
+
     if network and ref:
-        props.pop('network')
+        props.pop('network', None)
         props.pop('ref')
         mz_networks.extend([_guess_type_from_network(network), network, ref])
         props['mz_networks'] = mz_networks
 
     return (shape, props, fid)
 
 
+# a pattern to find any number in a string, as a fallback for looking up road
+# reference numbers.
+_ANY_NUMBER = re.compile('[^0-9]*([0-9]+)')
+
+
 def _road_network_importance(network, ref):
     """
     Returns an integer representing the numeric importance of the network,
@@ -3981,9 +4048,22 @@ def _road_network_importance(network, ref):
         network_code = len(network.split(':')) + 3
 
     try:
-        ref = max(int(ref or 0), 0)
+        # first, see if the reference is a number, or easily convertible
+        # into one.
+        ref = int(ref or 0)
     except ValueError:
-        ref = 0
+        # if not, we can try to extract anything that looks like a sequence
+        # of digits from the ref.
+        m = _ANY_NUMBER.match(ref)
+        if m:
+            ref = int(m.group(1))
+        else:
+            # failing that, we assume that a completely non-numeric ref is
+            # a name, which would make it quite important.
+            ref = 0
+
+    # make sure no ref is negative
+    ref = abs(ref)
 
     return network_code * 10000 + min(ref, 9999)
 
@@ -4033,6 +4113,7 @@ def _bus_network_importance(network, ref):
 
 
 _NUMBER_AT_FRONT = re.compile('^(\d+\w*)', re.UNICODE)
+_SINGLE_LETTER_AT_FRONT = re.compile('^([^\W\d]) *(\d+)', re.UNICODE)
 _LETTER_THEN_NUMBERS = re.compile('^[^\d\s_]+[ -]?([^\s]+)',
                                   re.UNICODE | re.IGNORECASE)
 _UA_TERRITORIAL_RE = re.compile('^(\w)-(\d+)-(\d+)$',
@@ -4084,6 +4165,12 @@ def _road_shield_text(network, ref):
     if m:
         return m.group(1)
 
+    # If there's a letter at the front, optionally space, and then a number,
+    # the ref is the concatenation (without space) of the letter and number.
+    m = _SINGLE_LETTER_AT_FRONT.match(ref)
+    if m:
+        return m.group(1) + m.group(2)
+
     # Otherwise, try to match a bunch of letters followed by a number.
     m = _LETTER_THEN_NUMBERS.match(ref)
     if m: