feat: filter invalid for geojson data (#7080)

* feat: filter invalid for geojson data fixes #6163 * chore: update examples [CI] Co-authored-by: GitHub Actions Bot <[email protected]>
vega · Dec 22, 2020 · bd92548 · bd92548
1 parent 7ba0c9b
commit bd92548
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 27 deletions.
diff --git a/examples/compiled/geo_repeat.vg.json b/examples/compiled/geo_repeat.vg.json
@@ -26,6 +26,7 @@
           "fields": ["id"],
           "as": ["geo"]
         },
+        {"type": "filter", "expr": "isValid(datum[\"geo\"])"},
         {
           "type": "geojson",
           "geojson": "geo",
@@ -48,6 +49,7 @@
           "fields": ["id"],
           "as": ["geo"]
         },
+        {"type": "filter", "expr": "isValid(datum[\"geo\"])"},
         {
           "type": "geojson",
           "geojson": "geo",
@@ -70,6 +72,7 @@
           "fields": ["id"],
           "as": ["geo"]
         },
+        {"type": "filter", "expr": "isValid(datum[\"geo\"])"},
         {
           "type": "geojson",
           "geojson": "geo",

diff --git a/examples/compiled/geo_trellis.vg.json b/examples/compiled/geo_trellis.vg.json
@@ -20,6 +20,7 @@
           "fields": ["id"],
           "as": ["geo"]
         },
+        {"type": "filter", "expr": "isValid(datum[\"geo\"])"},
         {"type": "geojson", "geojson": "geo", "signal": "child_geojson_0"},
         {
           "type": "filter",

diff --git a/src/compile/data/assemble.ts b/src/compile/data/assemble.ts
@@ -81,9 +81,7 @@ function makeWalkTree(data: VgData[]) {
         node.data = dataSource.source;
       }
 
-      for (const d of node.assemble()) {
-        data.push(d);
-      }
+      data.push(...node.assemble());
 
       // break here because the rest of the tree has to be taken care of by the facet.
       return;
@@ -96,7 +94,6 @@ function makeWalkTree(data: VgData[]) {
       node instanceof FilterNode ||
       node instanceof CalculateNode ||
       node instanceof GeoPointNode ||
-      node instanceof GeoJSONNode ||
       node instanceof AggregateNode ||
       node instanceof LookupNode ||
       node instanceof WindowTransformNode ||
@@ -118,7 +115,8 @@ function makeWalkTree(data: VgData[]) {
       node instanceof BinNode ||
       node instanceof TimeUnitNode ||
       node instanceof ImputeNode ||
-      node instanceof StackNode
+      node instanceof StackNode ||
+      node instanceof GeoJSONNode
     ) {
       dataSource.transform.push(...node.assemble());
     }

diff --git a/src/compile/data/geojson.ts b/src/compile/data/geojson.ts
@@ -1,4 +1,4 @@
-import {GeoJSONTransform as VgGeoJSONTransform, Vector2} from 'vega';
+import {Transforms as VgTransform, Vector2} from 'vega';
 import {isString} from 'vega-util';
 import {GeoPositionChannel, LATITUDE, LATITUDE2, LONGITUDE, LONGITUDE2, SHAPE} from '../../channel';
 import {getFieldOrDatumDef, isDatumDef, isFieldDef, isValueDef} from '../../channeldef';
@@ -72,12 +72,22 @@ export class GeoJSONNode extends DataFlowNode {
     return `GeoJSON ${this.geojson} ${this.signal} ${hash(this.fields)}`;
   }
 
-  public assemble(): VgGeoJSONTransform {
-    return {
-      type: 'geojson',
-      ...(this.fields ? {fields: this.fields} : {}),
-      ...(this.geojson ? {geojson: this.geojson} : {}),
-      signal: this.signal
-    };
+  public assemble(): VgTransform[] {
+    return [
+      ...(this.geojson
+        ? [
+            {
+              type: 'filter',
+              expr: `isValid(datum["${this.geojson}"])`
+            } as const
+          ]
+        : []),
+      {
+        type: 'geojson',
+        ...(this.fields ? {fields: this.fields} : {}),
+        ...(this.geojson ? {geojson: this.geojson} : {}),
+        signal: this.signal
+      }
+    ];
   }
 }
diff --git a/test/compile/data/geojson.test.ts b/test/compile/data/geojson.test.ts
@@ -1,4 +1,4 @@
-import {FieldRef, Vector2} from 'vega';
+import {FieldRef, GeoJSONTransform, Vector2} from 'vega';
 import {GeoJSONNode} from '../../../src/compile/data/geojson';
 import {contains, every} from '../../../src/util';
 import {parseUnitModelWithScaleAndLayoutSize} from '../../util';
@@ -29,23 +29,52 @@ describe('compile/data/geojson', () => {
     const root = new PlaceholderDataFlowNode(null);
     GeoJSONNode.parseAll(root, model);
 
-    let node = root.children[0];
+    const node = root.children[0];
 
-    while (node != null) {
-      expect(node).toBeInstanceOf(GeoJSONNode);
-      const transform = (node as GeoJSONNode).assemble();
-      expect(transform.type).toBe('geojson');
-      expect(every(['longitude', 'latitude'], field => contains(transform.fields as Vector2<FieldRef>, field))).toBe(
-        true
-      );
-      expect(transform.geojson).not.toBeDefined();
+    expect(node).toBeInstanceOf(GeoJSONNode);
+    const transforms = (node as GeoJSONNode).assemble();
 
-      expect(node.children.length).toBeLessThanOrEqual(1);
-      node = node.children[0];
-    }
+    expect(transforms).toHaveLength(1);
+
+    const geoJSONTransform = transforms[0] as GeoJSONTransform;
+
+    expect(geoJSONTransform.type).toBe('geojson');
+    expect(
+      every(['longitude', 'latitude'], field => contains(geoJSONTransform.fields as Vector2<FieldRef>, field))
+    ).toBe(true);
+    expect(geoJSONTransform.geojson).not.toBeDefined();
+  });
+
+  it('should add filter when shape channel is used', () => {
+    const model = parseUnitModelWithScaleAndLayoutSize({
+      data: {values: []},
+      mark: 'geoshape',
+      encoding: {
+        shape: {field: 'geo', type: 'geojson'},
+        color: {field: 'state', type: 'nominal'}
+      }
+    });
+
+    const root = new PlaceholderDataFlowNode(null);
+    GeoJSONNode.parseAll(root, model);
+
+    const node = root.children[0];
+
+    expect(node).toBeInstanceOf(GeoJSONNode);
+    const transforms = (node as GeoJSONNode).assemble();
+
+    expect(transforms).toHaveLength(2);
+
+    expect(transforms[0]).toEqual({
+      type: 'filter',
+      expr: 'isValid(datum["geo"])'
+    });
+
+    expect(transforms[1].type).toBe('geojson');
+    expect(transforms[1].geojson).toBe('geo');
   });
 
-  it('should skip geojson if custom projection', () => {
+  it('should skip geojson when there is a custom projection', () => {
     const model = parseUnitModelWithScaleAndLayoutSize({
       data: {
         url: 'data/zipcodes.csv',