add attribute featureidkey

etpinard · etpinard · commit 5709d1b6c465 · 2019-12-10T12:42:33.000-05:00
... to choroplethmapbox, scattergeo and choropleth traces.

- Coerced only when `locationmode: 'geojson-id'`. To determine
  which key (or nested key) to use to identify eahc geojson feature.
- Improve error messages in extractTraceFeature
diff --git a/src/lib/geo_location_utils.js b/src/lib/geo_location_utils.js
@@ -16,6 +16,7 @@ var turfCentroid = require('@turf/centroid');
 var identity = require('./identity');
 var loggers = require('./loggers');
 var isPlainObject = require('./is_plain_object');
+var nestedProperty = require('./nested_property');
 var polygon = require('./polygon');
 
 // make list of all country iso3 ids from at runtime
@@ -208,14 +209,16 @@ function extractTraceFeature(calcTrace) {
     }
 
     function appendFeature(fIn) {
-        var cdi = lookup[fIn.id];
+        var id = nestedProperty(fIn, trace.featureidkey || 'id').get();
+        var cdi = lookup[id];
 
         if(cdi) {
             var geometry = fIn.geometry;
 
             if(geometry.type === 'Polygon' || geometry.type === 'MultiPolygon') {
                 var fOut = {
                     type: 'Feature',
+                    id: id,
                     geometry: geometry,
                     properties: {}
                 };
@@ -230,15 +233,16 @@ function extractTraceFeature(calcTrace) {
                 featuresOut.push(fOut);
             } else {
                 loggers.log([
-                    'Location with id', cdi.loc, 'does not have a valid GeoJSON geometry,',
-                    'choroplethmapbox traces only support *Polygon* and *MultiPolygon* geometries.'
+                    'Location', cdi.loc, 'does not have a valid GeoJSON geometry.',
+                    'Traces with locationmode *geojson-id* only support',
+                    '*Polygon* and *MultiPolygon* geometries.'
                 ].join(' '));
             }
         }
 
         // remove key from lookup, so that we can track (if any)
         // the locations that did not have a corresponding GeoJSON feature
-        delete lookup[fIn.id];
+        delete lookup[id];
     }
 
     switch(geojsonIn.type) {
@@ -253,14 +257,19 @@ function extractTraceFeature(calcTrace) {
             break;
         default:
             loggers.warn([
-                'Invalid GeoJSON type', (geojsonIn.type || 'none') + ',',
-                'choroplethmapbox traces only support *FeatureCollection* and *Feature* types.'
+                'Invalid GeoJSON type', (geojsonIn.type || 'none') + '.',
+                'Traces with locationmode *geojson-id* only support',
+                '*FeatureCollection* and *Feature* types.'
             ].join(' '));
             return false;
     }
 
     for(var loc in lookup) {
-        loggers.log('Location with id ' + loc + ' does not have a matching feature');
+        loggers.log([
+            'Location *' + loc + '*',
+            'does not have a matching feature with id-key',
+            '*' + trace.featureidkey + '*.'
+        ].join(' '));
     }
 
     return featuresOut;
diff --git a/src/traces/choropleth/attributes.js b/src/traces/choropleth/attributes.js
@@ -46,6 +46,8 @@ module.exports = extendFlat({
             // https://github.com/topojson/topojson-specification/blob/master/README.md
         ].join(' ')
     }),
+    featureidkey: scatterGeoAttrs.featureidkey,
+
     text: extendFlat({}, scatterGeoAttrs.text, {
         description: 'Sets the text elements associated with each location.'
     }),
diff --git a/src/traces/choropleth/defaults.js b/src/traces/choropleth/defaults.js
@@ -28,11 +28,17 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
     traceOut._length = Math.min(locations.length, z.length);
 
     var geojson = coerce('geojson');
+
     var locationmodeDflt;
     if((typeof geojson === 'string' && geojson !== '') || Lib.isPlainObject(geojson)) {
         locationmodeDflt = 'geojson-id';
     }
-    coerce('locationmode', locationmodeDflt);
+
+    var locationMode = coerce('locationmode', locationmodeDflt);
+
+    if(locationMode === 'geojson-id') {
+        coerce('featureidkey');
+    }
 
     coerce('text');
     coerce('hovertext');
diff --git a/src/traces/choroplethmapbox/attributes.js b/src/traces/choroplethmapbox/attributes.js
@@ -28,9 +28,7 @@ module.exports = extendFlat({
     // Maybe start with only one value (that we could name e.g. 'geojson-id'),
     // but eventually:
     // - we could also support for our own dist/topojson/*
-    // - some people might want `geojson-properties-name` to map data arrays to
-    //   GeoJSON features
-    // locationmode: choroplethAttrs.locationmode,
+    //   .. and locationmode: choroplethAttrs.locationmode,
 
     z: {
         valType: 'data_array',
@@ -52,6 +50,13 @@ module.exports = extendFlat({
             'with geometries of type *Polygon* and *MultiPolygon*.'
         ].join(' ')
     },
+    featureidkey: extendFlat({}, choroplethAttrs.featureidkey, {
+        description: [
+            'Sets the key in GeoJSON features which is used as id to match the items',
+            'included in the `locations` array.',
+            'Support nested property, for example *properties.name*.'
+        ].join(' ')
+    }),
 
     // TODO agree on name / behaviour
     //
diff --git a/src/traces/choroplethmapbox/defaults.js b/src/traces/choroplethmapbox/defaults.js
@@ -29,6 +29,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
         return;
     }
 
+    coerce('featureidkey');
+
     traceOut._length = Math.min(locations.length, z.length);
 
     coerce('below');
diff --git a/src/traces/scattergeo/attributes.js b/src/traces/scattergeo/attributes.js
@@ -70,6 +70,18 @@ module.exports = overrideAll({
             // https://github.com/topojson/topojson-specification/blob/master/README.md
         ].join(' ')
     },
+    featureidkey: {
+        valType: 'string',
+        role: 'info',
+        editType: 'calc',
+        dflt: 'id',
+        description: [
+            'Sets the key in GeoJSON features which is used as id to match the items',
+            'included in the `locations` array.',
+            'Only has an effect when `geojson` is set.',
+            'Support nested property, for example *properties.name*.'
+        ].join(' ')
+    },
 
     mode: extendFlat({}, scatterAttrs.mode, {dflt: 'markers'}),
 
diff --git a/src/traces/scattergeo/defaults.js b/src/traces/scattergeo/defaults.js
@@ -32,7 +32,13 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
         if((typeof geojson === 'string' && geojson !== '') || Lib.isPlainObject(geojson)) {
             locationmodeDflt = 'geojson-id';
         }
-        coerce('locationmode', locationmodeDflt);
+
+        var locationMode = coerce('locationmode', locationmodeDflt);
+
+        if(locationMode === 'geojson-id') {
+            coerce('featureidkey');
+        }
+
         len = locations.length;
     } else {
         var lon = coerce('lon') || [];
diff --git a/test/image/baselines/geo_featureidkey.png b/test/image/baselines/geo_featureidkey.png
diff --git a/test/image/baselines/mapbox_choropleth-raw-geojson.png b/test/image/baselines/mapbox_choropleth-raw-geojson.png
diff --git a/test/image/mocks/geo_featureidkey.json b/test/image/mocks/geo_featureidkey.json
@@ -0,0 +1,72 @@
+{
+  "data": [
+    {
+      "type": "scattergeo",
+      "locations": ["AL"],
+      "featureidkey": "properties.name",
+      "geojson": {
+       "type": "Feature",
+       "properties": {
+         "name": "AL"
+       },
+       "geometry": {
+         "type": "Polygon",
+         "coordinates": [[
+             [-87.359296, 35.00118], [-85.606675, 34.984749], [-85.431413, 34.124869], [-85.184951, 32.859696],
+             [-85.069935, 32.580372], [-84.960397, 32.421541], [-85.004212, 32.322956], [-84.889196, 32.262709],
+             [-85.058981, 32.13674], [-85.053504, 32.01077], [-85.141136, 31.840985], [-85.042551, 31.539753],
+             [-85.113751, 31.27686], [-85.004212, 31.003013], [-85.497137, 30.997536], [-87.600282, 30.997536],
+             [-87.633143, 30.86609], [-87.408589, 30.674397], [-87.446927, 30.510088], [-87.37025, 30.427934],
+             [-87.518128, 30.280057], [-87.655051, 30.247195], [-87.90699, 30.411504], [-87.934375, 30.657966],
+             [-88.011052, 30.685351], [-88.10416, 30.499135], [-88.137022, 30.318396], [-88.394438, 30.367688],
+             [-88.471115, 31.895754], [-88.241084, 33.796253], [-88.098683, 34.891641], [-88.202745, 34.995703],
+             [-87.359296, 35.00118]
+         ]]
+        }
+      }
+    },
+    {
+      "type": "choropleth",
+      "name": "choropleth + RAW",
+      "locations": ["AL"],
+      "featureidkey": "properties.id",
+      "z": [10],
+      "showscale": false,
+      "geojson": {
+       "type": "Feature",
+       "properties": {
+         "id": "AL"
+       },
+       "geometry": {
+         "type": "Polygon",
+         "coordinates": [[
+             [-87.359296, 35.00118], [-85.606675, 34.984749], [-85.431413, 34.124869], [-85.184951, 32.859696],
+             [-85.069935, 32.580372], [-84.960397, 32.421541], [-85.004212, 32.322956], [-84.889196, 32.262709],
+             [-85.058981, 32.13674], [-85.053504, 32.01077], [-85.141136, 31.840985], [-85.042551, 31.539753],
+             [-85.113751, 31.27686], [-85.004212, 31.003013], [-85.497137, 30.997536], [-87.600282, 30.997536],
+             [-87.633143, 30.86609], [-87.408589, 30.674397], [-87.446927, 30.510088], [-87.37025, 30.427934],
+             [-87.518128, 30.280057], [-87.655051, 30.247195], [-87.90699, 30.411504], [-87.934375, 30.657966],
+             [-88.011052, 30.685351], [-88.10416, 30.499135], [-88.137022, 30.318396], [-88.394438, 30.367688],
+             [-88.471115, 31.895754], [-88.241084, 33.796253], [-88.098683, 34.891641], [-88.202745, 34.995703],
+             [-87.359296, 35.00118]
+         ]]
+        }
+      }
+    }
+  ],
+  "layout": {
+    "geo": {
+      "center": { "lon": -86, "lat": 33 },
+      "projection": {"scale": 30}
+    },
+    "width": 600,
+    "height": 400,
+    "showlegend": false,
+    "title": {
+      "text": "Geo traces with set <b>featureidkey</b>",
+      "x": 0.1,
+      "xref": "container",
+      "xanchor": "left"
+    }
+  }
+}
diff --git a/test/image/mocks/mapbox_choropleth-raw-geojson.json b/test/image/mocks/mapbox_choropleth-raw-geojson.json
@@ -28,6 +28,20 @@
        ]]
      }
     }
