fix: use resolve "shared" instead of steps to fix artifacts in groupe…

…d density transform (vega#9106) * Use resolve shared instead of steps * Update density tests * chore: update examples [CI] --------- Co-authored-by: GitHub Actions Bot <[email protected]>
alliefeldman · Sep 30, 2023 · e743afe · e743afe
1 parent ebe6e19
commit e743afe
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 10 deletions.
diff --git a/examples/compiled/area_density_facet.vg.json b/examples/compiled/area_density_facet.vg.json
@@ -15,7 +15,7 @@
           "groupby": ["Species"],
           "extent": [2500, 6500],
           "as": ["value", "density"],
-          "steps": 200
+          "resolve": "shared"
         },
         {
           "type": "impute",

diff --git a/examples/compiled/area_density_stacked.vg.json b/examples/compiled/area_density_stacked.vg.json
@@ -18,7 +18,7 @@
           "groupby": ["Species"],
           "extent": [2500, 6500],
           "as": ["value", "density"],
-          "steps": 200
+          "resolve": "shared"
         },
         {
           "type": "impute",

diff --git a/examples/compiled/area_density_stacked_fold.vg.json b/examples/compiled/area_density_stacked_fold.vg.json
@@ -24,7 +24,8 @@
           "extent": [10, 500],
           "counts": true,
           "steps": 200,
-          "as": ["value", "density"]
+          "as": ["value", "density"],
+          "resolve": "shared"
         },
         {
           "type": "impute",

diff --git a/src/compile/data/density.ts b/src/compile/data/density.ts
@@ -19,11 +19,6 @@ export class DensityTransformNode extends DataFlowNode {
     this.transform = duplicate(transform); // duplicate to prevent side effects
     const specifiedAs = this.transform.as ?? [undefined, undefined];
     this.transform.as = [specifiedAs[0] ?? 'value', specifiedAs[1] ?? 'density'];
-
-    // set steps when we are grouping so that we get consitent sampling points for imputing and grouping
-    if (transform.groupby && transform.minsteps == null && transform.maxsteps == null && transform.steps == null) {
-      this.transform.steps = 200;
-    }
   }
 
   public dependentFields() {
@@ -45,6 +40,9 @@ export class DensityTransformNode extends DataFlowNode {
       field: density,
       ...rest
     };
+    if (this.transform.groupby) {
+      result.resolve = 'shared';
+    }
     return result;
   }
 }
diff --git a/test/compile/data/density.test.ts b/test/compile/data/density.test.ts
@@ -27,6 +27,7 @@ describe('compile/data/fold', () => {
         extent: [0, 10],
         minsteps: 25,
         maxsteps: 200,
+        resolve: 'shared',
         as: ['x', 'y']
       });
     });
@@ -57,7 +58,7 @@ describe('compile/data/fold', () => {
       });
     });
 
-    it('should add steps if we group', () => {
+    it('should add resolve shared if we group', () => {
       const transform: Transform = {
         density: 'v',
         groupby: ['a']
@@ -67,7 +68,7 @@ describe('compile/data/fold', () => {
         type: 'kde',
         groupby: ['a'],
         field: 'v',
-        steps: 200,
+        resolve: 'shared',
         as: ['value', 'density']
       });
     });