fix: don't stack binned field (vega#7666)

* fix: don't stack binned field fix vega#7333 * chore: update TOC [CI] * chore: update examples [CI] Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
BradyJ27 · Oct 19, 2023 · ff985e9 · ff985e9
1 parent 850e245
commit ff985e9
Show file tree

Hide file tree

Showing 10 changed files with 180 additions and 23 deletions.
diff --git a/examples/compiled/bar_1d_binned.png b/examples/compiled/bar_1d_binned.png
diff --git a/examples/compiled/bar_1d_binned.svg b/examples/compiled/bar_1d_binned.svg
diff --git a/examples/compiled/bar_1d_binned.vg.json b/examples/compiled/bar_1d_binned.vg.json
@@ -0,0 +1,102 @@
+{
+  "$schema": "https://vega.github.io/schema/vega/v5.json",
+  "background": "white",
+  "padding": 5,
+  "width": 300,
+  "height": 50,
+  "style": "cell",
+  "data": [
+    {
+      "name": "source_0",
+      "url": "data/flights-5k.json",
+      "format": {"type": "json"},
+      "transform": [
+        {
+          "type": "extent",
+          "field": "delay",
+          "signal": "bin_maxbins_100_minstep_1_delay_extent"
+        },
+        {
+          "type": "bin",
+          "field": "delay",
+          "as": [
+            "bin_maxbins_100_minstep_1_delay",
+            "bin_maxbins_100_minstep_1_delay_end"
+          ],
+          "signal": "bin_maxbins_100_minstep_1_delay_bins",
+          "extent": {"signal": "bin_maxbins_100_minstep_1_delay_extent"},
+          "maxbins": 100,
+          "minstep": 1
+        },
+        {
+          "type": "filter",
+          "expr": "isValid(datum[\"bin_maxbins_100_minstep_1_delay\"]) && isFinite(+datum[\"bin_maxbins_100_minstep_1_delay\"])"
+        }
+      ]
+    }
+  ],
+  "marks": [
+    {
+      "name": "marks",
+      "type": "rect",
+      "style": ["bar"],
+      "from": {"data": "source_0"},
+      "encode": {
+        "update": {
+          "fill": {"value": "#4c78a8"},
+          "ariaRoleDescription": {"value": "bar"},
+          "description": {
+            "signal": "\"delay (binned): \" + (!isValid(datum[\"bin_maxbins_100_minstep_1_delay\"]) || !isFinite(+datum[\"bin_maxbins_100_minstep_1_delay\"]) ? \"null\" : format(datum[\"bin_maxbins_100_minstep_1_delay\"], \"d\") + \" – \" + format(datum[\"bin_maxbins_100_minstep_1_delay_end\"], \"d\"))"
+          },
+          "x2": [
+            {
+              "test": "!isValid(datum[\"bin_maxbins_100_minstep_1_delay\"]) || !isFinite(+datum[\"bin_maxbins_100_minstep_1_delay\"])",
+              "value": 0
+            },
+            {
+              "scale": "x",
+              "field": "bin_maxbins_100_minstep_1_delay",
+              "offset": 1
+            }
+          ],
+          "x": [
+            {
+              "test": "!isValid(datum[\"bin_maxbins_100_minstep_1_delay\"]) || !isFinite(+datum[\"bin_maxbins_100_minstep_1_delay\"])",
+              "value": 0
+            },
+            {"scale": "x", "field": "bin_maxbins_100_minstep_1_delay_end"}
+          ],
+          "y": {"value": 0},
+          "y2": {"field": {"group": "height"}}
+        }
+      }
+    }
+  ],
+  "scales": [
+    {
+      "name": "x",
+      "type": "linear",
+      "domain": {
+        "signal": "[bin_maxbins_100_minstep_1_delay_bins.start, bin_maxbins_100_minstep_1_delay_bins.stop]"
+      },
+      "range": [0, {"signal": "width"}],
+      "bins": {"signal": "bin_maxbins_100_minstep_1_delay_bins"},
+      "zero": false
+    }
+  ],
+  "axes": [
+    {
+      "scale": "x",
+      "orient": "bottom",
+      "grid": false,
+      "title": "delay",
+      "format": "d",
+      "titleAnchor": "start",
+      "labelFlush": true,
+      "labelOverlap": true,
+      "tickCount": {"signal": "ceil(width/10)"},
+      "zindex": 0
+    }
+  ],
+  "config": {"style": {"cell": {"stroke": null}}}
+}
diff --git a/examples/compiled/bar_binned_data.vg.json b/examples/compiled/bar_binned_data.vg.json
@@ -23,6 +23,14 @@
       "name": "data_0",
       "source": "source_0",
       "transform": [
+        {
+          "type": "stack",
+          "groupby": ["bin_start", "bin_start"],
+          "field": "count",
+          "sort": {"field": [], "order": []},
+          "as": ["count_start", "count_end"],
+          "offset": "zero"
+        },
         {
           "type": "filter",
           "expr": "isValid(datum[\"bin_start\"]) && isFinite(+datum[\"bin_start\"]) && isValid(datum[\"count\"]) && isFinite(+datum[\"count\"])"
@@ -45,8 +53,8 @@
           },
           "x2": {"scale": "x", "field": "bin_start", "offset": 1},
           "x": {"scale": "x", "field": "bin_end"},
-          "y": {"scale": "y", "field": "count"},
-          "y2": {"scale": "y", "value": 0}
+          "y": {"scale": "y", "field": "count_end"},
+          "y2": {"scale": "y", "field": "count_start"}
         }
       }
     }
@@ -63,7 +71,7 @@
     {
       "name": "y",
       "type": "linear",
-      "domain": {"data": "data_0", "field": "count"},
+      "domain": {"data": "data_0", "fields": ["count_start", "count_end"]},
       "range": [{"signal": "height"}, 0],
       "nice": true,
       "zero": true

diff --git a/examples/compiled/histogram_rel_freq.vg.json b/examples/compiled/histogram_rel_freq.vg.json
@@ -43,6 +43,14 @@
           "expr": "datum.Count/datum.TotalCount",
           "as": "PercentOfTotal"
         },
+        {
+          "type": "stack",
+          "groupby": ["bin_Horsepwoer", "bin_Horsepwoer"],
+          "field": "PercentOfTotal",
+          "sort": {"field": [], "order": []},
+          "as": ["PercentOfTotal_start", "PercentOfTotal_end"],
+          "offset": "zero"
+        },
         {
           "type": "filter",
           "expr": "isValid(datum[\"bin_Horsepwoer\"]) && isFinite(+datum[\"bin_Horsepwoer\"]) && isValid(datum[\"PercentOfTotal\"]) && isFinite(+datum[\"PercentOfTotal\"])"
@@ -68,8 +76,8 @@
           },
           "x2": {"scale": "x", "field": "bin_Horsepwoer", "offset": 1},
           "x": {"scale": "x", "field": "bin_Horsepwoer_end"},
-          "y": {"scale": "y", "field": "PercentOfTotal"},
-          "y2": {"scale": "y", "value": 0}
+          "y": {"scale": "y", "field": "PercentOfTotal_end"},
+          "y2": {"scale": "y", "field": "PercentOfTotal_start"}
         }
       }
     }
@@ -88,7 +96,10 @@
     {
       "name": "y",
       "type": "linear",
-      "domain": {"data": "source_0", "field": "PercentOfTotal"},
+      "domain": {
+        "data": "source_0",
+        "fields": ["PercentOfTotal_start", "PercentOfTotal_end"]
+      },
       "range": [{"signal": "height"}, 0],
       "nice": true,
       "zero": true

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> = {