+  }, {
+    "type": "choroplethmapbox",
+    "locations": ["Georgia"],
+    "z": [5],
+	"featureidkey": "properties.name",
+    "coloraxis": "coloraxis",
+    "geojson": {
+	  "type": "Feature",
+	  "properties": {"name": "Georgia"},
+	  "geometry": {
+	    "type": "Polygon",
+		"coordinates": [[[-83.109191,35.00118],[-83.322791,34.787579],[-83.339222,34.683517],[-83.005129,34.469916],[-82.901067,34.486347],[-82.747713,34.26727],[-82.714851,34.152254],[-82.55602,33.94413],[-82.325988,33.81816],[-82.194542,33.631944],[-81.926172,33.462159],[-81.937125,33.347144],[-81.761863,33.160928],[-81.493493,33.007573],[-81.42777,32.843265],[-81.416816,32.629664],[-81.279893,32.558464],[-81.121061,32.290094],[-81.115584,32.120309],[-80.885553,32.032678],[-81.132015,31.693108],[-81.175831,31.517845],[-81.279893,31.364491],[-81.290846,31.20566],[-81.400385,31.13446],[-81.444201,30.707258],[-81.718048,30.745597],[-81.948079,30.827751],[-82.041187,30.751074],[-82.002849,30.564858],[-82.046664,30.362211],[-82.167157,30.356734],[-82.216449,30.570335],[-83.498053,30.647012],[-84.867289,30.712735],[-85.004212,31.003013],[-85.113751,31.27686],[-85.042551,31.539753],[-85.141136,31.840985],[-85.053504,32.01077],[-85.058981,32.13674],[-84.889196,32.262709],[-85.004212,32.322956],[-84.960397,32.421541],[-85.069935,32.580372],[-85.184951,32.859696],[-85.431413,34.124869],[-85.606675,34.984749],[-84.319594,34.990226],[-83.618546,34.984749],[-83.109191,35.00118]]]
+      }
+    }
   }],
   "layout": {
     "width": 600,
diff --git a/test/jasmine/tests/choropleth_test.js b/test/jasmine/tests/choropleth_test.js
@@ -127,6 +127,27 @@ describe('Test choropleth', function() {
             Choropleth.supplyDefaults(traceIn, traceOut, defaultColor, layout);
             expect(traceOut.locationmode).toBe('ISO-3', 'invalid object');
         });
+
+        it('should only coerce *featureidkey* when locationmode is *geojson-id', function() {
+            traceIn = {
+                locations: ['CAN', 'USA'],
+                z: [1, 2],
+                geojson: 'url',
+                featureidkey: 'properties.name'
+            };
+            traceOut = {};
+            Choropleth.supplyDefaults(traceIn, traceOut, defaultColor, layout);
+            expect(traceOut.featureidkey).toBe('properties.name', 'coerced');
+
+            traceIn = {
+                locations: ['CAN', 'USA'],
+                z: [1, 2],
+                featureidkey: 'properties.name'
+            };
+            traceOut = {};
+            Choropleth.supplyDefaults(traceIn, traceOut, defaultColor, layout);
+            expect(traceOut.featureidkey).toBe(undefined, 'NOT coerced');
+        });
     });
 });
 
