From 1401c7dbfb15afcafe53cee3906796e69b9f87fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dominik=20Schr=C3=B6der?= <dschroeder@ethz.ch>
Date: Tue, 17 Sep 2024 00:05:02 +0200
Subject: [PATCH 1/2] facet missing data bug fix

---
 examples/specs/facet_bug.vl.json      | 30 +++++++++++++++++++++++++
 examples/specs/facet_bug_cars.vl.json | 17 ++++++++++++++
 src/compile/facet.ts                  | 32 ++++++++-------------------
 3 files changed, 56 insertions(+), 23 deletions(-)
 create mode 100644 examples/specs/facet_bug.vl.json
 create mode 100644 examples/specs/facet_bug_cars.vl.json

diff --git a/examples/specs/facet_bug.vl.json b/examples/specs/facet_bug.vl.json
new file mode 100644
index 0000000000..4436384932
--- /dev/null
+++ b/examples/specs/facet_bug.vl.json
@@ -0,0 +1,30 @@
+{
+  "$schema": "https://vega.github.io/schema/vega-lite/v5.json",
+  "data": {
+    "values": [
+      { "first": "B", "second": "a", "value": "B/a"},
+      { "first": "A", "second": "a", "value": "A/a"},
+      { "first": "C", "second": "a", "value": "C/a"},
+      { "first": "B", "second": "b", "value": "B/b"},
+      { "first": "A", "second": "b", "value": "A/b"}
+    ]
+  },
+  "facet": {
+    "row": { "field": "second" },
+    "column": { "field": "first", "sort": ["B","C","A"] }
+  },
+  "spec": {
+    "height": 50,
+    "width": 50,
+    "layer": [
+      {
+        "mark": { "type": "text" },
+        "encoding": {
+          "x": { "value": 25 },
+          "y": { "value": 25 },
+          "text": { "field": "value" }
+        }
+      }
+    ]
+  }
+}
diff --git a/examples/specs/facet_bug_cars.vl.json b/examples/specs/facet_bug_cars.vl.json
new file mode 100644
index 0000000000..c43d88db61
--- /dev/null
+++ b/examples/specs/facet_bug_cars.vl.json
@@ -0,0 +1,17 @@
+{
+  "$schema": "https://vega.github.io/schema/vega-lite/v5.json",
+  "data": {"url": "data/cars.json"},
+  "facet": {
+    "column": {"field": "Origin", "sort": {"field": "Acceleration", "op": "count"}},
+    "row": {"field": "Cylinders", "sort": {"field": "Horsepower", "op": "count"}}
+  },
+  "spec": {
+      "mark": "point",
+      "encoding": {
+        "x": {"field": "Horsepower", "type": "quantitative"},
+        "y": {"field": "Acceleration", "type": "quantitative"},
+        "color": {"field": "Origin", "type": "nominal"},
+        "shape": {"field": "Cylinders", "type": "nominal"}
+      }
+  }
+}
diff --git a/src/compile/facet.ts b/src/compile/facet.ts
index 9c43e1657a..ce0a92a6ce 100644
--- a/src/compile/facet.ts
+++ b/src/compile/facet.ts
@@ -7,7 +7,7 @@ import {Config} from '../config';
 import {ExprRef, replaceExprRef} from '../expr';
 import * as log from '../log';
 import {hasDiscreteDomain} from '../scale';
-import {DEFAULT_SORT_OP, EncodingSortField, isSortField, SortOrder} from '../sort';
+import {EncodingSortField, isSortField, SortOrder} from '../sort';
 import {NormalizedFacetSpec} from '../spec';
 import {EncodingFacetMapping, FacetFieldDef, FacetMapping, isFacetMapping} from '../spec/facet';
 import {hasProperty, keys} from '../util';
@@ -294,34 +294,20 @@ export class FacetModel extends ModelWithField {
     for (const channel of FACET_CHANNELS) {
       const fieldDef = this.facet[channel];
       if (fieldDef) {
-        groupby.push(vgField(fieldDef));
-
         const {bin, sort} = fieldDef;
 
-        if (isBinning(bin)) {
-          groupby.push(vgField(fieldDef, {binSuffix: 'end'}));
-        }
-
         if (isSortField(sort)) {
-          const {field, op = DEFAULT_SORT_OP} = sort;
           const outputName = facetSortFieldName(fieldDef, sort);
-          if (row && column) {
-            // For crossed facet, use pre-calculate field as it requires a different groupby
-            // For each calculated field, apply max and assign them to the same name as
-            // all values of the same group should be the same anyway.
-            fields.push(outputName);
-            ops.push('max');
-            as.push(outputName);
-          } else {
-            fields.push(field);
-            ops.push(op);
-            as.push(outputName);
-          }
+          groupby.push(outputName);
         } else if (isArray(sort)) {
           const outputName = sortArrayIndexField(fieldDef, channel);
-          fields.push(outputName);
-          ops.push('max');
-          as.push(outputName);
+          groupby.push(outputName);
+        } else {
+          groupby.push(vgField(fieldDef));
+        }
+
+        if (isBinning(bin)) {
+          groupby.push(vgField(fieldDef, {binSuffix: 'end'}));
         }
       }
     }

From 6b207f7752521b2599d38f87afadac0a83d24329 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dominik=20Schr=C3=B6der?= <dschroeder@ethz.ch>
Date: Tue, 17 Sep 2024 07:43:16 +0200
Subject: [PATCH 2/2] update test to match updated facet.ts

---
 test/compile/facet.test.ts | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/test/compile/facet.test.ts b/test/compile/facet.test.ts
index 41b5393dc8..f75cdd87a5 100644
--- a/test/compile/facet.test.ts
+++ b/test/compile/facet.test.ts
@@ -505,10 +505,7 @@ describe('FacetModel', () => {
       const marks = model.assembleMarks();
 
       expect(marks[0].from.facet.aggregate).toEqual({
-        cross: true,
-        fields: ['median_d_by_a', 'median_e_by_b'],
-        ops: ['max', 'max'],
-        as: ['median_d_by_a', 'median_e_by_b']
+        cross: true
       });
 
       expect(marks[0].sort).toEqual({