Merge pull request #1155 from Conengmo/fix-geojson-identifier

Fix geojson identifier

Author: ocefpaf

folium/features.py

@@ -521,17 +521,31 @@ def _validate_function(self, func, name):
                              .format(name))
 
     def find_identifier(self):
-        """Find a unique identifier for each feature, create it if needed."""
-        features = self.data['features']
-        n = len(features)
-        feature = features[0]
-        if 'id' in feature and len(set(feat['id'] for feat in features)) == n:
+        """Find a unique identifier for each feature, create it if needed.
+
+        According to the GeoJSON specs a feature:
+         - MAY have an 'id' field with a string or numerical value.
+         - MUST have a 'properties' field. The content can be any json object
+           or even null.
+
+        """
+        feats = self.data['features']
+        # Each feature has an 'id' field with a unique value.
+        unique_ids = set(feat.get('id', None) for feat in feats)
+        if None not in unique_ids and len(unique_ids) == len(feats):
             return 'feature.id'
-        for key in feature.get('properties', []):
-            if len(set(feat['properties'][key] for feat in features)) == n:
-                return 'feature.properties.{}'.format(key)
+        # Each feature has a unique string or int property.
+        if all(isinstance(feat.get('properties', None), dict) for feat in feats):
+            for key in feats[0]['properties']:
+                unique_values = set(
+                    feat['properties'].get(key, None) for feat in feats
+                    if isinstance(feat['properties'].get(key, None), (str, int))
+                )
+                if len(unique_values) == len(feats):
+                    return 'feature.properties.{}'.format(key)
+        # We add an 'id' field with a unique value to the data.
         if self.embed:
-            for i, feature in enumerate(self.data['features']):
+            for i, feature in enumerate(feats):
                 feature['id'] = str(i)
             return 'feature.id'
         raise ValueError(

tests/test_features.py

@@ -12,7 +12,7 @@
 from branca.element import Element
 
 import folium
-from folium import Map, Popup
+from folium import Map, Popup, GeoJson
 
 import pytest
 
@@ -213,3 +213,93 @@ def test_geojson_tooltip():
         warnings.simplefilter('always')
         m._repr_html_()
         assert issubclass(w[-1].category, UserWarning), 'GeoJsonTooltip GeometryCollection test failed.'
+
+
+def test_geojson_find_identifier():
+
+    def _create(*properties):
+        return {"type": "FeatureCollection", "features": [
+            {"type": "Feature", "properties": item}
+            for item in properties]}
+
+    def _assert_id_got_added(data):
+        _geojson = GeoJson(data)
+        assert _geojson.find_identifier() == 'feature.id'
+        assert _geojson.data['features'][0]['id'] == '0'
+
+    data_with_id = _create(None, None)
+    data_with_id['features'][0]['id'] = 'this-is-an-id'
+    data_with_id['features'][1]['id'] = 'this-is-another-id'
+    geojson = GeoJson(data_with_id)
+    assert geojson.find_identifier() == 'feature.id'
+    assert geojson.data['features'][0]['id'] == 'this-is-an-id'
+
+    data_with_unique_properties = _create(
+        {'property-key': 'some-value'},
+        {'property-key': 'another-value'},
+    )
+    geojson = GeoJson(data_with_unique_properties)
+    assert geojson.find_identifier() == 'feature.properties.property-key'
+
+    data_with_unique_properties = _create(
+        {'property-key': 42},
+        {'property-key': 43},
+        {'property-key': 'or a string'},
+    )
+    geojson = GeoJson(data_with_unique_properties)
+    assert geojson.find_identifier() == 'feature.properties.property-key'
+
+    # The test cases below have no id field or unique property,
+    # so an id will be added to the data.
+
+    data_with_identical_ids = _create(None, None)
+    data_with_identical_ids['features'][0]['id'] = 'identical-ids'
+    data_with_identical_ids['features'][1]['id'] = 'identical-ids'
+    _assert_id_got_added(data_with_identical_ids)
+
+    data_with_some_missing_ids = _create(None, None)
+    data_with_some_missing_ids['features'][0]['id'] = 'this-is-an-id'
+    # the second feature doesn't have an id
+    _assert_id_got_added(data_with_some_missing_ids)
+
+    data_with_identical_properties = _create(
+        {'property-key': 'identical-value'},
+        {'property-key': 'identical-value'},
+    )
+    _assert_id_got_added(data_with_identical_properties)
+
+    data_bare = _create(None)
+    _assert_id_got_added(data_bare)
+
+    data_empty_dict = _create({})
+    _assert_id_got_added(data_empty_dict)
+
+    data_without_properties = _create(None)
+    del data_without_properties['features'][0]['properties']
+    _assert_id_got_added(data_without_properties)
+
+    data_some_without_properties = _create({'key': 'value'}, 'will be deleted')
+    # the first feature has properties, but the second doesn't
+    del data_some_without_properties['features'][1]['properties']
+    _assert_id_got_added(data_some_without_properties)
+
+    data_with_nested_properties = _create({
+        "summary": {"distance": 343.2},
+        "way_points": [3, 5],
+    })
+    _assert_id_got_added(data_with_nested_properties)
+
+    data_with_incompatible_properties = _create({
+        "summary": {"distances": [0, 6], "durations": None},
+        "way_points": [3, 5],
+    })
+    _assert_id_got_added(data_with_incompatible_properties)
+
+    data_loose_geometry = {"type": "LineString", "coordinates": [
+        [3.961389, 43.583333], [3.968056, 43.580833], [3.974722, 43.578333],
+        [3.986389, 43.575278], [3.998333, 43.5725], [4.163333, 43.530556],
+    ]}
+    geojson = GeoJson(data_loose_geometry)
+    geojson.convert_to_feature_collection()
+    assert geojson.find_identifier() == 'feature.id'
+    assert geojson.data['features'][0]['id'] == '0'