diff --git a/test/jasmine/tests/choroplethmapbox_test.js b/test/jasmine/tests/choroplethmapbox_test.js
@@ -174,8 +174,8 @@ describe('Test choroplethmapbox convert:', function() {
             });
             expectBlank(opts);
             expect(loggers.warn).toHaveBeenCalledWith([
-                'Invalid GeoJSON type none,',
-                'choroplethmapbox traces only support *FeatureCollection* and *Feature* types.'
+                'Invalid GeoJSON type none.',
+                'Traces with locationmode *geojson-id* only support *FeatureCollection* and *Feature* types.'
             ].join(' '));
         });
 
@@ -188,8 +188,8 @@ describe('Test choroplethmapbox convert:', function() {
             });
             expectBlank(opts);
             expect(loggers.warn).toHaveBeenCalledWith([
-                'Invalid GeoJSON type nop!,',
-                'choroplethmapbox traces only support *FeatureCollection* and *Feature* types.'
+                'Invalid GeoJSON type nop!.',
+                'Traces with locationmode *geojson-id* only support *FeatureCollection* and *Feature* types.'
             ].join(' '));
         });
     });
@@ -204,8 +204,8 @@ describe('Test choroplethmapbox convert:', function() {
             var opts = _convert(trace);
             expect(opts.geojson.features.length).toBe(2, '# of feature to be rendered');
             expect(loggers.log).toHaveBeenCalledWith([
-                'Location with id b does not have a valid GeoJSON geometry,',
-                'choroplethmapbox traces only support *Polygon* and *MultiPolygon* geometries.'
+                'Location b does not have a valid GeoJSON geometry.',
+                'Traces with locationmode *geojson-id* only support *Polygon* and *MultiPolygon* geometries.'
             ].join(' '));
         });
 
