fix: don't stack binned field

fix #7333
vega · Aug 29, 2021 · 36dc50a · 36dc50a
1 parent 94c50d1
commit 36dc50a
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 17 deletions.
diff --git a/examples/specs/bar_1d_binned.vl.json b/examples/specs/bar_1d_binned.vl.json
@@ -0,0 +1,16 @@
+{
+  "$schema": "https://vega.github.io/schema/vega-lite/v5.json",
+  "data": {"url": "data/flights-5k.json"},
+  "mark": {"type": "bar"},
+  "encoding": {
+    "x": {
+      "field": "delay",
+      "type": "quantitative",
+      "bin": {"maxbins": 100, "minstep": 1},
+      "axis": {"title": "delay", "titleAnchor": "start", "format": "d"}
+    }
+  },
+  "width": 300,
+  "height": 50,
+  "config": {"view": {"stroke": null}}
+}
diff --git a/src/stack.ts b/src/stack.ts
@@ -8,6 +8,7 @@ import {
   isFieldDef,
   isFieldOrDatumDef,
   PositionDatumDef,
+  PositionDef,
   PositionFieldDef,
   TypedFieldDef,
   vgField
@@ -75,6 +76,10 @@ export interface StackProperties {
 export const STACKABLE_MARKS = new Set<Mark>([ARC, BAR, AREA, RULE, POINT, CIRCLE, SQUARE, LINE, TEXT, TICK]);
 export const STACK_BY_DEFAULT_MARKS = new Set<Mark>([BAR, AREA, ARC]);
 
+function isUnbinnedQuantitative(channelDef: PositionDef<string>) {
+  return isFieldDef(channelDef) && channelDefType(channelDef) === 'quantitative' && !channelDef.bin;
+}
+
 function potentialStackedChannel(
   encoding: Encoding<string>,
   x: 'x' | 'theta'
@@ -85,7 +90,7 @@ function potentialStackedChannel(
   const yDef = encoding[y];
 
   if (isFieldDef(xDef) && isFieldDef(yDef)) {
-    if (channelDefType(xDef) === 'quantitative' && channelDefType(yDef) === 'quantitative') {
+    if (isUnbinnedQuantitative(xDef) && isUnbinnedQuantitative(yDef)) {
       if (xDef.stack) {
         return x;
       } else if (yDef.stack) {
@@ -106,14 +111,14 @@ function potentialStackedChannel(
           return x;
         }
       }
-    } else if (channelDefType(xDef) === 'quantitative') {
+    } else if (isUnbinnedQuantitative(xDef)) {
       return x;
-    } else if (channelDefType(yDef) === 'quantitative') {
+    } else if (isUnbinnedQuantitative(yDef)) {
       return y;
     }
-  } else if (channelDefType(xDef) === 'quantitative') {
+  } else if (isUnbinnedQuantitative(xDef)) {
     return x;
-  } else if (channelDefType(yDef) === 'quantitative') {
+  } else if (isUnbinnedQuantitative(yDef)) {
     return y;
   }
   return undefined;

diff --git a/test/compile/mark/bar.test.ts b/test/compile/mark/bar.test.ts
@@ -1048,8 +1048,8 @@ describe('Mark: Bar', () => {
       it('should draw bar with x and x2', () => {
         expect(props.x2).toEqual({scale: 'x', field: 'bin_start', offset: 1});
         expect(props.x).toEqual({scale: 'x', field: 'bin_end'});
-        expect(props.y).toEqual({scale: 'y', field: 'count'});
-        expect(props.y2).toEqual({scale: 'y', value: 0});
+        expect(props.y).toEqual({scale: 'y', field: 'count_end'});
+        expect(props.y2).toEqual({scale: 'y', field: 'count_start'});
         expect(props.width).toBeUndefined();
       });
     });
@@ -1080,8 +1080,8 @@ describe('Mark: Bar', () => {
       it('should draw bar with x and x2', () => {
         expect(props.x2).toEqual({scale: 'x', field: 'bin_start', offset: 5.5});
         expect(props.x).toEqual({scale: 'x', field: 'bin_end', offset: -4.5});
-        expect(props.y).toEqual({scale: 'y', field: 'count'});
-        expect(props.y2).toEqual({scale: 'y', value: 0});
+        expect(props.y).toEqual({scale: 'y', field: 'count_end'});
+        expect(props.y2).toEqual({scale: 'y', field: 'count_start'});
         expect(props.width).toBeUndefined();
       });
     });
@@ -1114,8 +1114,8 @@ describe('Mark: Bar', () => {
       it('should draw bar with y and y2', () => {
         expect(props.y2).toEqual({scale: 'y', field: 'bin_start'});
         expect(props.y).toEqual({scale: 'y', field: 'bin_end', offset: 1});
-        expect(props.x).toEqual({scale: 'x', field: 'count'});
-        expect(props.x2).toEqual({scale: 'x', value: 0});
+        expect(props.x).toEqual({scale: 'x', field: 'count_end'});
+        expect(props.x2).toEqual({scale: 'x', field: 'count_start'});
         expect(props.width).toBeUndefined();
       });
     });
@@ -1146,8 +1146,8 @@ describe('Mark: Bar', () => {
       it('should draw bar with y and y2', () => {
         expect(props.y2).toEqual({scale: 'y', field: 'bin_start', offset: -4.5});
         expect(props.y).toEqual({scale: 'y', field: 'bin_end', offset: 5.5});
-        expect(props.x).toEqual({scale: 'x', field: 'count'});
-        expect(props.x2).toEqual({scale: 'x', value: 0});
+        expect(props.x).toEqual({scale: 'x', field: 'count_end'});
+        expect(props.x2).toEqual({scale: 'x', field: 'count_start'});
         expect(props.width).toBeUndefined();
       });
     });

diff --git a/test/compile/mark/rect.test.ts b/test/compile/mark/rect.test.ts
@@ -320,8 +320,8 @@ describe('Mark: Rect', () => {
     it('should draw bar with x and x2', () => {
       expect(props.x2).toEqual({scale: 'x', field: 'bin_start', offset: 1});
       expect(props.x).toEqual({scale: 'x', field: 'bin_end'});
-      expect(props.y).toEqual({scale: 'y', field: 'count'});
-      expect(props.y2).toEqual({scale: 'y', value: 0});
+      expect(props.y).toEqual({scale: 'y', field: 'count_end'});
+      expect(props.y2).toEqual({scale: 'y', field: 'count_start'});
       expect(props.width).toBeUndefined();
     });
   });
@@ -352,8 +352,8 @@ describe('Mark: Rect', () => {
     it('should draw bar with y and y2', () => {
       expect(props.y2).toEqual({scale: 'y', field: 'bin_start'});
       expect(props.y).toEqual({scale: 'y', field: 'bin_end', offset: 1});
-      expect(props.x).toEqual({scale: 'x', field: 'count'});
-      expect(props.x2).toEqual({scale: 'x', value: 0});
+      expect(props.x).toEqual({scale: 'x', field: 'count_end'});
+      expect(props.x2).toEqual({scale: 'x', field: 'count_start'});
       expect(props.width).toBeUndefined();
     });
   });

diff --git a/test/stack.test.ts b/test/stack.test.ts
@@ -42,6 +42,20 @@ describe('stack', () => {
     }
   });
 
+  it("doesn't stacked binned field", () => {
+    for (const mark of STACKABLE_NON_POLAR_MARKS) {
+      const spec: TopLevel<NormalizedUnitSpec> = {
+        data: {url: 'data/barley.json'},
+        mark: mark,
+        encoding: {
+          x: {field: 'yield', type: 'quantitative', bin: true}
+        }
+      };
+      const stackProps = stack(spec.mark, spec.encoding, undefined);
+      expect(stackProps).toBeNull();
+    }
+  });
+
   it('should be disabled when stack is false', () => {
     for (const mark of STACKABLE_NON_POLAR_MARKS) {
       const spec: TopLevel<NormalizedUnitSpec> = {