Warn users when facet data doesn't match mark data (#1014)

* Add warning when facet data doesn't match mark data

* Adjust for tests, add warning svg

* Run prettier

* Renamed files, improved conditions for warning

* Fix typo

* Use arrayify to check the length

Co-authored-by: Mike Bostock <[email protected]>

* Update warning language used.

Co-authored-by: Mike Bostock <[email protected]>

* Remove facet_warning.svg file

* facet warning logic & wording (#1015)

* logic & wording

* remove fcet_warning.svg

* An arguably better test/example plot.

* fix my mistake (ref #1015 (review))

Not sure how to remove the warning from the yarn test log—but this doesn't seem to be blocking

* arrayify(mark.data)?.length

* facetData.length

* revert wording

Co-authored-by: Mike Bostock <[email protected]>
Co-authored-by: Philippe Rivière <[email protected]>

Author: 3 people

src/plot.js

@@ -22,7 +22,7 @@ import {position, registry as scaleRegistry} from "./scales/index.js";
 import {applyInlineStyles, maybeClassName, maybeClip, styles} from "./style.js";
 import {basic, initializer} from "./transforms/basic.js";
 import {maybeInterval} from "./transforms/interval.js";
-import {consumeWarnings} from "./warnings.js";
+import {consumeWarnings, warn} from "./warnings.js";
 
 export function plot(options = {}) {
   const {facet, style, caption, ariaLabel, ariaDescription} = options;
@@ -59,10 +59,11 @@ export function plot(options = {}) {
   let facetChannels; // e.g. {fx: {value}, fy: {value}}
   let facetsIndex; // nested array of facet indexes [[0, 1, 3, …], [2, 5, …], …]
   let facetsExclude; // lazily-constructed opposite of facetsIndex
+  let facetData;
   if (facet !== undefined) {
     const {x, y} = facet;
     if (x != null || y != null) {
-      const facetData = arrayify(facet.data);
+      facetData = arrayify(facet.data);
       if (facetData == null) throw new Error("missing facet data");
       facetChannels = {};
       if (x != null) {
@@ -99,6 +100,18 @@ export function plot(options = {}) {
     const {data, facets, channels} = mark.initialize(markFacets, facetChannels);
     applyScaleTransforms(channels, options);
     stateByMark.set(mark, {data, facets, channels});
+
+    // Warn for the common pitfall of wanting to facet mapped data.
+    if (
+      facetIndex?.length > 1 && // non-trivial faceting
+      mark.facet === "auto" && // no explicit mark facet option
+      mark.data !== facet.data && // mark not implicitly faceted (different data)
+      arrayify(mark.data)?.length === facetData.length // mark data seems parallel to facet data
+    ) {
+      warn(
+        `Warning: the ${mark.ariaLabel} mark appears to use faceted data, but isn’t faceted. The mark data has the same length as the facet data and the mark facet option is "auto", but the mark data and facet data are distinct. If this mark should be faceted, set the mark facet option to true; otherwise, suppress this warning by setting the mark facet option to false.`
+      );
+    }
   }
 
   // Initalize the scales and axes.

test/output/facetWarning.svg

@@ -0,0 +1,23 @@
+import * as Plot from "@observablehq/plot";
+import * as d3 from "d3";
+
+export default async function () {
+  const anscombe = await d3.csv("data/anscombe.csv", d3.autoType);
+  return Plot.plot({
+    grid: true,
+    inset: 10,
+    width: 960,
+    height: 240,
+    facet: {
+      data: anscombe.filter((_, i) => i % 2),
+      x: "series"
+    },
+    marks: [
+      Plot.frame(),
+      Plot.dot(
+        anscombe.filter((_, i) => i % 2),
+        {x: "x", y: "y" /* , facet: true */}
+      )
+    ]
+  });
+}

test/plots/facet-warning.js

@@ -62,6 +62,7 @@ export {default as empty} from "./empty.js";
 export {default as emptyLegend} from "./empty-legend.js";
 export {default as emptyX} from "./empty-x.js";
 export {default as energyProduction} from "./energy-production.js";
+export {default as facetWarning} from "./facet-warning.js";
 export {default as faithfulDensity} from "./faithful-density.js";
 export {default as faithfulDensity1d} from "./faithful-density-1d.js";
 export {default as figcaption} from "./figcaption.js";