Repository: python-visualization/folium

Issue:
GeoJSON feature ID creator fails when properties contain list/dict
This code throws a `TypeError`:
(**edit**: add geometry type to feature)
```python
import folium
json = {
    "type": "FeatureCollection",
    "features": [
        {
            "bbox": [
                8.546326,
                47.417879,
                8.548813,
                47.420362
            ],
            "type": "Feature",
            "properties": {
                "summary": {
                    "distance": 343.2,
                    "duration": 50.5
                },
                "way_points": [
                    0,
                    15
                ]
            },
            "geometry": {
                "coordinates": [
                    [
                        8.548813,
                        47.420362
                    ],
                    [
                        8.548795,
                        47.420345
                    ],
                    [
                        8.548621,
                        47.420167
                    ]
                ],
                "type": "LineString"
            }
        }
    ]
}

m = folium.Map(location=[47.417879, 8.546326], zoom_start=13)

folium.features.GeoJson(json, style_function=lambda x: {'opacity': 1.0}).add_to(m)
m
```
#### Problem description
If a `style_function` is passed, `folium` tries to find an ID for the GeoJSON features. When no `id` field is present (not required in GeoJSONs AFAIK), it will try to pick a field from properties by checking if the values of the property fields are unique. However, the current implementation with `set` to check uniqueness fails when the property field values are `dict`s or `list`s.

#### Quick Fix
It works with a try/except around the [the appropriate lines](https://github.com/python-visualization/folium/blob/5d14833595718acad49d45e0a30fb4225e3f9c56/folium/features.py#L531):
```python
try:
    if len(set(feat['properties'][key] for feat in features)) == n:
        return 'feature.properties.{}'.format(key)
except TypeError:
    continue
```
I'd be happy with this if was my library, as far as I can see nothing else could raise a `TypeError` here. What do you think?

#### Output of ``0.9.0 ``
```
    530         for key in feature.get('properties', []):
    531             # try:
--> 532             if len(set(feat['properties'][key] for feat in features)) == n:
    533                 return 'feature.properties.{}'.format(key)
    534             # except TypeError:

TypeError: unhashable type: 'dict'
```


Discussion:

Patch:
diff --git a/folium/features.py b/folium/features.py
index 93d65616..7796c29b 100644
--- a/folium/features.py
+++ b/folium/features.py
@@ -521,17 +521,31 @@ class GeoJson(Layer):
                              .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(


Test patch:
diff --git a/tests/test_features.py b/tests/test_features.py
index e77f42ad..fb48e0a9 100644
--- a/tests/test_features.py
+++ b/tests/test_features.py
@@ -12,7 +12,7 @@ import warnings
 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'