@@ -216,8 +216,8 @@ describe('Test choroplethmapbox convert:', function() {
             var opts = _convert(trace);
             expect(opts.geojson.features.length).toBe(2, '# of feature to be rendered');
             expect(loggers.log).toHaveBeenCalledWith([
-                'Location with id c does not have a valid GeoJSON geometry,',
-                'choroplethmapbox traces only support *Polygon* and *MultiPolygon* geometries.'
+                'Location c does not have a valid GeoJSON geometry.',
+                'Traces with locationmode *geojson-id* only support *Polygon* and *MultiPolygon* geometries.'
             ].join(' '));
         });
     });
@@ -231,7 +231,7 @@ describe('Test choroplethmapbox convert:', function() {
 
         var opts = _convert(trace);
         expect(opts.geojson.features.length).toBe(3, '# of features to be rendered');
-        expect(loggers.log).toHaveBeenCalledWith('Location with id d does not have a matching feature');
+        expect(loggers.log).toHaveBeenCalledWith('Location *d* does not have a matching feature with id-key *id*.');
     });
 
     describe('should accept numbers as *locations* items', function() {
@@ -594,7 +594,7 @@ describe('@noCI Test choroplethmapbox hover:', function() {
         desc: 'with "typeof number" locations[i] and feature id (in *name* label case)',
         patch: function() {
             var fig = Lib.extendDeep({}, require('@mocks/mapbox_choropleth-raw-geojson.json'));
-            fig.data.shift();
+            fig.data = [fig.data[1]];
             fig.data[0].locations = [100];
             fig.data[0].geojson.id = 100;
             return fig;
@@ -606,7 +606,7 @@ describe('@noCI Test choroplethmapbox hover:', function() {
         desc: 'with "typeof number" locations[i] and feature id (in *nums* label case)',
         patch: function() {
             var fig = Lib.extendDeep({}, require('@mocks/mapbox_choropleth-raw-geojson.json'));
-            fig.data.shift();
+            fig.data = [fig.data[1]];
             fig.data[0].locations = [100];
             fig.data[0].geojson.id = 100;
             fig.data[0].hoverinfo = 'location+name';
@@ -619,7 +619,7 @@ describe('@noCI Test choroplethmapbox hover:', function() {
         desc: 'with "typeof number" locations[i] and feature id (hovertemplate case)',
         patch: function() {
             var fig = Lib.extendDeep({}, require('@mocks/mapbox_choropleth-raw-geojson.json'));
-            fig.data.shift();
+            fig.data = [fig.data[1]];
             fig.data[0].locations = [100];
             fig.data[0].geojson.id = 100;
             fig.data[0].hovertemplate = '### %{location}<extra>%{location} ###</extra>';
diff --git a/test/jasmine/tests/scattergeo_test.js b/test/jasmine/tests/scattergeo_test.js
@@ -129,6 +129,25 @@ describe('Test scattergeo defaults', function() {
         ScatterGeo.supplyDefaults(traceIn, traceOut, defaultColor, layout);
         expect(traceOut.locationmode).toBe(undefined, 'lon/lat coordinates');
     });
+
+    it('should only coerce *featureidkey* when locationmode is *geojson-id', function() {
+        traceIn = {
+            locations: ['CAN', 'USA'],
+            geojson: 'url',
+            featureidkey: 'properties.name'
+        };
+        traceOut = {};
+        ScatterGeo.supplyDefaults(traceIn, traceOut, defaultColor, layout);
+        expect(traceOut.featureidkey).toBe('properties.name', 'coerced');
+
+        traceIn = {
+            locations: ['CAN', 'USA'],
+            featureidkey: 'properties.name'
+        };
+        traceOut = {};
+        ScatterGeo.supplyDefaults(traceIn, traceOut, defaultColor, layout);
+        expect(traceOut.featureidkey).toBe(undefined, 'NOT coerced');
+    });
 });
 
 describe('Test scattergeo calc', function() {

Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,8 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout`
`29`	`29`	`return;`
`30`	`30`	`}`
`31`	`31`
	`32`	`+ coerce('featureidkey');`
	`33`	`+`
`32`	`34`	`traceOut._length = Math.min(locations.length, z.length);`
`33`	`35`
`34`	`36`	`coerce('below');`