From eb002f0cb5585ace71f25db17a7b62fb44d6db7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Mon, 10 Oct 2022 17:08:13 -0700 Subject: [PATCH 01/34] define facets from each mark --- README.md | 15 + src/facet.js | 38 ++ src/plot.js | 228 ++++---- test/output/multiplicationTable.svg | 644 +++++++++++++++++++++++ test/output/penguinFacetAnnotated.svg | 114 ++++ test/output/penguinFacetAnnotatedX.svg | 102 ++++ test/plots/index.js | 3 + test/plots/multiplication-table.js | 29 + test/plots/penguins-facet-annotated-x.js | 29 + test/plots/penguins-facet-annotated.js | 31 ++ 10 files changed, 1112 insertions(+), 121 deletions(-) create mode 100644 src/facet.js create mode 100644 test/output/multiplicationTable.svg create mode 100644 test/output/penguinFacetAnnotated.svg create mode 100644 test/output/penguinFacetAnnotatedX.svg create mode 100644 test/plots/multiplication-table.js create mode 100644 test/plots/penguins-facet-annotated-x.js create mode 100644 test/plots/penguins-facet-annotated.js diff --git a/README.md b/README.md index 72af6a9d3e..82217ceaad 100644 --- a/README.md +++ b/README.md @@ -560,6 +560,21 @@ Plot.plot({ When the *include* or *exclude* facet mode is chosen, the mark data must be parallel to the facet data: the mark data must have the same length and order as the facet data. If the data are not parallel, then the wrong data may be shown in each facet. The default *auto* therefore requires strict equality (`===`) for safety, and using the facet data as mark data is recommended when using the *exclude* facet mode. (To construct parallel data safely, consider using [*array*.map](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map) on the facet data.) +Alternatively, facets can be defined for each mark by specifying the channel options **fx** or **fy**. + +```js +Plot.plot({ + marks: [ + Plot.dot(penguins, { + x: "culmen_length_mm", + y: "culmen_depth_mm", + fx: "sex", + fy: "island" + }) + ] +}) +``` + ## Legends Plot can generate legends for *color*, *opacity*, and *symbol* [scales](#scale-options). (An opacity scale is treated as a color scale with varying transparency.) For an inline legend, use the *scale*.**legend** option: diff --git a/src/facet.js b/src/facet.js new file mode 100644 index 0000000000..8c66b992ed --- /dev/null +++ b/src/facet.js @@ -0,0 +1,38 @@ +import {keyword, range} from "./options.js"; + +export function maybeFacet(options) { + const {fx, fy, facet = "auto"} = options; + if (fx !== undefined || fy !== undefined) return {x: fx, y: fy}; + if (facet === null || facet === false) return null; + if (facet === true) return "include"; + if (typeof facet === "string") return keyword(facet, "facet", ["auto", "include", "exclude"]); + if (facet) throw new Error(`Unsupported facet ${facet}`); +} + +// facet filter, by mark +export function filterFacets(facets, {fx, fy}, facetChannels) { + const vx = fx != null ? fx.value : facetChannels?.fx?.value; + const vy = fy != null ? fy.value : facetChannels?.fy?.value; + if (!vx && !vy) return; + const index = range(vx || vy); + return facets.map(({x, y}) => { + let I = index; + if (vx) I = I.filter((i) => facetKeyEquals(vx[i], x)); + if (vy) I = I.filter((i) => facetKeyEquals(vy[i], y)); + return I; + }); +} + +// test if a value equals a facet key +export function facetKeyEquals(a, b) { + return a instanceof Date && b instanceof Date ? +a === +b : a === b; +} + +// This must match the key structure of facets +export function facetTranslate(fx, fy) { + return fx && fy + ? ({x, y}) => `translate(${fx(x)},${fy(y)})` + : fx + ? ({x}) => `translate(${fx(x)},0)` + : ({y}) => `translate(0,${fy(y)})`; +} diff --git a/src/plot.js b/src/plot.js index badf692b7e..e3cedb8770 100644 --- a/src/plot.js +++ b/src/plot.js @@ -1,28 +1,18 @@ -import {cross, difference, groups, InternMap, select} from "d3"; +import {difference, InternMap, InternSet, select, sort} from "d3"; import {Axes, autoAxisTicks, autoScaleLabels} from "./axes.js"; import {Channel, Channels, channelDomain, valueObject} from "./channel.js"; import {Context, create} from "./context.js"; import {defined} from "./defined.js"; import {Dimensions} from "./dimensions.js"; import {Legends, exposeLegends} from "./legends.js"; -import { - arrayify, - isDomainSort, - isScaleOptions, - keyword, - map, - maybeNamed, - range, - second, - where, - yes -} from "./options.js"; +import {arrayify, isDomainSort, isScaleOptions, map, maybeNamed, range, where, yes} from "./options.js"; import {Scales, ScaleFunctions, autoScaleRange, exposeScales} from "./scales.js"; 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, warn} from "./warnings.js"; +import {facetKeyEquals, facetTranslate, maybeFacet, filterFacets} from "./facet.js"; /** @jsdoc plot */ export function plot(options = {}) { @@ -40,6 +30,10 @@ export function plot(options = {}) { // faceted - a boolean indicating whether this mark is faceted // values - an object of scaled values e.g. {x: [40, 32, …], …} const stateByMark = new Map(); + for (const mark of marks) { + if (stateByMark.has(mark)) throw new Error("duplicate mark; each mark must be unique"); + stateByMark.set(mark, {}); + } // A Map from scale name to an array of associated channels. const channelsByScale = new Map(); @@ -55,50 +49,107 @@ export function plot(options = {}) { } // Faceting! - let facets; // array of facet definitions (e.g. [["foo", [0, 1, 3, …]], …]) - let facetIndex; // index over the facet data, e.g. [0, 1, 2, 3, …] - let facetChannels; // e.g. {fx: {value}, fy: {value}} + const facetChannels = {}; // e.g. {fx: {value}, fy: {value}} let facetsIndex; // nested array of facet indexes [[0, 1, 3, …], [2, 5, …], …] + let facetIndex; // index over the facet data, e.g. [0, 1, 2, 3, …] let facetsExclude; // lazily-constructed opposite of facetsIndex - let facetData; + + // Top-level facets if (facet !== undefined) { const {x, y} = facet; if (x != null || y != null) { - facetData = arrayify(facet.data); + const facetData = arrayify(facet.data); if (facetData == null) throw new Error("missing facet data"); - facetChannels = {}; if (x != null) { const fx = Channel(facetData, {value: x, scale: "fx"}); facetChannels.fx = fx; channelsByScale.set("fx", [fx]); + facetChannels.size = fx.value.length; } if (y != null) { const fy = Channel(facetData, {value: y, scale: "fy"}); facetChannels.fy = fy; channelsByScale.set("fy", [fy]); + facetChannels.size = fy.value.length; } - facetIndex = range(facetData); - facets = facetGroups(facetIndex, facetChannels); - facetsIndex = facets.map(second); } } + // Mark-level facets + for (const mark of marks) { + const {x, y} = mark.facet ?? {}; + if (x !== undefined) { + const channels = channelsByScale.get("fx") ?? []; + channelsByScale.set("fx", channels); + const channel = Channel(mark.data, {value: x, scale: "fx"}); + stateByMark.get(mark).fx = channel; + channels.push(channel); + } + if (y !== undefined) { + const channels = channelsByScale.get("fy") ?? []; + channelsByScale.set("fy", channels); + const channel = Channel(mark.data, {value: y, scale: "fy"}); + stateByMark.get(mark).fy = channel; + channels.push(channel); + } + } + + // Create the facet scales and array of subplots + const facetScales = Scales(channelsByScale); + + let facets; + if (facetScales.fx) facets = (facets || [{}]).flatMap((d) => facetScales.fx.domain.map((x) => ({...d, x}))); + if (facetScales.fy) facets = (facets || [{}]).flatMap((d) => facetScales.fy.domain.map((y) => ({...d, y}))); + if (facets) facets.forEach((d, j) => (d.j = j)); + + // Compute the top-level facet index + if (facet !== undefined) { + facetIndex = range({length: facetChannels.size}); + const {fx, fy} = facetChannels; + facetsIndex = facets.map(({x, y}) => + facetIndex.filter((i) => (!fx || facetKeyEquals(fx.value[i], x)) && (!fy || facetKeyEquals(fy.value[i], y))) + ); + } + + // Compute facet indexes for each mark + for (const mark of marks) { + if (mark.facet === null) continue; + const state = stateByMark.get(mark); + + // top-level facet? Note: for mark.facet === "exclude", the opposite index + // is computed *after* empty facets have been determined + if (typeof mark.facet === "string") { + if (!facet || (mark.facet === "auto" && mark.data !== facet.data)) continue; + state.facetsIndex = facetsIndex; + continue; + } + + // Otherwise, compute an index for that mark’s data and facet options + if (facets) state.facetsIndex = filterFacets(facets, state, facetChannels); + } + + // A facet is empty if none of the faceted index has contents for any mark + if (facets) { + facets = facets.filter((_, j) => { + let nonFaceted = true; + for (const [, {facetsIndex}] of stateByMark) { + if (facetsIndex) { + nonFaceted = false; + if (facetsIndex?.[j].length) return true; + } + } + return nonFaceted; + }); + } + // Initialize the marks’ state. for (const mark of marks) { - if (stateByMark.has(mark)) throw new Error("duplicate mark; each mark must be unique"); - const markFacets = - facetsIndex === undefined - ? undefined - : mark.facet === "auto" - ? mark.data === facet.data - ? facetsIndex - : undefined - : mark.facet === "include" - ? facetsIndex - : mark.facet === "exclude" - ? facetsExclude || (facetsExclude = facetsIndex.map((f) => Uint32Array.from(difference(facetIndex, f)))) - : undefined; - const {data, facets, channels} = mark.initialize(markFacets, facetChannels); + let {facetsIndex} = stateByMark.get(mark); + if (mark.facet === "exclude") { + facetsIndex = + facetsExclude || (facetsExclude = facetsIndex.map((f) => Uint32Array.from(difference(facetIndex, f)))); + } + const {data, facets, channels} = mark.initialize(facetsIndex, facetChannels); applyScaleTransforms(channels, options); stateByMark.set(mark, {data, facets, channels}); @@ -107,7 +158,7 @@ export function plot(options = {}) { 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 + arrayify(mark.data)?.length === facetChannels.size // 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.` @@ -212,11 +263,14 @@ export function plot(options = {}) { if (axisX) svg.appendChild(axisX.render(null, scales, dimensions, context)); // Render (possibly faceted) marks. - if (facets !== undefined) { + if (facets) { const fyDomain = fy && fy.domain(); const fxDomain = fx && fx.domain(); - const indexByFacet = facetMap(facetChannels); - facets.forEach(([key], i) => indexByFacet.set(key, i)); + const nonEmptyFacets = new InternMap(); + facets.forEach(({x, y}) => { + if (!nonEmptyFacets.has(x)) nonEmptyFacets.set(x, new InternSet()); + nonEmptyFacets.get(x).add(y); + }); const selection = select(svg); if (fy && axes.y) { const axis1 = axes.y, @@ -233,7 +287,7 @@ export function plot(options = {}) { .enter() .append((ky, i) => (i === j ? axis1 : axis2).render( - fx && where(fxDomain, (kx) => indexByFacet.has([kx, ky])), + fx && where(fxDomain, (kx) => nonEmptyFacets.get(kx)?.has(ky)), scales, {...dimensions, ...fyMargins, offsetTop: fy(ky)}, context @@ -252,7 +306,7 @@ export function plot(options = {}) { .enter() .append((kx, i) => (i === j ? axis1 : axis2).render( - fy && where(fyDomain, (ky) => indexByFacet.has([kx, ky])), + fy && where(fyDomain, (ky) => nonEmptyFacets.get(kx)?.has(ky)), scales, { ...dimensions, @@ -265,17 +319,23 @@ export function plot(options = {}) { ) ); } + const fxI = fx && new Map(fx.domain().map((x, i) => [x, i])); + const fyI = fy && new Map(fy.domain().map((y, i) => [y, i])); selection .selectAll() - .data(facetKeys(scales).filter(indexByFacet.has, indexByFacet)) + .data( + sort( + facets, + ({x: xa, y: ya}, {x: xb, y: yb}) => (fxI && fxI.get(xa) - fxI.get(xb)) || (fyI && fyI.get(ya) - fyI.get(yb)) + ) + ) .enter() .append("g") .attr("aria-label", "facet") .attr("transform", facetTranslate(fx, fy)) .each(function (key) { - const j = indexByFacet.get(key); for (const [mark, {channels, values, facets}] of stateByMark) { - const facet = facets ? mark.filter(facets[j] ?? facets[0], channels, values) : null; + const facet = facets ? mark.filter(facets[key.j] ?? facets[0], channels, values) : null; const node = mark.render(facet, scales, values, subdimensions, context); if (node != null) this.appendChild(node); } @@ -326,15 +386,12 @@ export function plot(options = {}) { export class Mark { constructor(data, channels = {}, options = {}, defaults) { - const {facet = "auto", sort, dx, dy, clip, channels: extraChannels} = options; + const {sort, dx, dy, clip, channels: extraChannels} = options; this.data = data; this.sort = isDomainSort(sort) ? sort : null; this.initializer = initializer(options).initializer; this.transform = this.initializer ? options.transform : basic(options).transform; - this.facet = - facet == null || facet === false - ? null - : keyword(facet === true ? "include" : facet, "facet", ["auto", "include", "exclude"]); + this.facet = this.facet = maybeFacet(options); channels = maybeNamed(channels); if (extraChannels !== undefined) channels = {...maybeNamed(extraChannels), ...channels}; if (defaults !== undefined) channels = {...styles(this, options, defaults), ...channels}; @@ -464,74 +521,3 @@ function nolabel(axis) { ? axis // use the existing axis if unlabeled : Object.assign(Object.create(axis), {label: undefined}); } - -// Unlike facetGroups, which returns groups in order of input data, this returns -// keys in order of the associated scale’s domains. -function facetKeys({fx, fy}) { - return fx && fy ? cross(fx.domain(), fy.domain()) : fx ? fx.domain() : fy.domain(); -} - -// Returns an array of [[key1, index1], [key2, index2], …] representing the data -// indexes associated with each facet. For two-dimensional faceting, each key -// is a two-element array; see also facetMap. -function facetGroups(index, {fx, fy}) { - return fx && fy ? facetGroup2(index, fx, fy) : fx ? facetGroup1(index, fx) : facetGroup1(index, fy); -} - -function facetGroup1(index, {value: F}) { - return groups(index, (i) => F[i]); -} - -function facetGroup2(index, {value: FX}, {value: FY}) { - return groups( - index, - (i) => FX[i], - (i) => FY[i] - ).flatMap(([x, xgroup]) => xgroup.map(([y, ygroup]) => [[x, y], ygroup])); -} - -// This must match the key structure returned by facetGroups. -function facetTranslate(fx, fy) { - return fx && fy - ? ([kx, ky]) => `translate(${fx(kx)},${fy(ky)})` - : fx - ? (kx) => `translate(${fx(kx)},0)` - : (ky) => `translate(0,${fy(ky)})`; -} - -function facetMap({fx, fy}) { - return new (fx && fy ? FacetMap2 : FacetMap)(); -} - -class FacetMap { - constructor() { - this._ = new InternMap(); - } - has(key) { - return this._.has(key); - } - get(key) { - return this._.get(key); - } - set(key, value) { - return this._.set(key, value), this; - } -} - -// A Map-like interface that supports paired keys. -class FacetMap2 extends FacetMap { - has([key1, key2]) { - const map = super.get(key1); - return map ? map.has(key2) : false; - } - get([key1, key2]) { - const map = super.get(key1); - return map && map.get(key2); - } - set([key1, key2], value) { - const map = super.get(key1); - if (map) map.set(key2, value); - else super.set(key1, new InternMap([[key2, value]])); - return this; - } -} diff --git a/test/output/multiplicationTable.svg b/test/output/multiplicationTable.svg new file mode 100644 index 0000000000..9ea64d61a8 --- /dev/null +++ b/test/output/multiplicationTable.svg @@ -0,0 +1,644 @@ + + + + + 2 + + + 3 + + + 4 + + + 5 + + + 6 + + + 7 + + + 8 + + + 9 + + + + + 2 + + + 3 + + + 4 + + + 5 + + + 6 + + + 7 + + + 8 + + + 9 + + + + + + + + + + 4 + + + + + + + + + 6 + + + + + + + + + 8 + + + + + + + + + 10 + + + + + + + + + 12 + + + + + + + + + 14 + + + + + + + + + 16 + + + + + + + + + 18 + + + + + + + + + 6 + + + + + + + + + 9 + + + + + + + + + 12 + + + + + + + + + 15 + + + + + + + + + 18 + + + + + + + + + 21 + + + + + + + + + 24 + + + + + + + + + 27 + + + + + + + + + 8 + + + + + + + + + 12 + + + + + + + + + 16 + + + + + + + + + 20 + + + + + + + + + 24 + + + + + + + + + 28 + + + + + + + + + 32 + + + + + + + + + 36 + + + + + + + + + 10 + + + + + + + + + 15 + + + + + + + + + 20 + + + + + + + + + 25 + + + + + + + + + 30 + + + + + + + + + 35 + + + + + + + + + 40 + + + + + + + + + 45 + + + + + + + + + 12 + + + + + + + + + 18 + + + + + + + + + 24 + + + + + + + + + 30 + + + + + + + + + 36 + + + + + + + + + 42 + + + + + + + + + 48 + + + + + + + + + 54 + + + + + + + + + 14 + + + + + + + + + 21 + + + + + + + + + 28 + + + + + + + + + 35 + + + + + + + + + 42 + + + + + + + + + 49 + + + + + + + + + 56 + + + + + + + + + 63 + + + + + + + + + 16 + + + + + + + + + 24 + + + + + + + + + 32 + + + + + + + + + 40 + + + + + + + + + 48 + + + + + + + + + 56 + + + + + + + + + 64 + + + + + + + + + 72 + + + + + + + + + 18 + + + + + + + + + 27 + + + + + + + + + 36 + + + + + + + + + 45 + + + + + + + + + 54 + + + + + + + + + 63 + + + + + + + + + 72 + + + + + + + + + 81 + + \ No newline at end of file diff --git a/test/output/penguinFacetAnnotated.svg b/test/output/penguinFacetAnnotated.svg new file mode 100644 index 0000000000..a64c066a24 --- /dev/null +++ b/test/output/penguinFacetAnnotated.svg @@ -0,0 +1,114 @@ + + + + + Biscoe + + + Dream + + + Torgersen + island + + + + 0 + + + 20 + + + 40 + + + 60 + + + 80 + + + 100 + + + 120 + Frequency → + + + + Adelie + + + Chinstrap + + + Gentoo + + + + + Adelie + + + Chinstrap + + + Gentoo + species + + + + Adelie + + + Chinstrap + + + Gentoo + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + ↞ Torgersen Island only has Adelie penguins! + + \ No newline at end of file diff --git a/test/output/penguinFacetAnnotatedX.svg b/test/output/penguinFacetAnnotatedX.svg new file mode 100644 index 0000000000..b60f23d573 --- /dev/null +++ b/test/output/penguinFacetAnnotatedX.svg @@ -0,0 +1,102 @@ + + + + + Adelie + + + Chinstrap + + + Gentoo + species + + + + Biscoe + + + Dream + + + Torgersen + island + + + + 0 + + + 50 + + + 100 + + + + + 0 + + + 50 + + + 100 + + + + + 0 + + + 50 + + + 100 + Frequency → + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Torgersen Islandonly hasAdeliepenguins! + + \ No newline at end of file diff --git a/test/plots/index.js b/test/plots/index.js index fbf646fdf8..b1d66361e9 100644 --- a/test/plots/index.js +++ b/test/plots/index.js @@ -137,6 +137,7 @@ export {default as morleyBoxplot} from "./morley-boxplot.js"; export {default as moviesProfitByGenre} from "./movies-profit-by-genre.js"; export {default as moviesRatingByGenre} from "./movies-rating-by-genre.js"; export {default as musicRevenue} from "./music-revenue.js"; +export {default as multiplicationTable} from "./multiplication-table.js"; export {default as ordinalBar} from "./ordinal-bar.js"; export {default as penguinAnnotated} from "./penguin-annotated.js"; export {default as penguinCulmen} from "./penguin-culmen.js"; @@ -152,6 +153,8 @@ export {default as penguinDensityZ} from "./penguin-density-z.js"; export {default as penguinDodge} from "./penguin-dodge.js"; export {default as penguinDodgeHexbin} from "./penguin-dodge-hexbin.js"; export {default as penguinDodgeVoronoi} from "./penguin-dodge-voronoi.js"; +export {default as penguinFacetAnnotated} from "./penguins-facet-annotated.js"; +export {default as penguinFacetAnnotatedX} from "./penguins-facet-annotated-x.js"; export {default as penguinFacetDodge} from "./penguin-facet-dodge.js"; export {default as penguinFacetDodgeIdentity} from "./penguin-facet-dodge-identity.js"; export {default as penguinFacetDodgeIsland} from "./penguin-facet-dodge-island.js"; diff --git a/test/plots/multiplication-table.js b/test/plots/multiplication-table.js new file mode 100644 index 0000000000..39bb35f0eb --- /dev/null +++ b/test/plots/multiplication-table.js @@ -0,0 +1,29 @@ +import * as Plot from "@observablehq/plot"; +import * as d3 from "d3"; + +export default async function () { + const nums = d3.range(2, 10); + return Plot.plot({ + height: 450, + width: 450, + color: {type: "ordinal", scheme: "tableau10"}, + fx: {axis: "top"}, + marks: [ + Plot.rect(nums, {fy: nums, fill: nums}), + Plot.dot(nums, { + frameAnchor: "middle", + r: 19, + fill: nums, + stroke: "white", + fx: nums + }), + Plot.text(d3.cross(nums, nums), { + frameAnchor: "middle", + text: ([a, b]) => a * b, + fill: "white", + fx: (d) => d[1], + fy: (d) => d[0] + }) + ] + }); +} diff --git a/test/plots/penguins-facet-annotated-x.js b/test/plots/penguins-facet-annotated-x.js new file mode 100644 index 0000000000..2c955729f1 --- /dev/null +++ b/test/plots/penguins-facet-annotated-x.js @@ -0,0 +1,29 @@ +import * as Plot from "@observablehq/plot"; +import * as d3 from "d3"; + +export default async function () { + const penguins = await d3.csv("data/penguins.csv", d3.autoType); + return Plot.plot({ + marginLeft: 75, + x: {insetRight: 10}, + marks: [ + Plot.frame(), + Plot.barX( + penguins, + Plot.groupY( + {x: "count"}, + { + y: "species", + fill: "sex", + fx: "island" + } + ) + ), + Plot.text(["Torgersen Island\nonly has\nAdelie\npenguins!"], { + frameAnchor: "right", + dx: -10, + fx: ["Torgersen"] + }) + ] + }); +} diff --git a/test/plots/penguins-facet-annotated.js b/test/plots/penguins-facet-annotated.js new file mode 100644 index 0000000000..a8cd77d0c0 --- /dev/null +++ b/test/plots/penguins-facet-annotated.js @@ -0,0 +1,31 @@ +import * as Plot from "@observablehq/plot"; +import * as d3 from "d3"; + +export default async function () { + const penguins = await d3.csv("data/penguins.csv", d3.autoType); + return Plot.plot({ + marginLeft: 75, + marginRight: 70, + x: {insetRight: 10}, + facet: {marginRight: 70}, + marks: [ + Plot.frame(), + Plot.barX( + penguins, + Plot.groupY( + {x: "count"}, + { + y: "species", + fill: "sex", + fy: "island" + } + ) + ), + Plot.text(["↞ Torgersen Island only has Adelie penguins!"], { + frameAnchor: "top-right", + dx: -10, + fy: ["Torgersen"] + }) + ] + }); +} From 1c282d7b900d6e00f516ee19406ab49d2e852fe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Tue, 11 Oct 2022 16:17:18 -0700 Subject: [PATCH 02/34] allow facet: "exclude" in mark faceting this changes one unit test as it does not maintain the data order in the exclude facet. but that order was kinda arbitrary anyway. Use an explicit sort transform if necessary. --- src/facet.js | 8 +- src/plot.js | 82 +- test/output/penguinCulmen.svg | 2134 ++++++++++++++++----------------- 3 files changed, 1119 insertions(+), 1105 deletions(-) diff --git a/src/facet.js b/src/facet.js index 8c66b992ed..865ab25ea4 100644 --- a/src/facet.js +++ b/src/facet.js @@ -1,11 +1,9 @@ import {keyword, range} from "./options.js"; -export function maybeFacet(options) { - const {fx, fy, facet = "auto"} = options; - if (fx !== undefined || fy !== undefined) return {x: fx, y: fy}; +export function maybeFacet({fx, fy, facet = "auto"} = {}) { if (facet === null || facet === false) return null; - if (facet === true) return "include"; - if (typeof facet === "string") return keyword(facet, "facet", ["auto", "include", "exclude"]); + if (facet === true) facet = "include"; + if (typeof facet === "string") return {x: fx, y: fy, method: keyword(facet, "facet", ["auto", "include", "exclude"])}; if (facet) throw new Error(`Unsupported facet ${facet}`); } diff --git a/src/plot.js b/src/plot.js index e3cedb8770..ad73e5ebcb 100644 --- a/src/plot.js +++ b/src/plot.js @@ -1,4 +1,4 @@ -import {difference, InternMap, InternSet, select, sort} from "d3"; +import {InternMap, InternSet, select, sort, sum} from "d3"; import {Axes, autoAxisTicks, autoScaleLabels} from "./axes.js"; import {Channel, Channels, channelDomain, valueObject} from "./channel.js"; import {Context, create} from "./context.js"; @@ -51,8 +51,7 @@ export function plot(options = {}) { // Faceting! const facetChannels = {}; // e.g. {fx: {value}, fy: {value}} let facetsIndex; // nested array of facet indexes [[0, 1, 3, …], [2, 5, …], …] - let facetIndex; // index over the facet data, e.g. [0, 1, 2, 3, …] - let facetsExclude; // lazily-constructed opposite of facetsIndex + let facetDataIndex; // index over the facet data, e.g. [0, 1, 2, 3, …] // Top-level facets if (facet !== undefined) { @@ -64,28 +63,28 @@ export function plot(options = {}) { const fx = Channel(facetData, {value: x, scale: "fx"}); facetChannels.fx = fx; channelsByScale.set("fx", [fx]); - facetChannels.size = fx.value.length; + facetDataIndex = range(facetData); } if (y != null) { const fy = Channel(facetData, {value: y, scale: "fy"}); facetChannels.fy = fy; channelsByScale.set("fy", [fy]); - facetChannels.size = fy.value.length; + facetDataIndex = facetDataIndex || range(facetData); } } } // Mark-level facets for (const mark of marks) { - const {x, y} = mark.facet ?? {}; - if (x !== undefined) { + const {x, y, method} = mark.facet ?? {}; + if (method && x !== undefined) { const channels = channelsByScale.get("fx") ?? []; channelsByScale.set("fx", channels); const channel = Channel(mark.data, {value: x, scale: "fx"}); stateByMark.get(mark).fx = channel; channels.push(channel); } - if (y !== undefined) { + if (method && y !== undefined) { const channels = channelsByScale.get("fy") ?? []; channelsByScale.set("fy", channels); const channel = Channel(mark.data, {value: y, scale: "fy"}); @@ -101,36 +100,40 @@ export function plot(options = {}) { if (facetScales.fx) facets = (facets || [{}]).flatMap((d) => facetScales.fx.domain.map((x) => ({...d, x}))); if (facetScales.fy) facets = (facets || [{}]).flatMap((d) => facetScales.fy.domain.map((y) => ({...d, y}))); if (facets) facets.forEach((d, j) => (d.j = j)); + const facetLength = facets && facets.length; // Compute the top-level facet index - if (facet !== undefined) { - facetIndex = range({length: facetChannels.size}); + if (facetDataIndex) { const {fx, fy} = facetChannels; - facetsIndex = facets.map(({x, y}) => - facetIndex.filter((i) => (!fx || facetKeyEquals(fx.value[i], x)) && (!fy || facetKeyEquals(fy.value[i], y))) - ); + facetsIndex = []; + for (const {x, y} of facets) + facetsIndex.push( + facetDataIndex.filter((i) => (!fx || facetKeyEquals(fx.value[i], x)) && (!fy || facetKeyEquals(fy.value[i], y))) + ); } - // Compute facet indexes for each mark + // Compute a facet index for each mark for (const mark of marks) { if (mark.facet === null) continue; const state = stateByMark.get(mark); - // top-level facet? Note: for mark.facet === "exclude", the opposite index - // is computed *after* empty facets have been determined - if (typeof mark.facet === "string") { - if (!facet || (mark.facet === "auto" && mark.data !== facet.data)) continue; - state.facetsIndex = facetsIndex; + const {x, y, method} = mark.facet; + // Mark-level facet ? Compute an index for that mark’s data and options + if (x !== undefined || y !== undefined) { + if (facets) state.facetsIndex = filterFacets(facets, state, facetChannels); continue; } - // Otherwise, compute an index for that mark’s data and facet options - if (facets) state.facetsIndex = filterFacets(facets, state, facetChannels); + // Otherwise, it's a top-level facet. Note: for mark.facet === "exclude", + // the opposite index is computed *after* empty facets have been determined + if (!facet || (method === "auto" && mark.data !== facet.data)) continue; + state.facetsIndex = facetsIndex; } // A facet is empty if none of the faceted index has contents for any mark - if (facets) { - facets = facets.filter((_, j) => { + facets = + facets && + facets.filter((_, j) => { let nonFaceted = true; for (const [, {facetsIndex}] of stateByMark) { if (facetsIndex) { @@ -140,25 +143,21 @@ export function plot(options = {}) { } return nonFaceted; }); - } // Initialize the marks’ state. for (const mark of marks) { let {facetsIndex} = stateByMark.get(mark); - if (mark.facet === "exclude") { - facetsIndex = - facetsExclude || (facetsExclude = facetsIndex.map((f) => Uint32Array.from(difference(facetIndex, f)))); - } + if (mark.facet?.method === "exclude") facetsIndex = excludeIndex(facetsIndex); const {data, facets, channels} = mark.initialize(facetsIndex, 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 === facetChannels.size // mark data seems parallel to facet data + facetLength > 1 && // non-trivial faceting + mark.facet?.method === "auto" && // no explicit mark facet option + mark.data !== facet?.data && // mark not implicitly faceted (different data) + arrayify(mark.data)?.length === facetDataIndex?.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.` @@ -391,7 +390,7 @@ export class Mark { this.sort = isDomainSort(sort) ? sort : null; this.initializer = initializer(options).initializer; this.transform = this.initializer ? options.transform : basic(options).transform; - this.facet = this.facet = maybeFacet(options); + this.facet = maybeFacet(options); channels = maybeNamed(channels); if (extraChannels !== undefined) channels = {...maybeNamed(extraChannels), ...channels}; if (defaults !== undefined) channels = {...styles(this, options, defaults), ...channels}; @@ -521,3 +520,20 @@ function nolabel(axis) { ? axis // use the existing axis if unlabeled : Object.assign(Object.create(axis), {label: undefined}); } + +// Returns an index that for each facet lists all the elements present in other +// facets in the original index +function excludeIndex(index) { + const ex = []; + const e = new Uint32Array(sum(index, (d) => d.length)); + for (const i of index) { + let n = 0; + for (const j of index) { + if (i === j) continue; + e.set(j, n); + n += j.length; + } + ex.push(e.slice(0, n)); + } + return ex; +} diff --git a/test/output/penguinCulmen.svg b/test/output/penguinCulmen.svg index ea4102a7dc..02ae4c1549 100644 --- a/test/output/penguinCulmen.svg +++ b/test/output/penguinCulmen.svg @@ -134,13 +134,101 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - @@ -159,7 +247,6 @@ - @@ -212,197 +299,110 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + @@ -483,156 +483,209 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -668,129 +721,76 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + @@ -832,225 +832,220 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - @@ -1064,7 +1059,6 @@ - @@ -1085,7 +1079,6 @@ - @@ -1105,17 +1098,24 @@ - - + + + + + + + + + @@ -1185,10 +1185,6 @@ - - - - @@ -1206,7 +1202,6 @@ - @@ -1260,196 +1255,201 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + @@ -1530,157 +1530,79 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -1716,128 +1638,206 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - - + + + + + + + + + @@ -1879,224 +1879,112 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -2111,7 +1999,6 @@ - @@ -2131,7 +2018,6 @@ - @@ -2150,16 +2036,130 @@ - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -2228,693 +2228,693 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From b4aa8b5caa9a397d912fc23b199031ccc2b0474a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Tue, 11 Oct 2022 16:34:45 -0700 Subject: [PATCH 03/34] a bit more documentation --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 82217ceaad..b61d009033 100644 --- a/README.md +++ b/README.md @@ -560,7 +560,7 @@ Plot.plot({ When the *include* or *exclude* facet mode is chosen, the mark data must be parallel to the facet data: the mark data must have the same length and order as the facet data. If the data are not parallel, then the wrong data may be shown in each facet. The default *auto* therefore requires strict equality (`===`) for safety, and using the facet data as mark data is recommended when using the *exclude* facet mode. (To construct parallel data safely, consider using [*array*.map](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map) on the facet data.) -Alternatively, facets can be defined for each mark by specifying the channel options **fx** or **fy**. +Alternatively, facets can be defined for each individual mark by specifying the channel options **fx** or **fy**. In that case, the **facet** option only considers the mark data, and the default *auto* setting is equivalent to *include*. Other values of the *facet* option are unchanged: null or false disable faceting, and *exclude* draws the subset of the mark’s data *not* in the current facet. ```js Plot.plot({ From 1f597e9cb986f5ea223b9dacaef844ee531513ce Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Wed, 12 Oct 2022 10:06:26 -0700 Subject: [PATCH 04/34] unnecessarily polish multiplication table test --- test/output/multiplicationTable.svg | 558 ++++++++++++++-------------- test/plots/multiplication-table.js | 39 +- 2 files changed, 306 insertions(+), 291 deletions(-) diff --git a/test/output/multiplicationTable.svg b/test/output/multiplicationTable.svg index 9ea64d61a8..62c9632384 100644 --- a/test/output/multiplicationTable.svg +++ b/test/output/multiplicationTable.svg @@ -15,630 +15,630 @@ - 2 + 2 - 3 + 3 - 4 + 4 - 5 + 5 - 6 + 6 - 7 + 7 - 8 + 8 - 9 + 9 - - 2 + + 2 - - 3 + + 3 - - 4 + + 4 - - 5 + + 5 - 6 + 6 - - 7 + + 7 - - 8 + + 8 - - 9 + + 9 - + - + - + - 4 + 4 - + - + - + - 6 + 6 - + - + - + - 8 + 8 - + - + - + - 10 + 10 - + - + - + - 12 + 12 - + - + - + - 14 + 14 - + - + - + - 16 + 16 - + - + - + - 18 + 18 - + - + - + - 6 + 6 - + - + - + - 9 + 9 - + - + - + - 12 + 12 - + - + - + - 15 + 15 - + - + - + - 18 + 18 - + - + - + - 21 + 21 - + - + - + - 24 + 24 - + - + - + - 27 + 27 - + - + - + - 8 + 8 - + - + - + - 12 + 12 - + - + - + - 16 + 16 - + - + - + - 20 + 20 - + - + - + - 24 + 24 - + - + - + - 28 + 28 - + - + - + - 32 + 32 - + - + - + - 36 + 36 - + - + - + - 10 + 10 - + - + - + - 15 + 15 - + - + - + - 20 + 20 - + - + - + - 25 + 25 - + - + - + - 30 + 30 - + - + - + - 35 + 35 - + - + - + - 40 + 40 - + - + - + - 45 + 45 - + - + - + - 12 + 12 - + - + - + - 18 + 18 - + - + - + - 24 + 24 - + - + - + - 30 + 30 - + - + - + - 36 + 36 - + - + - + - 42 + 42 - + - + - + - 48 + 48 - + - + - + - 54 + 54 - + - + - + - 14 + 14 - + - + - + - 21 + 21 - + - + - + - 28 + 28 - + - + - + - 35 + 35 - + - + - + - 42 + 42 - + - + - + - 49 + 49 - + - + - + - 56 + 56 - + - + - + - 63 + 63 - + - + - + - 16 + 16 - + - + - + - 24 + 24 - + - + - + - 32 + 32 - + - + - + - 40 + 40 - + - + - + - 48 + 48 - + - + - + - 56 + 56 - + - + - + - 64 + 64 - + - + - + - 72 + 72 - + - + - + - 18 + 18 - + - + - + - 27 + 27 - + - + - + - 36 + 36 - + - + - + - 45 + 45 - + - + - + - 54 + 54 - + - + - + - 63 + 63 - + - + - + - 72 + 72 - + - + - + - 81 + 81 \ No newline at end of file diff --git a/test/plots/multiplication-table.js b/test/plots/multiplication-table.js index 39bb35f0eb..23a227c590 100644 --- a/test/plots/multiplication-table.js +++ b/test/plots/multiplication-table.js @@ -2,27 +2,42 @@ import * as Plot from "@observablehq/plot"; import * as d3 from "d3"; export default async function () { - const nums = d3.range(2, 10); + const numbers = d3.range(2, 10); return Plot.plot({ height: 450, width: 450, - color: {type: "ordinal", scheme: "tableau10"}, - fx: {axis: "top"}, + padding: 0, + color: {type: "categorical"}, + fx: {axis: "top", tickSize: 6}, + fy: {tickSize: 6}, marks: [ - Plot.rect(nums, {fy: nums, fill: nums}), - Plot.dot(nums, { + // This rect is faceted by y and repeated across x, and hence all rects in + // a row have the same fill. With rect, the default definitions of x1, x2, + // y1, and y2 will fill the entire frame, similar to Plot.frame. + Plot.rect(numbers, { + fy: numbers, + fill: numbers, + inset: 1 + }), + // This dot is faceted by x and repeated across y, and hence all dots in a + // column have the same fill. With dot, the default definitions of x and y + // would assume that the data is a tuple [x, y], so we set the frameAnchor + // to middle to draw one dot in the center of each frame. + Plot.dot(numbers, { frameAnchor: "middle", r: 19, - fill: nums, - stroke: "white", - fx: nums + fx: numbers, + fill: numbers, + stroke: "white" }), - Plot.text(d3.cross(nums, nums), { + // This text is faceted by x and y, and hence we need the cross product of + // the numbers. Again there is just one text mark per facet. + Plot.text(d3.cross(numbers, numbers), { frameAnchor: "middle", - text: ([a, b]) => a * b, + text: ([x, y]) => x * y, fill: "white", - fx: (d) => d[1], - fy: (d) => d[0] + fx: ([x]) => x, + fy: ([, y]) => y }) ] }); From 109f6ec437cfbc5eff2eeb4fff4b6ca5309fa168 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Wed, 12 Oct 2022 10:12:31 -0700 Subject: [PATCH 05/34] stricter/looser facet validation --- src/facet.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/facet.js b/src/facet.js index 865ab25ea4..308417a3c6 100644 --- a/src/facet.js +++ b/src/facet.js @@ -3,8 +3,7 @@ import {keyword, range} from "./options.js"; export function maybeFacet({fx, fy, facet = "auto"} = {}) { if (facet === null || facet === false) return null; if (facet === true) facet = "include"; - if (typeof facet === "string") return {x: fx, y: fy, method: keyword(facet, "facet", ["auto", "include", "exclude"])}; - if (facet) throw new Error(`Unsupported facet ${facet}`); + return {x: fx, y: fy, method: keyword(facet, "facet", ["auto", "include", "exclude"])}; } // facet filter, by mark From 14273d87fe10de47e5952362ed9ced74b027cdc2 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Wed, 12 Oct 2022 10:57:35 -0700 Subject: [PATCH 06/34] add TODO --- src/plot.js | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/src/plot.js b/src/plot.js index ad73e5ebcb..81198dbd1d 100644 --- a/src/plot.js +++ b/src/plot.js @@ -1,4 +1,4 @@ -import {InternMap, InternSet, select, sort, sum} from "d3"; +import {InternMap, InternSet, cross, select, sort, sum} from "d3"; import {Axes, autoAxisTicks, autoScaleLabels} from "./axes.js"; import {Channel, Channels, channelDomain, valueObject} from "./channel.js"; import {Context, create} from "./context.js"; @@ -96,28 +96,35 @@ export function plot(options = {}) { // Create the facet scales and array of subplots const facetScales = Scales(channelsByScale); - let facets; - if (facetScales.fx) facets = (facets || [{}]).flatMap((d) => facetScales.fx.domain.map((x) => ({...d, x}))); - if (facetScales.fy) facets = (facets || [{}]).flatMap((d) => facetScales.fy.domain.map((y) => ({...d, y}))); - if (facets) facets.forEach((d, j) => (d.j = j)); + // TODO Avoid j here? TODO Document this data structure. Note that empty is + // mutated below after we’ve computed the per-mark facet channels. + let facets = + facetScales.fx && facetScales.fy + ? cross(facetScales.fx.domain, facetScales.fy.domain).map(([x, y], j) => ({x, y, j, empty: true})) + : facetScales.fx + ? facetScales.fx.domain.map((x, j) => ({x, j, empty: true})) + : facetScales.fy + ? facetScales.fy.domain.map((y, j) => ({y, j, empty: true})) + : null; const facetLength = facets && facets.length; // Compute the top-level facet index if (facetDataIndex) { const {fx, fy} = facetChannels; facetsIndex = []; - for (const {x, y} of facets) + for (const {x, y} of facets) { facetsIndex.push( facetDataIndex.filter((i) => (!fx || facetKeyEquals(fx.value[i], x)) && (!fy || facetKeyEquals(fy.value[i], y))) ); + } } // Compute a facet index for each mark for (const mark of marks) { if (mark.facet === null) continue; const state = stateByMark.get(mark); - const {x, y, method} = mark.facet; + // Mark-level facet ? Compute an index for that mark’s data and options if (x !== undefined || y !== undefined) { if (facets) state.facetsIndex = filterFacets(facets, state, facetChannels); @@ -130,12 +137,30 @@ export function plot(options = {}) { state.facetsIndex = facetsIndex; } - // A facet is empty if none of the faceted index has contents for any mark + // When faceting by both x and y (i.e., when both fx and fy scales are + // present), then the cross product of the domains of fx and fy can include + // fx-fy combinations for which no mark has an instance associated with that + // combination of fx and fy, and therefore we don’t want to render this facet + // (not even the frame). The same can occur if you specify the domain of fx + // and fy explicitly, but there is no mark instance associated with some + // values in the domain. + // + // TODO We need to do two (or three?) passes here. First, we need determine + // the domains of the fx and fy scales (as needed) based on the union of + // distinct channel values, including both mark-level facets and top-level + // facets. Then we need to check whether we have any mark instances (or + // top-level facet data “instances”) associated with each value, or + // cross-product of values, in the fx and fy scale domains. This probably + // means having an InternSet of fx?+fx? keys recording which facets are + // non-empty, similar to the FacetMap data structure we had before. + + // A facet is empty if none of the faceted index has contents for any mark. + // TODO Can we do this more declaratively rather than re-assigning facets? facets = facets && facets.filter((_, j) => { let nonFaceted = true; - for (const [, {facetsIndex}] of stateByMark) { + for (const {facetsIndex} of stateByMark.values()) { if (facetsIndex) { nonFaceted = false; if (facetsIndex?.[j].length) return true; From 60b0684585dfcaac50db1c68e2ad8ec1ad51f496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Wed, 12 Oct 2022 23:16:44 -0700 Subject: [PATCH 07/34] remove and simplify the weird parts (j, empty facet determination) add a test plot with mark-level facets and exclude clarify comments simplify proper facet look-up for grid lines --- src/facet.js | 39 +- src/plot.js | 280 ++- test/output/penguinCulmenMarkFacet.svg | 2905 +++++++++++++++++++++++ test/plots/index.js | 1 + test/plots/penguin-culmen-mark-facet.js | 28 + 5 files changed, 3106 insertions(+), 147 deletions(-) create mode 100644 test/output/penguinCulmenMarkFacet.svg create mode 100644 test/plots/penguin-culmen-mark-facet.js diff --git a/src/facet.js b/src/facet.js index 308417a3c6..ab1f3af39b 100644 --- a/src/facet.js +++ b/src/facet.js @@ -1,3 +1,4 @@ +import {group, sort} from "d3"; import {keyword, range} from "./options.js"; export function maybeFacet({fx, fy, facet = "auto"} = {}) { @@ -7,9 +8,9 @@ export function maybeFacet({fx, fy, facet = "auto"} = {}) { } // facet filter, by mark -export function filterFacets(facets, {fx, fy}, facetChannels) { - const vx = fx != null ? fx.value : facetChannels?.fx?.value; - const vy = fy != null ? fy.value : facetChannels?.fy?.value; +export function filterFacets(facets, {fx, fy}) { + const vx = fx != null && fx.value; + const vy = fy != null && fy.value; if (!vx && !vy) return; const index = range(vx || vy); return facets.map(({x, y}) => { @@ -25,7 +26,37 @@ export function facetKeyEquals(a, b) { return a instanceof Date && b instanceof Date ? +a === +b : a === b; } -// This must match the key structure of facets +// Unlike facetGroups, which returns groups in order of input data, this returns +// keys in order of the associated scale’s domains. +export function facetKeys(facets, {fx, fy}) { + const fxI = fx && new Map(fx.domain().map((x, i) => [x, i])); + const fyI = fy && new Map(fy.domain().map((y, i) => [y, i])); + return sort( + facets, + ({x: xa, y: ya}, {x: xb, y: yb}) => (fxI && fxI.get(xa) - fxI.get(xb)) || (fyI && fyI.get(ya) - fyI.get(yb)) + ); +} + +// Returns an array of [[key1, index1], [key2, index2], …] representing the data +// indexes associated with each facet. For two-dimensional faceting, each key +// is a two-element array. +export function facetGroups(index, {fx, fy}) { + return fx && fy ? facetGroup2(index, fx, fy) : fx ? facetGroup1(index, fx) : facetGroup1(index, fy); +} + +function facetGroup1(index, {value: F}) { + return group(index, (i) => F[i]); //.map(([k, group]) => [{[x]: k}, group]); +} + +function facetGroup2(index, {value: FX}, {value: FY}) { + return group( + index, + (i) => FX[i], + (i) => FY[i] + ); //.flatMap(([x, xgroup]) => xgroup.map(([y, ygroup]) => [{x, y}, ygroup])); +} + +// This must match the key structure returned by facetGroups. export function facetTranslate(fx, fy) { return fx && fy ? ({x, y}) => `translate(${fx(x)},${fy(y)})` diff --git a/src/plot.js b/src/plot.js index 81198dbd1d..079f264f80 100644 --- a/src/plot.js +++ b/src/plot.js @@ -1,4 +1,4 @@ -import {InternMap, InternSet, cross, select, sort, sum} from "d3"; +import {cross, group, select, sum} from "d3"; import {Axes, autoAxisTicks, autoScaleLabels} from "./axes.js"; import {Channel, Channels, channelDomain, valueObject} from "./channel.js"; import {Context, create} from "./context.js"; @@ -12,7 +12,7 @@ import {applyInlineStyles, maybeClassName, maybeClip, styles} from "./style.js"; import {basic, initializer} from "./transforms/basic.js"; import {maybeInterval} from "./transforms/interval.js"; import {consumeWarnings, warn} from "./warnings.js"; -import {facetKeyEquals, facetTranslate, maybeFacet, filterFacets} from "./facet.js"; +import {facetGroups, facetKeys, facetTranslate, maybeFacet, filterFacets} from "./facet.js"; /** @jsdoc plot */ export function plot(options = {}) { @@ -38,170 +38,155 @@ export function plot(options = {}) { // A Map from scale name to an array of associated channels. const channelsByScale = new Map(); - // If a scale is explicitly declared in options, initialize its associated - // channels to the empty array; this will guarantee that a corresponding scale - // will be created later (even if there are no other channels). But ignore - // facet scale declarations if faceting is not enabled. - for (const key of scaleRegistry.keys()) { - if (isScaleOptions(options[key]) && key !== "fx" && key !== "fy") { - channelsByScale.set(key, []); - } - } - // Faceting! - const facetChannels = {}; // e.g. {fx: {value}, fy: {value}} - let facetsIndex; // nested array of facet indexes [[0, 1, 3, …], [2, 5, …], …] - let facetDataIndex; // index over the facet data, e.g. [0, 1, 2, 3, …] - - // Top-level facets - if (facet !== undefined) { - const {x, y} = facet; - if (x != null || y != null) { - const facetData = arrayify(facet.data); - if (facetData == null) throw new Error("missing facet data"); + + // Collect all facet definitions (top-level facets then mark facets), + // materialize the associated channels, and derive facet scales. + const top = + facet !== undefined + ? {data: facet.data, facet: {x: facet.x, y: facet.y, method: "top"}, ariaLabel: "top-level facet option"} + : undefined; + stateByMark.set(top, {}); + for (const mark of [top, ...marks]) { + const {x, y, method} = mark?.facet ?? {}; + if (!method) continue; + const state = stateByMark.get(mark); + if (method === "auto" && x == null && y == null && facet != null) { + if (mark.data === facet.data) { + state.groups = stateByMark.get(top).groups; + } else { + // Warn for the common pitfall of wanting to facet mapped data. See + // below for the initialization of facetChannelLength. + const {facetChannelLength} = stateByMark.get(top); + if (facetChannelLength !== undefined && arrayify(mark.data)?.length === facetChannelLength) + 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.` + ); + } + } else { + const data = arrayify(mark.data); + if ((x != null || y != null) && data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); if (x != null) { - const fx = Channel(facetData, {value: x, scale: "fx"}); - facetChannels.fx = fx; - channelsByScale.set("fx", [fx]); - facetDataIndex = range(facetData); + state.fx = Channel(data, {value: x, scale: "fx"}); + if (!channelsByScale.has("fx")) channelsByScale.set("fx", []); + channelsByScale.get("fx").push(state.fx); } if (y != null) { - const fy = Channel(facetData, {value: y, scale: "fy"}); - facetChannels.fy = fy; - channelsByScale.set("fy", [fy]); - facetDataIndex = facetDataIndex || range(facetData); + state.fy = Channel(data, {value: y, scale: "fy"}); + if (!channelsByScale.has("fy")) channelsByScale.set("fy", []); + channelsByScale.get("fy").push(state.fy); + } + if (state.fx || state.fy) { + const groups = facetGroups(range(data), state); + state.groups = groups; + // If the top-level faceting is non-trivial, store the corresponding + // data length, in order to compare it for the warning above. + if ( + mark === top && + (groups.size > 1 || (state.fx && state.fy && groups.size === 1 && [...groups][0][1].size > 1)) + ) + state.facetChannelLength = data.length; } } } - // Mark-level facets - for (const mark of marks) { - const {x, y, method} = mark.facet ?? {}; - if (method && x !== undefined) { - const channels = channelsByScale.get("fx") ?? []; - channelsByScale.set("fx", channels); - const channel = Channel(mark.data, {value: x, scale: "fx"}); - stateByMark.get(mark).fx = channel; - channels.push(channel); - } - if (method && y !== undefined) { - const channels = channelsByScale.get("fy") ?? []; - channelsByScale.set("fy", channels); - const channel = Channel(mark.data, {value: y, scale: "fy"}); - stateByMark.get(mark).fy = channel; - channels.push(channel); - } - } - - // Create the facet scales and array of subplots - const facetScales = Scales(channelsByScale); + const facetScales = Scales(channelsByScale, options); - // TODO Avoid j here? TODO Document this data structure. Note that empty is - // mutated below after we’ve computed the per-mark facet channels. + // All the possible facets are given by the domains of fx or fy, or the + // cross-product of these domains if we facet by both x and y. + const fxDomain = facetScales.fx?.scale.domain(); + const fyDomain = facetScales.fy?.scale.domain(); let facets = - facetScales.fx && facetScales.fy - ? cross(facetScales.fx.domain, facetScales.fy.domain).map(([x, y], j) => ({x, y, j, empty: true})) - : facetScales.fx - ? facetScales.fx.domain.map((x, j) => ({x, j, empty: true})) - : facetScales.fy - ? facetScales.fy.domain.map((y, j) => ({y, j, empty: true})) + fxDomain && fyDomain + ? cross(fxDomain, fyDomain).map(([x, y]) => ({x, y})) + : fxDomain + ? fxDomain.map((x) => ({x})) + : fyDomain + ? fyDomain.map((y) => ({y})) : null; - const facetLength = facets && facets.length; - - // Compute the top-level facet index - if (facetDataIndex) { - const {fx, fy} = facetChannels; - facetsIndex = []; - for (const {x, y} of facets) { - facetsIndex.push( - facetDataIndex.filter((i) => (!fx || facetKeyEquals(fx.value[i], x)) && (!fy || facetKeyEquals(fy.value[i], y))) - ); + + // However, the cross product of the domains of fx and fy can include fx-fy + // combinations for which no mark has an instance associated with that + // combination, and therefore we don’t want to render this facet (not even the + // frame). The same can occur if you specify the domain of fx and fy + // explicitly, but there is no mark instance associated with some values in + // the domain. + if (facets) { + const nonEmpty = new Set(); + for (const state of stateByMark.values()) { + const {fx, fy, groups} = state; + if (!groups) continue; + state.facetsIndex = facets.map(({x, y}, i) => { + const I = fx && fy ? groups.get(x)?.get(y) : fx ? groups.get(x) : groups.get(y); + if (I) nonEmpty.add(i); + return I; + }); + } + // Expunge empty facets, and clear the corresponding elements from the + // nested index in each mark. + if (nonEmpty.size < facets.length) { + facets = facets.filter((_, i) => nonEmpty.has(i)); + for (const state of stateByMark.values()) { + const {facetsIndex} = state; + if (!facetsIndex) continue; + state.facetsIndex = facetsIndex.filter((_, i) => nonEmpty.has(i)); + } } } - // Compute a facet index for each mark + // Compute a facet index for each mark, parallel to the facets array. for (const mark of marks) { if (mark.facet === null) continue; const state = stateByMark.get(mark); const {x, y, method} = mark.facet; - // Mark-level facet ? Compute an index for that mark’s data and options + // For mark-level facets, compute an index for that mark’s data and options. if (x !== undefined || y !== undefined) { - if (facets) state.facetsIndex = filterFacets(facets, state, facetChannels); - continue; + if (facets) { + const facetsIndex = filterFacets(facets, state); + state.facetsIndex = method === "exclude" ? excludeIndex(facetsIndex) : facetsIndex; + } + } + // Otherwise, link to the top-level facet information. + else if (facet && (method !== "auto" || mark.data === facet.data)) { + const {facetsIndex, fx, fy} = stateByMark.get(top); + state.facetsIndex = method === "exclude" ? excludeIndex(facetsIndex) : facetsIndex; + if (fx !== undefined) state.fx = fx; + if (fy !== undefined) state.fy = fy; } - - // Otherwise, it's a top-level facet. Note: for mark.facet === "exclude", - // the opposite index is computed *after* empty facets have been determined - if (!facet || (method === "auto" && mark.data !== facet.data)) continue; - state.facetsIndex = facetsIndex; } + stateByMark.delete(top); - // When faceting by both x and y (i.e., when both fx and fy scales are - // present), then the cross product of the domains of fx and fy can include - // fx-fy combinations for which no mark has an instance associated with that - // combination of fx and fy, and therefore we don’t want to render this facet - // (not even the frame). The same can occur if you specify the domain of fx - // and fy explicitly, but there is no mark instance associated with some - // values in the domain. - // - // TODO We need to do two (or three?) passes here. First, we need determine - // the domains of the fx and fy scales (as needed) based on the union of - // distinct channel values, including both mark-level facets and top-level - // facets. Then we need to check whether we have any mark instances (or - // top-level facet data “instances”) associated with each value, or - // cross-product of values, in the fx and fy scale domains. This probably - // means having an InternSet of fx?+fx? keys recording which facets are - // non-empty, similar to the FacetMap data structure we had before. - - // A facet is empty if none of the faceted index has contents for any mark. - // TODO Can we do this more declaratively rather than re-assigning facets? - facets = - facets && - facets.filter((_, j) => { - let nonFaceted = true; - for (const {facetsIndex} of stateByMark.values()) { - if (facetsIndex) { - nonFaceted = false; - if (facetsIndex?.[j].length) return true; - } - } - return nonFaceted; - }); + // If a scale is explicitly declared in options, initialize its associated + // channels to the empty array; this will guarantee that a corresponding scale + // will be created later (even if there are no other channels). But ignore + // facet scale declarations if faceting is not enabled. + for (const key of scaleRegistry.keys()) { + if (isScaleOptions(options[key]) && key !== "fx" && key !== "fy") { + channelsByScale.set(key, []); + } + } // Initialize the marks’ state. for (const mark of marks) { - let {facetsIndex} = stateByMark.get(mark); - if (mark.facet?.method === "exclude") facetsIndex = excludeIndex(facetsIndex); - const {data, facets, channels} = mark.initialize(facetsIndex, facetChannels); + const state = stateByMark.get(mark); + const {data, facets, channels} = mark.initialize(state.facetsIndex, state); applyScaleTransforms(channels, options); stateByMark.set(mark, {data, facets, channels}); - - // Warn for the common pitfall of wanting to facet mapped data. - if ( - facetLength > 1 && // non-trivial faceting - mark.facet?.method === "auto" && // no explicit mark facet option - mark.data !== facet?.data && // mark not implicitly faceted (different data) - arrayify(mark.data)?.length === facetDataIndex?.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. const scaleDescriptors = Scales(addScaleChannels(channelsByScale, stateByMark), options); - const scales = ScaleFunctions(scaleDescriptors); const axes = Axes(scaleDescriptors, options); const dimensions = Dimensions(scaleDescriptors, hasGeometry(stateByMark), axes, options); autoScaleRange(scaleDescriptors, dimensions); autoAxisTicks(scaleDescriptors, axes); + const scales = ScaleFunctions(scaleDescriptors); const {fx, fy} = scales; - const fyMargins = fy && {marginTop: 0, marginBottom: 0, height: fy.bandwidth()}; const fxMargins = fx && {marginRight: 0, marginLeft: 0, width: fx.bandwidth()}; + const fyMargins = fy && {marginTop: 0, marginBottom: 0, height: fy.bandwidth()}; const subdimensions = {...dimensions, ...fxMargins, ...fyMargins}; const context = Context(options, subdimensions); @@ -288,13 +273,8 @@ export function plot(options = {}) { // Render (possibly faceted) marks. if (facets) { - const fyDomain = fy && fy.domain(); const fxDomain = fx && fx.domain(); - const nonEmptyFacets = new InternMap(); - facets.forEach(({x, y}) => { - if (!nonEmptyFacets.has(x)) nonEmptyFacets.set(x, new InternSet()); - nonEmptyFacets.get(x).add(y); - }); + const fyDomain = fy && fy.domain(); const selection = select(svg); if (fy && axes.y) { const axis1 = axes.y, @@ -305,13 +285,23 @@ export function plot(options = {}) { : axis1.labelAnchor === "center" ? fyDomain.length >> 1 : 0; + + // When faceting by both fx and fy, this nested Map allows to look up the + // non-empty facets and draw the grid lines properly. + const cx = + fx && + group( + facets, + ({y}) => y, + ({x}) => x + ); selection .selectAll() .data(fyDomain) .enter() .append((ky, i) => (i === j ? axis1 : axis2).render( - fx && where(fxDomain, (kx) => nonEmptyFacets.get(kx)?.has(ky)), + cx && where(fxDomain, (kx) => cx.get(ky).has(kx)), scales, {...dimensions, ...fyMargins, offsetTop: fy(ky)}, context @@ -323,6 +313,13 @@ export function plot(options = {}) { axis2 = nolabel(axis1); const j = axis1.labelAnchor === "right" ? fxDomain.length - 1 : axis1.labelAnchor === "center" ? fxDomain.length >> 1 : 0; + const cy = + fy && + group( + facets, + ({x}) => x, + ({y}) => y + ); const {marginLeft, marginRight} = dimensions; selection .selectAll() @@ -330,7 +327,7 @@ export function plot(options = {}) { .enter() .append((kx, i) => (i === j ? axis1 : axis2).render( - fy && where(fyDomain, (ky) => nonEmptyFacets.get(kx)?.has(ky)), + cy && where(fyDomain, (ky) => cy.get(kx).has(ky)), scales, { ...dimensions, @@ -343,23 +340,20 @@ export function plot(options = {}) { ) ); } - const fxI = fx && new Map(fx.domain().map((x, i) => [x, i])); - const fyI = fy && new Map(fy.domain().map((y, i) => [y, i])); + + // Render facets in the order of the fx-fy domain—which might not be the + // ordering used to build the nested index initially, see domainChannel + const facetPosition = new Map(facets.map((f, j) => [f, j])); selection .selectAll() - .data( - sort( - facets, - ({x: xa, y: ya}, {x: xb, y: yb}) => (fxI && fxI.get(xa) - fxI.get(xb)) || (fyI && fyI.get(ya) - fyI.get(yb)) - ) - ) + .data(facetKeys(facets, {fx, fy})) .enter() .append("g") .attr("aria-label", "facet") .attr("transform", facetTranslate(fx, fy)) .each(function (key) { for (const [mark, {channels, values, facets}] of stateByMark) { - const facet = facets ? mark.filter(facets[key.j] ?? facets[0], channels, values) : null; + const facet = facets ? mark.filter(facets[facetPosition.get(key)] ?? facets[0], channels, values) : null; const node = mark.render(facet, scales, values, subdimensions, context); if (node != null) this.appendChild(node); } diff --git a/test/output/penguinCulmenMarkFacet.svg b/test/output/penguinCulmenMarkFacet.svg new file mode 100644 index 0000000000..a6d9782f26 --- /dev/null +++ b/test/output/penguinCulmenMarkFacet.svg @@ -0,0 +1,2905 @@ + + + + + Adelie + + + Chinstrap + + + Gentoo + species + + + + FEMALE + + + MALE + + + + sex + + + + 35 + + + 40 + + + 45 + + + 50 + + + 55 + ↑ culmen_length_mm + + + + 35 + + + 40 + + + 45 + + + 50 + + + 55 + + + + + 35 + + + 40 + + + 45 + + + 50 + + + 55 + + + + + 15 + + + 20 + + + + + 15 + + + 20 + + + + + 15 + + + 20 + culmen_depth_mm → + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/test/plots/index.js b/test/plots/index.js index b1d66361e9..dbf5e51e63 100644 --- a/test/plots/index.js +++ b/test/plots/index.js @@ -145,6 +145,7 @@ export {default as penguinCulmenArray} from "./penguin-culmen-array.js"; export {default as penguinCulmenDelaunay} from "./penguin-culmen-delaunay.js"; export {default as penguinCulmenDelaunayMesh} from "./penguin-culmen-delaunay-mesh.js"; export {default as penguinCulmenDelaunaySpecies} from "./penguin-culmen-delaunay-species.js"; +export {default as penguinCulmenMarkFacet} from "./penguin-culmen-mark-facet.js"; export {default as penguinCulmenVoronoi} from "./penguin-culmen-voronoi.js"; export {default as penguinVoronoi1D} from "./penguin-voronoi-1d.js"; export {default as penguinDensity} from "./penguin-density.js"; diff --git a/test/plots/penguin-culmen-mark-facet.js b/test/plots/penguin-culmen-mark-facet.js new file mode 100644 index 0000000000..37b1d8e47a --- /dev/null +++ b/test/plots/penguin-culmen-mark-facet.js @@ -0,0 +1,28 @@ +import * as Plot from "@observablehq/plot"; +import * as d3 from "d3"; + +export default async function () { + const data = await d3.csv("data/penguins.csv", d3.autoType); + return Plot.plot({ + height: 600, + facet: {marginRight: 80}, + marks: [ + Plot.frame(), + Plot.dot(data, { + fx: "sex", + fy: "species", + facet: "exclude", + x: "culmen_depth_mm", + y: "culmen_length_mm", + r: 2, + fill: "#ddd" + }), + Plot.dot(data, { + fx: "sex", + fy: "species", + x: "culmen_depth_mm", + y: "culmen_length_mm" + }) + ] + }); +} From 7cfd8d5e512bb728d0095269bd27e968153f758a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 13 Oct 2022 16:19:55 -0700 Subject: [PATCH 08/34] clean up filterFacets --- src/facet.js | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/facet.js b/src/facet.js index ab1f3af39b..5bde67c56b 100644 --- a/src/facet.js +++ b/src/facet.js @@ -1,5 +1,5 @@ -import {group, sort} from "d3"; -import {keyword, range} from "./options.js"; +import {group, intersection, sort} from "d3"; +import {arrayify, keyword, range} from "./options.js"; export function maybeFacet({fx, fy, facet = "auto"} = {}) { if (facet === null || facet === false) return null; @@ -7,23 +7,18 @@ export function maybeFacet({fx, fy, facet = "auto"} = {}) { return {x: fx, y: fy, method: keyword(facet, "facet", ["auto", "include", "exclude"])}; } -// facet filter, by mark +// Facet filter, by mark; for now only the "eq" filter is provided. export function filterFacets(facets, {fx, fy}) { - const vx = fx != null && fx.value; - const vy = fy != null && fy.value; - if (!vx && !vy) return; - const index = range(vx || vy); - return facets.map(({x, y}) => { - let I = index; - if (vx) I = I.filter((i) => facetKeyEquals(vx[i], x)); - if (vy) I = I.filter((i) => facetKeyEquals(vy[i], y)); - return I; - }); -} - -// test if a value equals a facet key -export function facetKeyEquals(a, b) { - return a instanceof Date && b instanceof Date ? +a === +b : a === b; + const X = fx != null && fx.value; + const Y = fy != null && fy.value; + const index = range(X || Y); + const gx = X && group(index, (i) => X[i]); + const gy = Y && group(index, (i) => Y[i]); + return X && Y + ? facets.map(({x, y}) => arrayify(intersection(gx.get(x) ?? [], gy.get(y) ?? []))) + : X + ? facets.map(({x}) => gx.get(x) ?? []) + : facets.map(({y}) => gy.get(y) ?? []); } // Unlike facetGroups, which returns groups in order of input data, this returns From 532e4c3b31e42cc5713c6aad76b2294cf20335b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 13 Oct 2022 16:34:08 -0700 Subject: [PATCH 09/34] fix comments --- src/facet.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/facet.js b/src/facet.js index 5bde67c56b..e19f9cfcc6 100644 --- a/src/facet.js +++ b/src/facet.js @@ -21,8 +21,7 @@ export function filterFacets(facets, {fx, fy}) { : facets.map(({y}) => gy.get(y) ?? []); } -// Unlike facetGroups, which returns groups in order of input data, this returns -// keys in order of the associated scale’s domains. +// Returns keys in order of the associated scale’s domains. export function facetKeys(facets, {fx, fy}) { const fxI = fx && new Map(fx.domain().map((x, i) => [x, i])); const fyI = fy && new Map(fy.domain().map((y, i) => [y, i])); @@ -32,15 +31,14 @@ export function facetKeys(facets, {fx, fy}) { ); } -// Returns an array of [[key1, index1], [key2, index2], …] representing the data -// indexes associated with each facet. For two-dimensional faceting, each key -// is a two-element array. +// Returns a (possibly nested) Map of [[key1, index1], [key2, index2], …] +// representing the data indexes associated with each facet. export function facetGroups(index, {fx, fy}) { return fx && fy ? facetGroup2(index, fx, fy) : fx ? facetGroup1(index, fx) : facetGroup1(index, fy); } function facetGroup1(index, {value: F}) { - return group(index, (i) => F[i]); //.map(([k, group]) => [{[x]: k}, group]); + return group(index, (i) => F[i]); } function facetGroup2(index, {value: FX}, {value: FY}) { @@ -48,10 +46,9 @@ function facetGroup2(index, {value: FX}, {value: FY}) { index, (i) => FX[i], (i) => FY[i] - ); //.flatMap(([x, xgroup]) => xgroup.map(([y, ygroup]) => [{x, y}, ygroup])); + ); } -// This must match the key structure returned by facetGroups. export function facetTranslate(fx, fy) { return fx && fy ? ({x, y}) => `translate(${fx(x)},${fy(y)})` From eb1bb81662c63c3e878475d05d961432b68423a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Sat, 15 Oct 2022 18:58:05 -0700 Subject: [PATCH 10/34] regroup all facet handling, simplify --- src/plot.js | 198 ++++++++++++++++++++++++++-------------------------- 1 file changed, 100 insertions(+), 98 deletions(-) diff --git a/src/plot.js b/src/plot.js index 079f264f80..0c6f9496aa 100644 --- a/src/plot.js +++ b/src/plot.js @@ -1,4 +1,4 @@ -import {cross, group, select, sum} from "d3"; +import {ascending, cross, group, select, sort, sum} from "d3"; import {Axes, autoAxisTicks, autoScaleLabels} from "./axes.js"; import {Channel, Channels, channelDomain, valueObject} from "./channel.js"; import {Context, create} from "./context.js"; @@ -39,91 +39,113 @@ export function plot(options = {}) { const channelsByScale = new Map(); // Faceting! + let facets; // Collect all facet definitions (top-level facets then mark facets), // materialize the associated channels, and derive facet scales. - const top = - facet !== undefined - ? {data: facet.data, facet: {x: facet.x, y: facet.y, method: "top"}, ariaLabel: "top-level facet option"} - : undefined; - stateByMark.set(top, {}); - for (const mark of [top, ...marks]) { - const {x, y, method} = mark?.facet ?? {}; - if (!method) continue; - const state = stateByMark.get(mark); - if (method === "auto" && x == null && y == null && facet != null) { - if (mark.data === facet.data) { - state.groups = stateByMark.get(top).groups; + if (facet || marks.some(({facet}) => facet.x || facet.y)) { + const top = + facet !== undefined + ? {data: facet.data, facet: {x: facet.x, y: facet.y, method: "top"}, ariaLabel: "top-level facet option"} + : {facet: null}; + + stateByMark.set(top, {}); + + for (const mark of [top, ...marks]) { + const {x, y, method} = mark?.facet ?? {}; + if (!method) continue; + const state = stateByMark.get(mark); + if (x == null && y == null && facet != null) { + if (method !== "auto" || mark.data === facet.data) { + state.groups = stateByMark.get(top).groups; + } else { + // Warn for the common pitfall of wanting to facet mapped data. See + // below for the initialization of facetChannelLength. + const {facetChannelLength} = stateByMark.get(top); + if (facetChannelLength !== undefined && arrayify(mark.data)?.length === facetChannelLength) + 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.` + ); + } } else { - // Warn for the common pitfall of wanting to facet mapped data. See - // below for the initialization of facetChannelLength. - const {facetChannelLength} = stateByMark.get(top); - if (facetChannelLength !== undefined && arrayify(mark.data)?.length === facetChannelLength) - 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.` - ); - } - } else { - const data = arrayify(mark.data); - if ((x != null || y != null) && data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); - if (x != null) { - state.fx = Channel(data, {value: x, scale: "fx"}); - if (!channelsByScale.has("fx")) channelsByScale.set("fx", []); - channelsByScale.get("fx").push(state.fx); + const data = arrayify(mark.data); + if ((x != null || y != null) && data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); + if (x != null) { + state.fx = Channel(data, {value: x, scale: "fx"}); + if (!channelsByScale.has("fx")) channelsByScale.set("fx", []); + channelsByScale.get("fx").push(state.fx); + } + if (y != null) { + state.fy = Channel(data, {value: y, scale: "fy"}); + if (!channelsByScale.has("fy")) channelsByScale.set("fy", []); + channelsByScale.get("fy").push(state.fy); + } + if (state.fx || state.fy) { + const groups = facetGroups(range(data), state); + state.groups = groups; + // If the top-level faceting is non-trivial, store the corresponding + // data length, in order to compare it for the warning above. + if ( + mark === top && + (groups.size > 1 || (state.fx && state.fy && groups.size === 1 && [...groups][0][1].size > 1)) + ) + state.facetChannelLength = data.length; + } } - if (y != null) { - state.fy = Channel(data, {value: y, scale: "fy"}); - if (!channelsByScale.has("fy")) channelsByScale.set("fy", []); - channelsByScale.get("fy").push(state.fy); + } + + const facetScales = Scales(channelsByScale, options); + + // All the possible facets are given by the domains of fx or fy, or the + // cross-product of these domains if we facet by both x and y. We sort them in + // order to apply the facet filters afterwards. + const fxDomain = facetScales.fx?.scale.domain(); + const fyDomain = facetScales.fy?.scale.domain(); + facets = + fxDomain && fyDomain + ? cross(sort(fxDomain, ascending), sort(fyDomain, ascending)).map(([x, y]) => ({x, y})) + : fxDomain + ? sort(fxDomain, ascending).map((x) => ({x})) + : fyDomain + ? sort(fyDomain, ascending).map((y) => ({y})) + : null; + + // Compute a facet index for each mark, parallel to the facets array. + for (const mark of [top, ...marks]) { + if (mark.facet === null) continue; + + const state = stateByMark.get(mark); + const {x, y, method} = mark.facet; + + // For mark-level facets, compute an index for that mark’s data and options. + if (x !== undefined || y !== undefined) { + state.facetsIndex = filterFacets(facets, state); } - if (state.fx || state.fy) { - const groups = facetGroups(range(data), state); - state.groups = groups; - // If the top-level faceting is non-trivial, store the corresponding - // data length, in order to compare it for the warning above. - if ( - mark === top && - (groups.size > 1 || (state.fx && state.fy && groups.size === 1 && [...groups][0][1].size > 1)) - ) - state.facetChannelLength = data.length; + + // Otherwise, link to the top-level facet information. + else if (facet && (method !== "auto" || mark.data === facet.data)) { + const {facetsIndex, fx, fy} = stateByMark.get(top); + state.facetsIndex = facetsIndex; + if (fx !== undefined) state.fx = fx; + if (fy !== undefined) state.fy = fy; } } - } - const facetScales = Scales(channelsByScale, options); - - // All the possible facets are given by the domains of fx or fy, or the - // cross-product of these domains if we facet by both x and y. - const fxDomain = facetScales.fx?.scale.domain(); - const fyDomain = facetScales.fy?.scale.domain(); - let facets = - fxDomain && fyDomain - ? cross(fxDomain, fyDomain).map(([x, y]) => ({x, y})) - : fxDomain - ? fxDomain.map((x) => ({x})) - : fyDomain - ? fyDomain.map((y) => ({y})) - : null; - - // However, the cross product of the domains of fx and fy can include fx-fy - // combinations for which no mark has an instance associated with that - // combination, and therefore we don’t want to render this facet (not even the - // frame). The same can occur if you specify the domain of fx and fy - // explicitly, but there is no mark instance associated with some values in - // the domain. - if (facets) { + // The cross product of the domains of fx and fy can include fx-fy + // combinations for which no mark has an instance associated with that + // combination, and therefore we don’t want to render this facet (not even + // the frame). The same can occur if you specify the domain of fx and fy + // explicitly, but there is no mark instance associated with some values in + // the domain. Expunge empty facets, and clear the corresponding elements + // from the nested index in each mark. const nonEmpty = new Set(); - for (const state of stateByMark.values()) { - const {fx, fy, groups} = state; - if (!groups) continue; - state.facetsIndex = facets.map(({x, y}, i) => { - const I = fx && fy ? groups.get(x)?.get(y) : fx ? groups.get(x) : groups.get(y); - if (I) nonEmpty.add(i); - return I; - }); + for (const {facetsIndex} of stateByMark.values()) { + if (facetsIndex) { + facetsIndex.forEach((index, i) => { + if (index?.length > 0) nonEmpty.add(i); + }); + } } - // Expunge empty facets, and clear the corresponding elements from the - // nested index in each mark. if (nonEmpty.size < facets.length) { facets = facets.filter((_, i) => nonEmpty.has(i)); for (const state of stateByMark.values()) { @@ -132,30 +154,9 @@ export function plot(options = {}) { state.facetsIndex = facetsIndex.filter((_, i) => nonEmpty.has(i)); } } - } - - // Compute a facet index for each mark, parallel to the facets array. - for (const mark of marks) { - if (mark.facet === null) continue; - const state = stateByMark.get(mark); - const {x, y, method} = mark.facet; - // For mark-level facets, compute an index for that mark’s data and options. - if (x !== undefined || y !== undefined) { - if (facets) { - const facetsIndex = filterFacets(facets, state); - state.facetsIndex = method === "exclude" ? excludeIndex(facetsIndex) : facetsIndex; - } - } - // Otherwise, link to the top-level facet information. - else if (facet && (method !== "auto" || mark.data === facet.data)) { - const {facetsIndex, fx, fy} = stateByMark.get(top); - state.facetsIndex = method === "exclude" ? excludeIndex(facetsIndex) : facetsIndex; - if (fx !== undefined) state.fx = fx; - if (fy !== undefined) state.fy = fy; - } + stateByMark.delete(top); } - stateByMark.delete(top); // If a scale is explicitly declared in options, initialize its associated // channels to the empty array; this will guarantee that a corresponding scale @@ -170,7 +171,8 @@ export function plot(options = {}) { // Initialize the marks’ state. for (const mark of marks) { const state = stateByMark.get(mark); - const {data, facets, channels} = mark.initialize(state.facetsIndex, state); + const facetsIndex = mark.facet?.method === "exclude" ? excludeIndex(state.facetsIndex) : state.facetsIndex; + const {data, facets, channels} = mark.initialize(facetsIndex, state); applyScaleTransforms(channels, options); stateByMark.set(mark, {data, facets, channels}); } From 356f72594785c4a7d5adde20ba914e870b7c48db Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Sat, 22 Oct 2022 10:19:29 -0700 Subject: [PATCH 11/34] inline maybeFacet; improve backwards compatibility --- src/facet.js | 8 +------- src/plot.js | 47 +++++++++++++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/facet.js b/src/facet.js index e19f9cfcc6..900f304355 100644 --- a/src/facet.js +++ b/src/facet.js @@ -1,11 +1,5 @@ import {group, intersection, sort} from "d3"; -import {arrayify, keyword, range} from "./options.js"; - -export function maybeFacet({fx, fy, facet = "auto"} = {}) { - if (facet === null || facet === false) return null; - if (facet === true) facet = "include"; - return {x: fx, y: fy, method: keyword(facet, "facet", ["auto", "include", "exclude"])}; -} +import {arrayify, range} from "./options.js"; // Facet filter, by mark; for now only the "eq" filter is provided. export function filterFacets(facets, {fx, fy}) { diff --git a/src/plot.js b/src/plot.js index 0c6f9496aa..b6d1fe0504 100644 --- a/src/plot.js +++ b/src/plot.js @@ -5,14 +5,14 @@ import {Context, create} from "./context.js"; import {defined} from "./defined.js"; import {Dimensions} from "./dimensions.js"; import {Legends, exposeLegends} from "./legends.js"; -import {arrayify, isDomainSort, isScaleOptions, map, maybeNamed, range, where, yes} from "./options.js"; +import {arrayify, isDomainSort, isScaleOptions, keyword, map, maybeNamed, range, where, yes} from "./options.js"; import {Scales, ScaleFunctions, autoScaleRange, exposeScales} from "./scales.js"; 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, warn} from "./warnings.js"; -import {facetGroups, facetKeys, facetTranslate, maybeFacet, filterFacets} from "./facet.js"; +import {facetGroups, facetKeys, facetTranslate, filterFacets} from "./facet.js"; /** @jsdoc plot */ export function plot(options = {}) { @@ -43,19 +43,24 @@ export function plot(options = {}) { // Collect all facet definitions (top-level facets then mark facets), // materialize the associated channels, and derive facet scales. - if (facet || marks.some(({facet}) => facet.x || facet.y)) { + if (facet || marks.some((mark) => mark.fx || mark.fy)) { // TODO non-null, not truthy + // TODO Remove/refactor this: here “top” is pretending to be a mark, but + // it’s not actually a mark. Also there’s no “top” facet method, and the + // ariaLabel isn’t used for anything. And eventually top is removed from + // stateByMark. We can find a cleaner way to do this. const top = facet !== undefined - ? {data: facet.data, facet: {x: facet.x, y: facet.y, method: "top"}, ariaLabel: "top-level facet option"} + ? {data: facet.data, fx: facet.x, fy: facet.y, facet: "top", ariaLabel: "top-level facet option"} : {facet: null}; stateByMark.set(top, {}); for (const mark of [top, ...marks]) { - const {x, y, method} = mark?.facet ?? {}; - if (!method) continue; + const method = mark?.facet; // TODO rename to facet; remove check if mark is undefined? + if (!method) continue; // TODO explicitly check for null + const {fx: x, fy: y} = mark; const state = stateByMark.get(mark); - if (x == null && y == null && facet != null) { + if (x == null && y == null && facet != null) { // TODO strict equality if (method !== "auto" || mark.data === facet.data) { state.groups = stateByMark.get(top).groups; } else { @@ -69,18 +74,18 @@ export function plot(options = {}) { } } else { const data = arrayify(mark.data); - if ((x != null || y != null) && data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); - if (x != null) { + if ((x != null || y != null) && data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); // TODO strict equality + if (x != null) { // TODO strict equality state.fx = Channel(data, {value: x, scale: "fx"}); if (!channelsByScale.has("fx")) channelsByScale.set("fx", []); channelsByScale.get("fx").push(state.fx); } - if (y != null) { + if (y != null) { // TODO strict equality state.fy = Channel(data, {value: y, scale: "fy"}); if (!channelsByScale.has("fy")) channelsByScale.set("fy", []); channelsByScale.get("fy").push(state.fy); } - if (state.fx || state.fy) { + if (state.fx || state.fy) { // TODO strict equality const groups = facetGroups(range(data), state); state.groups = groups; // If the top-level faceting is non-trivial, store the corresponding @@ -89,7 +94,7 @@ export function plot(options = {}) { mark === top && (groups.size > 1 || (state.fx && state.fy && groups.size === 1 && [...groups][0][1].size > 1)) ) - state.facetChannelLength = data.length; + state.facetChannelLength = data.length; // TODO curly braces } } } @@ -112,10 +117,10 @@ export function plot(options = {}) { // Compute a facet index for each mark, parallel to the facets array. for (const mark of [top, ...marks]) { - if (mark.facet === null) continue; - + const method = mark.facet; // TODO rename to facet + if (method === null) continue; + const {fx: x, fy: y} = mark; const state = stateByMark.get(mark); - const {x, y, method} = mark.facet; // For mark-level facets, compute an index for that mark’s data and options. if (x !== undefined || y !== undefined) { @@ -171,7 +176,7 @@ export function plot(options = {}) { // Initialize the marks’ state. for (const mark of marks) { const state = stateByMark.get(mark); - const facetsIndex = mark.facet?.method === "exclude" ? excludeIndex(state.facetsIndex) : state.facetsIndex; + const facetsIndex = mark.facet === "exclude" ? excludeIndex(state.facetsIndex) : state.facetsIndex; const {data, facets, channels} = mark.initialize(facetsIndex, state); applyScaleTransforms(channels, options); stateByMark.set(mark, {data, facets, channels}); @@ -406,12 +411,18 @@ export function plot(options = {}) { export class Mark { constructor(data, channels = {}, options = {}, defaults) { - const {sort, dx, dy, clip, channels: extraChannels} = options; + const {facet = "auto", fx, fy, sort, dx, dy, clip, channels: extraChannels} = options; this.data = data; this.sort = isDomainSort(sort) ? sort : null; this.initializer = initializer(options).initializer; this.transform = this.initializer ? options.transform : basic(options).transform; - this.facet = maybeFacet(options); + if (facet === null || facet === false) { + this.facet = null; + } else { + this.facet = keyword(facet === true ? "include" : facet, "facet", ["auto", "include", "exclude"]); + this.fx = fx; + this.fy = fy; + } channels = maybeNamed(channels); if (extraChannels !== undefined) channels = {...maybeNamed(extraChannels), ...channels}; if (defaults !== undefined) channels = {...styles(this, options, defaults), ...channels}; From 611cef017fa06596c535c100e03f35526ea5293c Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Sat, 22 Oct 2022 10:23:00 -0700 Subject: [PATCH 12/34] minimize diff --- src/plot.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/plot.js b/src/plot.js index b6d1fe0504..19e5bb555c 100644 --- a/src/plot.js +++ b/src/plot.js @@ -184,16 +184,16 @@ export function plot(options = {}) { // Initalize the scales and axes. const scaleDescriptors = Scales(addScaleChannels(channelsByScale, stateByMark), options); + const scales = ScaleFunctions(scaleDescriptors); const axes = Axes(scaleDescriptors, options); const dimensions = Dimensions(scaleDescriptors, hasGeometry(stateByMark), axes, options); autoScaleRange(scaleDescriptors, dimensions); autoAxisTicks(scaleDescriptors, axes); - const scales = ScaleFunctions(scaleDescriptors); const {fx, fy} = scales; - const fxMargins = fx && {marginRight: 0, marginLeft: 0, width: fx.bandwidth()}; const fyMargins = fy && {marginTop: 0, marginBottom: 0, height: fy.bandwidth()}; + const fxMargins = fx && {marginRight: 0, marginLeft: 0, width: fx.bandwidth()}; const subdimensions = {...dimensions, ...fxMargins, ...fyMargins}; const context = Context(options, subdimensions); @@ -279,9 +279,9 @@ export function plot(options = {}) { if (axisX) svg.appendChild(axisX.render(null, scales, dimensions, context)); // Render (possibly faceted) marks. - if (facets) { - const fxDomain = fx && fx.domain(); + if (facets !== undefined) { const fyDomain = fy && fy.domain(); + const fxDomain = fx && fx.domain(); const selection = select(svg); if (fy && axes.y) { const axis1 = axes.y, @@ -348,8 +348,8 @@ export function plot(options = {}) { ); } - // Render facets in the order of the fx-fy domain—which might not be the - // ordering used to build the nested index initially, see domainChannel + // Render facets in the order of the fx-fy domain, which might not be the + // ordering used to build the nested index initially; see domainChannel. const facetPosition = new Map(facets.map((f, j) => [f, j])); selection .selectAll() From 868a6f51068ff159ad091ef8a7427f07ac44bafe Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Sat, 22 Oct 2022 10:29:23 -0700 Subject: [PATCH 13/34] comments --- src/plot.js | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/plot.js b/src/plot.js index 19e5bb555c..9bec503e90 100644 --- a/src/plot.js +++ b/src/plot.js @@ -32,6 +32,17 @@ export function plot(options = {}) { const stateByMark = new Map(); for (const mark of marks) { if (stateByMark.has(mark)) throw new Error("duplicate mark; each mark must be unique"); + + // TODO It’s undesirable to set this to an empty object here because it + // makes it less obvious what the expected type of mark state is. And also + // when we (eventually) migrate to TypeScript, this would be disallowed. + // Previously mark state was a {data, facet, channels, values} object; now + // it looks like we also use: fx, fy, groups, facetChannelLength, + // facetsIndex. And these are set at various different points below, so + // there are more intermediate representations where the state is partially + // initialized. If possible we should try to reduce the number of + // intermediate states and simplify the state representations to make the + // logic easier to follow. stateByMark.set(mark, {}); } @@ -43,7 +54,9 @@ export function plot(options = {}) { // Collect all facet definitions (top-level facets then mark facets), // materialize the associated channels, and derive facet scales. - if (facet || marks.some((mark) => mark.fx || mark.fy)) { // TODO non-null, not truthy + if (facet || marks.some((mark) => mark.fx || mark.fy)) { + // TODO non-null, not truthy + // TODO Remove/refactor this: here “top” is pretending to be a mark, but // it’s not actually a mark. Also there’s no “top” facet method, and the // ariaLabel isn’t used for anything. And eventually top is removed from @@ -60,7 +73,8 @@ export function plot(options = {}) { if (!method) continue; // TODO explicitly check for null const {fx: x, fy: y} = mark; const state = stateByMark.get(mark); - if (x == null && y == null && facet != null) { // TODO strict equality + if (x == null && y == null && facet != null) { + // TODO strict equality if (method !== "auto" || mark.data === facet.data) { state.groups = stateByMark.get(top).groups; } else { @@ -75,17 +89,20 @@ export function plot(options = {}) { } else { const data = arrayify(mark.data); if ((x != null || y != null) && data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); // TODO strict equality - if (x != null) { // TODO strict equality + if (x != null) { + // TODO strict equality state.fx = Channel(data, {value: x, scale: "fx"}); if (!channelsByScale.has("fx")) channelsByScale.set("fx", []); channelsByScale.get("fx").push(state.fx); } - if (y != null) { // TODO strict equality + if (y != null) { + // TODO strict equality state.fy = Channel(data, {value: y, scale: "fy"}); if (!channelsByScale.has("fy")) channelsByScale.set("fy", []); channelsByScale.get("fy").push(state.fy); } - if (state.fx || state.fy) { // TODO strict equality + if (state.fx || state.fy) { + // TODO strict equality const groups = facetGroups(range(data), state); state.groups = groups; // If the top-level faceting is non-trivial, store the corresponding From 98dd563de7a54cd6c5a6b357f5cd7f0c87df6a36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Sun, 23 Oct 2022 16:09:22 -0700 Subject: [PATCH 14/34] clean --- src/facet.js | 70 ++++++++++++++++- src/plot.js | 209 ++++++++++++++++++--------------------------------- 2 files changed, 144 insertions(+), 135 deletions(-) diff --git a/src/facet.js b/src/facet.js index 900f304355..9f3cc1efab 100644 --- a/src/facet.js +++ b/src/facet.js @@ -1,5 +1,7 @@ -import {group, intersection, sort} from "d3"; +import {group, intersection, sort, sum} from "d3"; import {arrayify, range} from "./options.js"; +import {Channel} from "./channel.js"; +import {warn} from "./warnings.js"; // Facet filter, by mark; for now only the "eq" filter is provided. export function filterFacets(facets, {fx, fy}) { @@ -50,3 +52,69 @@ export function facetTranslate(fx, fy) { ? ({x}) => `translate(${fx(x)},0)` : ({y}) => `translate(0,${fy(y)})`; } + +// Returns an index that for each facet lists all the elements present in other +// facets in the original index +export function excludeIndex(index) { + const ex = []; + const e = new Uint32Array(sum(index, (d) => d.length)); + for (const i of index) { + let n = 0; + for (const j of index) { + if (i === j) continue; + e.set(j, n); + n += j.length; + } + ex.push(e.slice(0, n)); + } + return ex; +} + +// Returns the facet groups, and possibly fx and fy channels, associated to the +// top-level facet options {data, x, y} +export function topFacetRead(facet) { + if (facet == null) return; + const {x, y} = facet; + if (x != null || y != null) { + const data = arrayify(facet.data); + if (data == null) throw new Error(`missing facet data`); // TODO strict equality + const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; + const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; + const groups = facetGroups(range(data), {fx, fy}); + // If the top-level faceting is non-trivial, track the corresponding data + // length, in order to compare it for the warning above. + const facetChannelLength = + groups.size > 1 || (fx && fy && groups.size === 1 && [...groups][0][1].size > 1) ? data.length : undefined; + return {groups, fx, fy, facetChannelLength}; + } +} + +// Returns the facet groups, and possibly fx and fy channels, associated to a +// mark, either through top-level faceting or mark-level facet options {fx, fy} +export function facetRead(mark, facetOptions, topFacetInfo) { + if (mark.facet === null) return; + + // This mark defines a mark-level facet. + const {fx: x, fy: y} = mark; + if (x != null || y != null) { + const data = arrayify(mark.data); + if (data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); // TODO strict equality + const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; + const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; + return {groups: facetGroups(range(data), {fx, fy}), fx, fy}; + } + + // This mark links to a top-level facet, if present. + if (topFacetInfo === undefined) return; + + const {groups, facetChannelLength} = topFacetInfo; + if (mark.facet !== "auto" || mark.data === facetOptions.data) return {groups}; + + // Warn for the common pitfall of wanting to facet mapped data. See + // above for the initialization of facetChannelLength. + if (facetChannelLength !== undefined && arrayify(mark.data)?.length === facetChannelLength) { + 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.` + ); + } +} diff --git a/src/plot.js b/src/plot.js index 9bec503e90..5eda59213f 100644 --- a/src/plot.js +++ b/src/plot.js @@ -1,6 +1,6 @@ -import {ascending, cross, group, select, sort, sum} from "d3"; +import {ascending, cross, group, select, sort} from "d3"; import {Axes, autoAxisTicks, autoScaleLabels} from "./axes.js"; -import {Channel, Channels, channelDomain, valueObject} from "./channel.js"; +import {Channels, channelDomain, valueObject} from "./channel.js"; import {Context, create} from "./context.js"; import {defined} from "./defined.js"; import {Dimensions} from "./dimensions.js"; @@ -11,8 +11,8 @@ 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, warn} from "./warnings.js"; -import {facetGroups, facetKeys, facetTranslate, filterFacets} from "./facet.js"; +import {consumeWarnings} from "./warnings.js"; +import {excludeIndex, facetKeys, facetTranslate, filterFacets, topFacetRead, facetRead} from "./facet.js"; /** @jsdoc plot */ export function plot(options = {}) { @@ -24,132 +24,83 @@ export function plot(options = {}) { // Flatten any nested marks. const marks = options.marks === undefined ? [] : options.marks.flat(Infinity).map(markify); - // A Map from Mark instance to its render state, including: - // index - the data index e.g. [0, 1, 2, 3, …] - // channels - an array of materialized channels e.g. [["x", {value}], …] - // faceted - a boolean indicating whether this mark is faceted - // values - an object of scaled values e.g. {x: [40, 32, …], …} - const stateByMark = new Map(); - for (const mark of marks) { - if (stateByMark.has(mark)) throw new Error("duplicate mark; each mark must be unique"); - - // TODO It’s undesirable to set this to an empty object here because it - // makes it less obvious what the expected type of mark state is. And also - // when we (eventually) migrate to TypeScript, this would be disallowed. - // Previously mark state was a {data, facet, channels, values} object; now - // it looks like we also use: fx, fy, groups, facetChannelLength, - // facetsIndex. And these are set at various different points below, so - // there are more intermediate representations where the state is partially - // initialized. If possible we should try to reduce the number of - // intermediate states and simplify the state representations to make the - // logic easier to follow. - stateByMark.set(mark, {}); - } - // A Map from scale name to an array of associated channels. const channelsByScale = new Map(); // Faceting! let facets; + // A map from top-level facet or mark to facet information, including: + // * groups - a possibly nested map from facet values to indexes in the data + // array + // * fx - a channel to add to the fx scale + // * fy - a channel to add to the fy scale + // * facetChannelLength - the top-level facet indicates a facet channel length + // to help warn the user if a different data of the same length is used in a + // mark + // * facetsIndex - In a second pass, a nested array of indices corresponding + // to the valid facets + const facetCollect = new Map(); + // Collect all facet definitions (top-level facets then mark facets), // materialize the associated channels, and derive facet scales. - if (facet || marks.some((mark) => mark.fx || mark.fy)) { - // TODO non-null, not truthy - - // TODO Remove/refactor this: here “top” is pretending to be a mark, but - // it’s not actually a mark. Also there’s no “top” facet method, and the - // ariaLabel isn’t used for anything. And eventually top is removed from - // stateByMark. We can find a cleaner way to do this. - const top = - facet !== undefined - ? {data: facet.data, fx: facet.x, fy: facet.y, facet: "top", ariaLabel: "top-level facet option"} - : {facet: null}; - - stateByMark.set(top, {}); - - for (const mark of [top, ...marks]) { - const method = mark?.facet; // TODO rename to facet; remove check if mark is undefined? - if (!method) continue; // TODO explicitly check for null - const {fx: x, fy: y} = mark; - const state = stateByMark.get(mark); - if (x == null && y == null && facet != null) { - // TODO strict equality - if (method !== "auto" || mark.data === facet.data) { - state.groups = stateByMark.get(top).groups; - } else { - // Warn for the common pitfall of wanting to facet mapped data. See - // below for the initialization of facetChannelLength. - const {facetChannelLength} = stateByMark.get(top); - if (facetChannelLength !== undefined && arrayify(mark.data)?.length === facetChannelLength) - 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.` - ); - } - } else { - const data = arrayify(mark.data); - if ((x != null || y != null) && data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); // TODO strict equality - if (x != null) { - // TODO strict equality - state.fx = Channel(data, {value: x, scale: "fx"}); - if (!channelsByScale.has("fx")) channelsByScale.set("fx", []); - channelsByScale.get("fx").push(state.fx); - } - if (y != null) { - // TODO strict equality - state.fy = Channel(data, {value: y, scale: "fy"}); - if (!channelsByScale.has("fy")) channelsByScale.set("fy", []); - channelsByScale.get("fy").push(state.fy); - } - if (state.fx || state.fy) { - // TODO strict equality - const groups = facetGroups(range(data), state); - state.groups = groups; - // If the top-level faceting is non-trivial, store the corresponding - // data length, in order to compare it for the warning above. - if ( - mark === top && - (groups.size > 1 || (state.fx && state.fy && groups.size === 1 && [...groups][0][1].size > 1)) - ) - state.facetChannelLength = data.length; // TODO curly braces - } - } + const topFacetInfo = topFacetRead(facet); + if (topFacetInfo) facetCollect.set(null, topFacetInfo); + + for (const mark of marks) { + const f = facetRead(mark, facet, topFacetInfo); + if (f) facetCollect.set(mark, f); + } + for (const f of facetCollect.values()) { + const {fx, fy} = f; + if (fx) { + if (!channelsByScale.has("fx")) channelsByScale.set("fx", []); + channelsByScale.get("fx").push(fx); + } + if (fy) { + if (!channelsByScale.has("fy")) channelsByScale.set("fy", []); + channelsByScale.get("fy").push(fy); } + } + + const facetScales = Scales(channelsByScale, options); + + // All the possible facets are given by the domains of fx or fy, or the + // cross-product of these domains if we facet by both x and y. We sort them in + // order to apply the facet filters afterwards. + const fxDomain = facetScales.fx?.scale.domain(); + const fyDomain = facetScales.fy?.scale.domain(); + facets = + fxDomain && fyDomain + ? cross(sort(fxDomain, ascending), sort(fyDomain, ascending)).map(([x, y]) => ({x, y})) + : fxDomain + ? sort(fxDomain, ascending).map((x) => ({x})) + : fyDomain + ? sort(fyDomain, ascending).map((y) => ({y})) + : undefined; - const facetScales = Scales(channelsByScale, options); - - // All the possible facets are given by the domains of fx or fy, or the - // cross-product of these domains if we facet by both x and y. We sort them in - // order to apply the facet filters afterwards. - const fxDomain = facetScales.fx?.scale.domain(); - const fyDomain = facetScales.fy?.scale.domain(); - facets = - fxDomain && fyDomain - ? cross(sort(fxDomain, ascending), sort(fyDomain, ascending)).map(([x, y]) => ({x, y})) - : fxDomain - ? sort(fxDomain, ascending).map((x) => ({x})) - : fyDomain - ? sort(fyDomain, ascending).map((y) => ({y})) - : null; + if (facets !== undefined) { + const facetsIndex = topFacetInfo ? filterFacets(facets, topFacetInfo) : undefined; // Compute a facet index for each mark, parallel to the facets array. - for (const mark of [top, ...marks]) { - const method = mark.facet; // TODO rename to facet - if (method === null) continue; + for (const mark of marks) { + const {facet} = mark; + if (facet === null) continue; const {fx: x, fy: y} = mark; - const state = stateByMark.get(mark); + const facetInfo = facetCollect.get(mark); + if (facetInfo === undefined) continue; // For mark-level facets, compute an index for that mark’s data and options. if (x !== undefined || y !== undefined) { - state.facetsIndex = filterFacets(facets, state); + facetInfo.facetsIndex = filterFacets(facets, facetInfo); } // Otherwise, link to the top-level facet information. - else if (facet && (method !== "auto" || mark.data === facet.data)) { - const {facetsIndex, fx, fy} = stateByMark.get(top); - state.facetsIndex = facetsIndex; - if (fx !== undefined) state.fx = fx; - if (fy !== undefined) state.fy = fy; + else if (topFacetInfo !== undefined) { + facetInfo.facetsIndex = facetsIndex; + const {fx, fy} = topFacetInfo; + if (fx !== undefined) facetInfo.fx = fx; + if (fy !== undefined) facetInfo.fy = fy; } } @@ -161,23 +112,22 @@ export function plot(options = {}) { // the domain. Expunge empty facets, and clear the corresponding elements // from the nested index in each mark. const nonEmpty = new Set(); - for (const {facetsIndex} of stateByMark.values()) { + for (const {facetsIndex} of facetCollect.values()) { if (facetsIndex) { facetsIndex.forEach((index, i) => { if (index?.length > 0) nonEmpty.add(i); }); } } - if (nonEmpty.size < facets.length) { + if (0 < nonEmpty.size && nonEmpty.size < facets.length) { facets = facets.filter((_, i) => nonEmpty.has(i)); - for (const state of stateByMark.values()) { + for (const state of facetCollect.values()) { const {facetsIndex} = state; + //console.warn(facetsIndex); if (!facetsIndex) continue; state.facetsIndex = facetsIndex.filter((_, i) => nonEmpty.has(i)); } } - - stateByMark.delete(top); } // If a scale is explicitly declared in options, initialize its associated @@ -190,9 +140,17 @@ export function plot(options = {}) { } } + // A Map from Mark instance to its render state, including: + // index - the data index e.g. [0, 1, 2, 3, …] + // channels - an array of materialized channels e.g. [["x", {value}], …] + // faceted - a boolean indicating whether this mark is faceted + // values - an object of scaled values e.g. {x: [40, 32, …], …} + const stateByMark = new Map(); + // Initialize the marks’ state. for (const mark of marks) { - const state = stateByMark.get(mark); + if (stateByMark.has(mark)) throw new Error("duplicate mark; each mark must be unique"); + const state = facetCollect.get(mark) || {}; const facetsIndex = mark.facet === "exclude" ? excludeIndex(state.facetsIndex) : state.facetsIndex; const {data, facets, channels} = mark.initialize(facetsIndex, state); applyScaleTransforms(channels, options); @@ -569,20 +527,3 @@ function nolabel(axis) { ? axis // use the existing axis if unlabeled : Object.assign(Object.create(axis), {label: undefined}); } - -// Returns an index that for each facet lists all the elements present in other -// facets in the original index -function excludeIndex(index) { - const ex = []; - const e = new Uint32Array(sum(index, (d) => d.length)); - for (const i of index) { - let n = 0; - for (const j of index) { - if (i === j) continue; - e.set(j, n); - n += j.length; - } - ex.push(e.slice(0, n)); - } - return ex; -} From aaa44e6b45aa7e9115dde7ebe25d2bae50004130 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 15:47:50 -0800 Subject: [PATCH 15/34] fix tests --- test/output/penguinFacetAnnotated.svg | 6 +++--- test/output/penguinFacetAnnotatedX.svg | 6 +++--- test/plots/penguins-facet-annotated-x.js | 22 +++++++--------------- test/plots/penguins-facet-annotated.js | 19 +++++-------------- 4 files changed, 18 insertions(+), 35 deletions(-) diff --git a/test/output/penguinFacetAnnotated.svg b/test/output/penguinFacetAnnotated.svg index a64c066a24..a7500b4bbd 100644 --- a/test/output/penguinFacetAnnotated.svg +++ b/test/output/penguinFacetAnnotated.svg @@ -89,7 +89,7 @@ - + @@ -100,7 +100,7 @@ - + @@ -109,6 +109,6 @@ - ↞ Torgersen Island only has Adelie penguins! + Torgersen Island only has Adelie penguins! \ No newline at end of file diff --git a/test/output/penguinFacetAnnotatedX.svg b/test/output/penguinFacetAnnotatedX.svg index b60f23d573..a3ae5fed67 100644 --- a/test/output/penguinFacetAnnotatedX.svg +++ b/test/output/penguinFacetAnnotatedX.svg @@ -77,7 +77,7 @@ - + @@ -88,7 +88,7 @@ - + @@ -97,6 +97,6 @@ - Torgersen Islandonly hasAdeliepenguins! + Torgersen Islandonly has Adeliepenguins! \ No newline at end of file diff --git a/test/plots/penguins-facet-annotated-x.js b/test/plots/penguins-facet-annotated-x.js index 2c955729f1..f7d1ddb2e2 100644 --- a/test/plots/penguins-facet-annotated-x.js +++ b/test/plots/penguins-facet-annotated-x.js @@ -8,21 +8,13 @@ export default async function () { x: {insetRight: 10}, marks: [ Plot.frame(), - Plot.barX( - penguins, - Plot.groupY( - {x: "count"}, - { - y: "species", - fill: "sex", - fx: "island" - } - ) - ), - Plot.text(["Torgersen Island\nonly has\nAdelie\npenguins!"], { - frameAnchor: "right", - dx: -10, - fx: ["Torgersen"] + Plot.barX(penguins, Plot.groupY({x: "count"}, {fx: "island", y: "species", fill: "sex"})), + Plot.text(["Torgersen Island only has Adelie penguins!"], { + fx: ["Torgersen"], + frameAnchor: "top-right", + dy: 4, + dx: -4, + lineWidth: 10 }) ] }); diff --git a/test/plots/penguins-facet-annotated.js b/test/plots/penguins-facet-annotated.js index a8cd77d0c0..3f9bea5b21 100644 --- a/test/plots/penguins-facet-annotated.js +++ b/test/plots/penguins-facet-annotated.js @@ -10,21 +10,12 @@ export default async function () { facet: {marginRight: 70}, marks: [ Plot.frame(), - Plot.barX( - penguins, - Plot.groupY( - {x: "count"}, - { - y: "species", - fill: "sex", - fy: "island" - } - ) - ), - Plot.text(["↞ Torgersen Island only has Adelie penguins!"], { + Plot.barX(penguins, Plot.groupY({x: "count"}, {fy: "island", y: "species", fill: "sex"})), + Plot.text(["Torgersen Island only has Adelie penguins!"], { + fy: ["Torgersen"], frameAnchor: "top-right", - dx: -10, - fy: ["Torgersen"] + dy: 4, + dx: -4 }) ] }); From 0e641c825f534f6c92618c7f2b059aba3bede521 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 16:10:10 -0800 Subject: [PATCH 16/34] minimize diff --- src/facet.js | 120 ------------------------------------------------- src/plot.js | 123 +++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 119 insertions(+), 124 deletions(-) delete mode 100644 src/facet.js diff --git a/src/facet.js b/src/facet.js deleted file mode 100644 index 9f3cc1efab..0000000000 --- a/src/facet.js +++ /dev/null @@ -1,120 +0,0 @@ -import {group, intersection, sort, sum} from "d3"; -import {arrayify, range} from "./options.js"; -import {Channel} from "./channel.js"; -import {warn} from "./warnings.js"; - -// Facet filter, by mark; for now only the "eq" filter is provided. -export function filterFacets(facets, {fx, fy}) { - const X = fx != null && fx.value; - const Y = fy != null && fy.value; - const index = range(X || Y); - const gx = X && group(index, (i) => X[i]); - const gy = Y && group(index, (i) => Y[i]); - return X && Y - ? facets.map(({x, y}) => arrayify(intersection(gx.get(x) ?? [], gy.get(y) ?? []))) - : X - ? facets.map(({x}) => gx.get(x) ?? []) - : facets.map(({y}) => gy.get(y) ?? []); -} - -// Returns keys in order of the associated scale’s domains. -export function facetKeys(facets, {fx, fy}) { - const fxI = fx && new Map(fx.domain().map((x, i) => [x, i])); - const fyI = fy && new Map(fy.domain().map((y, i) => [y, i])); - return sort( - facets, - ({x: xa, y: ya}, {x: xb, y: yb}) => (fxI && fxI.get(xa) - fxI.get(xb)) || (fyI && fyI.get(ya) - fyI.get(yb)) - ); -} - -// Returns a (possibly nested) Map of [[key1, index1], [key2, index2], …] -// representing the data indexes associated with each facet. -export function facetGroups(index, {fx, fy}) { - return fx && fy ? facetGroup2(index, fx, fy) : fx ? facetGroup1(index, fx) : facetGroup1(index, fy); -} - -function facetGroup1(index, {value: F}) { - return group(index, (i) => F[i]); -} - -function facetGroup2(index, {value: FX}, {value: FY}) { - return group( - index, - (i) => FX[i], - (i) => FY[i] - ); -} - -export function facetTranslate(fx, fy) { - return fx && fy - ? ({x, y}) => `translate(${fx(x)},${fy(y)})` - : fx - ? ({x}) => `translate(${fx(x)},0)` - : ({y}) => `translate(0,${fy(y)})`; -} - -// Returns an index that for each facet lists all the elements present in other -// facets in the original index -export function excludeIndex(index) { - const ex = []; - const e = new Uint32Array(sum(index, (d) => d.length)); - for (const i of index) { - let n = 0; - for (const j of index) { - if (i === j) continue; - e.set(j, n); - n += j.length; - } - ex.push(e.slice(0, n)); - } - return ex; -} - -// Returns the facet groups, and possibly fx and fy channels, associated to the -// top-level facet options {data, x, y} -export function topFacetRead(facet) { - if (facet == null) return; - const {x, y} = facet; - if (x != null || y != null) { - const data = arrayify(facet.data); - if (data == null) throw new Error(`missing facet data`); // TODO strict equality - const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; - const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; - const groups = facetGroups(range(data), {fx, fy}); - // If the top-level faceting is non-trivial, track the corresponding data - // length, in order to compare it for the warning above. - const facetChannelLength = - groups.size > 1 || (fx && fy && groups.size === 1 && [...groups][0][1].size > 1) ? data.length : undefined; - return {groups, fx, fy, facetChannelLength}; - } -} - -// Returns the facet groups, and possibly fx and fy channels, associated to a -// mark, either through top-level faceting or mark-level facet options {fx, fy} -export function facetRead(mark, facetOptions, topFacetInfo) { - if (mark.facet === null) return; - - // This mark defines a mark-level facet. - const {fx: x, fy: y} = mark; - if (x != null || y != null) { - const data = arrayify(mark.data); - if (data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); // TODO strict equality - const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; - const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; - return {groups: facetGroups(range(data), {fx, fy}), fx, fy}; - } - - // This mark links to a top-level facet, if present. - if (topFacetInfo === undefined) return; - - const {groups, facetChannelLength} = topFacetInfo; - if (mark.facet !== "auto" || mark.data === facetOptions.data) return {groups}; - - // Warn for the common pitfall of wanting to facet mapped data. See - // above for the initialization of facetChannelLength. - if (facetChannelLength !== undefined && arrayify(mark.data)?.length === facetChannelLength) { - 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.` - ); - } -} diff --git a/src/plot.js b/src/plot.js index 5eda59213f..e7c94fcf1a 100644 --- a/src/plot.js +++ b/src/plot.js @@ -1,6 +1,6 @@ -import {ascending, cross, group, select, sort} from "d3"; +import {ascending, cross, group, intersection, sum, select, sort} from "d3"; import {Axes, autoAxisTicks, autoScaleLabels} from "./axes.js"; -import {Channels, channelDomain, valueObject} from "./channel.js"; +import {Channel, Channels, channelDomain, valueObject} from "./channel.js"; import {Context, create} from "./context.js"; import {defined} from "./defined.js"; import {Dimensions} from "./dimensions.js"; @@ -11,8 +11,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 {excludeIndex, facetKeys, facetTranslate, filterFacets, topFacetRead, facetRead} from "./facet.js"; +import {consumeWarnings, warn} from "./warnings.js"; /** @jsdoc plot */ export function plot(options = {}) { @@ -527,3 +526,119 @@ function nolabel(axis) { ? axis // use the existing axis if unlabeled : Object.assign(Object.create(axis), {label: undefined}); } + +// Returns keys in order of the associated scale’s domains. +function facetKeys(facets, {fx, fy}) { + const fxI = fx && new Map(fx.domain().map((x, i) => [x, i])); + const fyI = fy && new Map(fy.domain().map((y, i) => [y, i])); + return sort( + facets, + ({x: xa, y: ya}, {x: xb, y: yb}) => (fxI && fxI.get(xa) - fxI.get(xb)) || (fyI && fyI.get(ya) - fyI.get(yb)) + ); +} + +// Returns a (possibly nested) Map of [[key1, index1], [key2, index2], …] +// representing the data indexes associated with each facet. +function facetGroups(index, {fx, fy}) { + return fx && fy ? facetGroup2(index, fx, fy) : fx ? facetGroup1(index, fx) : facetGroup1(index, fy); +} + +function facetGroup1(index, {value: F}) { + return group(index, (i) => F[i]); +} + +function facetGroup2(index, {value: FX}, {value: FY}) { + return group( + index, + (i) => FX[i], + (i) => FY[i] + ); +} + +function facetTranslate(fx, fy) { + return fx && fy + ? ({x, y}) => `translate(${fx(x)},${fy(y)})` + : fx + ? ({x}) => `translate(${fx(x)},0)` + : ({y}) => `translate(0,${fy(y)})`; +} + +// Returns an index that for each facet lists all the elements present in other +// facets in the original index +function excludeIndex(index) { + const ex = []; + const e = new Uint32Array(sum(index, (d) => d.length)); + for (const i of index) { + let n = 0; + for (const j of index) { + if (i === j) continue; + e.set(j, n); + n += j.length; + } + ex.push(e.slice(0, n)); + } + return ex; +} + +// Returns the facet groups, and possibly fx and fy channels, associated to the +// top-level facet options {data, x, y} +function topFacetRead(facet) { + if (facet == null) return; + const {x, y} = facet; + if (x != null || y != null) { + const data = arrayify(facet.data); + if (data == null) throw new Error(`missing facet data`); // TODO strict equality + const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; + const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; + const groups = facetGroups(range(data), {fx, fy}); + // If the top-level faceting is non-trivial, track the corresponding data + // length, in order to compare it for the warning above. + const facetChannelLength = + groups.size > 1 || (fx && fy && groups.size === 1 && [...groups][0][1].size > 1) ? data.length : undefined; + return {groups, fx, fy, facetChannelLength}; + } +} + +// Returns the facet groups, and possibly fx and fy channels, associated to a +// mark, either through top-level faceting or mark-level facet options {fx, fy} +function facetRead(mark, facetOptions, topFacetInfo) { + if (mark.facet === null) return; + + // This mark defines a mark-level facet. + const {fx: x, fy: y} = mark; + if (x != null || y != null) { + const data = arrayify(mark.data); + if (data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); // TODO strict equality + const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; + const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; + return {groups: facetGroups(range(data), {fx, fy}), fx, fy}; + } + + // This mark links to a top-level facet, if present. + if (topFacetInfo === undefined) return; + + const {groups, facetChannelLength} = topFacetInfo; + if (mark.facet !== "auto" || mark.data === facetOptions.data) return {groups}; + + // Warn for the common pitfall of wanting to facet mapped data. See + // above for the initialization of facetChannelLength. + if (facetChannelLength !== undefined && arrayify(mark.data)?.length === facetChannelLength) { + 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.` + ); + } +} + +// Facet filter, by mark; for now only the "eq" filter is provided. +function filterFacets(facets, {fx, fy}) { + const X = fx != null && fx.value; + const Y = fy != null && fy.value; + const index = range(X || Y); + const gx = X && group(index, (i) => X[i]); + const gy = Y && group(index, (i) => Y[i]); + return X && Y + ? facets.map(({x, y}) => arrayify(intersection(gx.get(x) ?? [], gy.get(y) ?? []))) + : X + ? facets.map(({x}) => gx.get(x) ?? []) + : facets.map(({y}) => gy.get(y) ?? []); +} From ac90ac65287cc65ac788f254103848f67c19381f Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 16:15:30 -0800 Subject: [PATCH 17/34] keep top-level facet state separate --- src/plot.js | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/plot.js b/src/plot.js index e7c94fcf1a..8ecb2d72c6 100644 --- a/src/plot.js +++ b/src/plot.js @@ -29,28 +29,29 @@ export function plot(options = {}) { // Faceting! let facets; - // A map from top-level facet or mark to facet information, including: - // * groups - a possibly nested map from facet values to indexes in the data - // array - // * fx - a channel to add to the fx scale - // * fy - a channel to add to the fy scale - // * facetChannelLength - the top-level facet indicates a facet channel length - // to help warn the user if a different data of the same length is used in a - // mark - // * facetsIndex - In a second pass, a nested array of indices corresponding - // to the valid facets - const facetCollect = new Map(); + // A map from mark to facet state, including: + // groups - a possibly nested map from facet values to indexes in the data array + // fx - a channel to add to the fx scale + // fy - a channel to add to the fy scale + // facetsIndex - a nested array of indices corresponding to the valid facets + const facetStateByMark = new Map(); // Collect all facet definitions (top-level facets then mark facets), // materialize the associated channels, and derive facet scales. + // TODO Rename to topFacetState. + // TODO facetChannelLength - the top-level facet indicates a facet channel + // length to help warn the user if a different data of the same length is used + // in a mark const topFacetInfo = topFacetRead(facet); - if (topFacetInfo) facetCollect.set(null, topFacetInfo); + // TODO for (const mark of marks) { const f = facetRead(mark, facet, topFacetInfo); - if (f) facetCollect.set(mark, f); + if (f) facetStateByMark.set(mark, f); } - for (const f of facetCollect.values()) { + + // TODO + for (const f of [...facetStateByMark.values(), ...(topFacetInfo ? [topFacetInfo] : [])]) { const {fx, fy} = f; if (fx) { if (!channelsByScale.has("fx")) channelsByScale.set("fx", []); @@ -86,7 +87,7 @@ export function plot(options = {}) { const {facet} = mark; if (facet === null) continue; const {fx: x, fy: y} = mark; - const facetInfo = facetCollect.get(mark); + const facetInfo = facetStateByMark.get(mark); if (facetInfo === undefined) continue; // For mark-level facets, compute an index for that mark’s data and options. @@ -111,7 +112,7 @@ export function plot(options = {}) { // the domain. Expunge empty facets, and clear the corresponding elements // from the nested index in each mark. const nonEmpty = new Set(); - for (const {facetsIndex} of facetCollect.values()) { + for (const {facetsIndex} of facetStateByMark.values()) { if (facetsIndex) { facetsIndex.forEach((index, i) => { if (index?.length > 0) nonEmpty.add(i); @@ -120,7 +121,7 @@ export function plot(options = {}) { } if (0 < nonEmpty.size && nonEmpty.size < facets.length) { facets = facets.filter((_, i) => nonEmpty.has(i)); - for (const state of facetCollect.values()) { + for (const state of facetStateByMark.values()) { const {facetsIndex} = state; //console.warn(facetsIndex); if (!facetsIndex) continue; @@ -149,7 +150,7 @@ export function plot(options = {}) { // Initialize the marks’ state. for (const mark of marks) { if (stateByMark.has(mark)) throw new Error("duplicate mark; each mark must be unique"); - const state = facetCollect.get(mark) || {}; + const state = facetStateByMark.get(mark) || {}; const facetsIndex = mark.facet === "exclude" ? excludeIndex(state.facetsIndex) : state.facetsIndex; const {data, facets, channels} = mark.initialize(facetsIndex, state); applyScaleTransforms(channels, options); From c7ad01be42d321fe7098daae6bf04732415542b1 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 18:00:09 -0800 Subject: [PATCH 18/34] avoid re-initializing fx and fy channels --- src/channel.js | 6 +- src/plot.js | 168 ++++++++++++++++++++++++++++--------------------- 2 files changed, 98 insertions(+), 76 deletions(-) diff --git a/src/channel.js b/src/channel.js index 159e829dab..f6c74b703f 100644 --- a/src/channel.js +++ b/src/channel.js @@ -18,9 +18,9 @@ export function Channel(data, {scale, type, value, filter, hint}) { export function Channels(descriptors, data) { return Object.fromEntries( - Object.entries(descriptors).map(([name, channel]) => { - return [name, Channel(data, channel)]; - }) + Object.entries(descriptors) + .filter(([name]) => name !== "fx" && name !== "fy") // exclude facet channels + .map(([name, channel]) => [name, Channel(data, channel)]) ); } diff --git a/src/plot.js b/src/plot.js index 8ecb2d72c6..e841a7642a 100644 --- a/src/plot.js +++ b/src/plot.js @@ -23,54 +23,46 @@ export function plot(options = {}) { // Flatten any nested marks. const marks = options.marks === undefined ? [] : options.marks.flat(Infinity).map(markify); - // A Map from scale name to an array of associated channels. - const channelsByScale = new Map(); - - // Faceting! - let facets; + // Compute the top-level facet state. This has roughly the same structure as + // mark-specific facet state, except there isn’t a facetsIndex, and there’s a + // data and dataLength so we can warn the user if a different data of the same + // length is used in a mark. + const topFacetState = maybeTopFacet(facet); - // A map from mark to facet state, including: - // groups - a possibly nested map from facet values to indexes in the data array + // Construct a map from (faceted) Mark instance to facet state, including: // fx - a channel to add to the fx scale // fy - a channel to add to the fy scale + // groups - a possibly-nested map from facet values to indexes in the data array // facetsIndex - a nested array of indices corresponding to the valid facets const facetStateByMark = new Map(); - - // Collect all facet definitions (top-level facets then mark facets), - // materialize the associated channels, and derive facet scales. - // TODO Rename to topFacetState. - // TODO facetChannelLength - the top-level facet indicates a facet channel - // length to help warn the user if a different data of the same length is used - // in a mark - const topFacetInfo = topFacetRead(facet); - - // TODO for (const mark of marks) { - const f = facetRead(mark, facet, topFacetInfo); - if (f) facetStateByMark.set(mark, f); - } - - // TODO - for (const f of [...facetStateByMark.values(), ...(topFacetInfo ? [topFacetInfo] : [])]) { - const {fx, fy} = f; - if (fx) { - if (!channelsByScale.has("fx")) channelsByScale.set("fx", []); - channelsByScale.get("fx").push(fx); - } - if (fy) { - if (!channelsByScale.has("fy")) channelsByScale.set("fy", []); - channelsByScale.get("fy").push(fy); + const facetState = maybeMarkFacet(mark, topFacetState); + if (facetState) { + const facetChannels = {}; + if (facetState.fx) facetChannels.fx = facetState.fx; + if (facetState.fy) facetChannels.fy = facetState.fy; + applyScaleTransforms(facetChannels, options); + facetStateByMark.set(mark, facetState); } } - const facetScales = Scales(channelsByScale, options); + // Const a Map from scale name to an array of associated channels, but only + // for the fx and fy scales and channels, which are evaluated specially. + const facetChannelsByScale = new Map(); + if (topFacetState) addFacetChannels(facetChannelsByScale, topFacetState); + for (const facetState of facetStateByMark.values()) addFacetChannels(facetChannelsByScale, facetState); + + // Construct the initial facet scales. + const facetScaleDescriptors = Scales(facetChannelsByScale, options); + const facetScales = ScaleFunctions(facetScaleDescriptors); + let {fx, fy} = facetScales; // All the possible facets are given by the domains of fx or fy, or the // cross-product of these domains if we facet by both x and y. We sort them in // order to apply the facet filters afterwards. - const fxDomain = facetScales.fx?.scale.domain(); - const fyDomain = facetScales.fy?.scale.domain(); - facets = + let fxDomain = fx?.domain(); + let fyDomain = fy?.domain(); + let facets = fxDomain && fyDomain ? cross(sort(fxDomain, ascending), sort(fyDomain, ascending)).map(([x, y]) => ({x, y})) : fxDomain @@ -80,25 +72,25 @@ export function plot(options = {}) { : undefined; if (facets !== undefined) { - const facetsIndex = topFacetInfo ? filterFacets(facets, topFacetInfo) : undefined; + const facetsIndex = topFacetState ? filterFacets(facets, topFacetState) : undefined; // Compute a facet index for each mark, parallel to the facets array. for (const mark of marks) { const {facet} = mark; if (facet === null) continue; - const {fx: x, fy: y} = mark; const facetInfo = facetStateByMark.get(mark); if (facetInfo === undefined) continue; // For mark-level facets, compute an index for that mark’s data and options. - if (x !== undefined || y !== undefined) { + const {fx, fy} = mark; + if (fx !== undefined || fy !== undefined) { facetInfo.facetsIndex = filterFacets(facets, facetInfo); } // Otherwise, link to the top-level facet information. - else if (topFacetInfo !== undefined) { + else if (topFacetState !== undefined) { facetInfo.facetsIndex = facetsIndex; - const {fx, fy} = topFacetInfo; + const {fx, fy} = topFacetState; if (fx !== undefined) facetInfo.fx = fx; if (fy !== undefined) facetInfo.fy = fy; } @@ -123,17 +115,19 @@ export function plot(options = {}) { facets = facets.filter((_, i) => nonEmpty.has(i)); for (const state of facetStateByMark.values()) { const {facetsIndex} = state; - //console.warn(facetsIndex); if (!facetsIndex) continue; state.facetsIndex = facetsIndex.filter((_, i) => nonEmpty.has(i)); } } } + // A Map from scale name to an array of associated (non-facet) channels. + const channelsByScale = new Map(); + // If a scale is explicitly declared in options, initialize its associated // channels to the empty array; this will guarantee that a corresponding scale - // will be created later (even if there are no other channels). But ignore - // facet scale declarations if faceting is not enabled. + // will be created later (even if there are no other channels). Ignore facet + // scale declarations, which are handled above. for (const key of scaleRegistry.keys()) { if (isScaleOptions(options[key]) && key !== "fx" && key !== "fy") { channelsByScale.set(key, []); @@ -150,23 +144,36 @@ export function plot(options = {}) { // Initialize the marks’ state. for (const mark of marks) { if (stateByMark.has(mark)) throw new Error("duplicate mark; each mark must be unique"); - const state = facetStateByMark.get(mark) || {}; - const facetsIndex = mark.facet === "exclude" ? excludeIndex(state.facetsIndex) : state.facetsIndex; - const {data, facets, channels} = mark.initialize(facetsIndex, state); + const facetState = facetStateByMark.get(mark) || {}; + const facetsIndex = mark.facet === "exclude" ? excludeIndex(facetState.facetsIndex) : facetState.facetsIndex; + const {data, facets, channels} = mark.initialize(facetsIndex, facetState); // TODO Only pass {fx, fy}, not all facetState. applyScaleTransforms(channels, options); stateByMark.set(mark, {data, facets, channels}); } - // Initalize the scales and axes. + // TODO Don’t initialize the fx and fy scales twice. + for (const [key, channels] of facetChannelsByScale) channelsByScale.set(key, channels); + + // Initalize the scales. const scaleDescriptors = Scales(addScaleChannels(channelsByScale, stateByMark), options); const scales = ScaleFunctions(scaleDescriptors); + + // TODO Don’t initialize the fx and fy scales twice. + ({fx, fy} = scales); + fxDomain = fx?.domain(); + fyDomain = fy?.domain(); + + // Merge the non-facet scales with the previously-computed facet scales. + // Object.assign(scaleDescriptors, facetScaleDescriptors); + // Object.assign(scales, facetScales); + + // Intialize the axes. const axes = Axes(scaleDescriptors, options); const dimensions = Dimensions(scaleDescriptors, hasGeometry(stateByMark), axes, options); autoScaleRange(scaleDescriptors, dimensions); autoAxisTicks(scaleDescriptors, axes); - const {fx, fy} = scales; const fyMargins = fy && {marginTop: 0, marginBottom: 0, height: fy.bandwidth()}; const fxMargins = fx && {marginRight: 0, marginLeft: 0, width: fx.bandwidth()}; const subdimensions = {...dimensions, ...fxMargins, ...fyMargins}; @@ -198,8 +205,11 @@ export function plot(options = {}) { // Reconstruct scales if new scaled channels were created during reinitialization. if (newByScale.size) { - for (const key of newByScale) - if (scaleRegistry.get(key) === position) throw new Error(`initializers cannot declare position scales: ${key}`); + for (const key of newByScale) { + if (scaleRegistry.get(key) === position) { + throw new Error(`initializers cannot declare position scales: ${key}`); + } + } const newScaleDescriptors = Scales( addScaleChannels(new Map(), stateByMark, (key) => newByScale.has(key)), options @@ -209,6 +219,7 @@ export function plot(options = {}) { Object.assign(scales, newScales); } + // TODO Pass facetChannelsByScale here, separately? autoScaleLabels(channelsByScale, scaleDescriptors, axes, dimensions, options); // Compute value objects, applying scales and projection as needed. @@ -505,15 +516,25 @@ function addScaleChannels(channelsByScale, stateByMark, filter = yes) { const channel = channels[name]; const {scale} = channel; if (scale != null && filter(scale)) { - const channels = channelsByScale.get(scale); - if (channels !== undefined) channels.push(channel); - else channelsByScale.set(scale, [channel]); + addMapValue(channelsByScale, scale, channel); } } } return channelsByScale; } +function addFacetChannels(channelsByScale, facetState) { + const {fx, fy} = facetState; + if (fx) addMapValue(channelsByScale, "fx", fx); + if (fy) addMapValue(channelsByScale, "fy", fy); +} + +function addMapValue(map, key, value) { + const values = map.get(key); + if (values !== undefined) values.push(value); + else map.set(key, [value]); +} + function hasGeometry(stateByMark) { for (const {channels} of stateByMark.values()) { if (channels.geometry) return true; @@ -540,7 +561,8 @@ function facetKeys(facets, {fx, fy}) { // Returns a (possibly nested) Map of [[key1, index1], [key2, index2], …] // representing the data indexes associated with each facet. -function facetGroups(index, {fx, fy}) { +function facetGroups(data, fx, fy) { + const index = range(data); return fx && fy ? facetGroup2(index, fx, fy) : fx ? facetGroup1(index, fx) : facetGroup1(index, fy); } @@ -565,7 +587,7 @@ function facetTranslate(fx, fy) { } // Returns an index that for each facet lists all the elements present in other -// facets in the original index +// facets in the original index. function excludeIndex(index) { const ex = []; const e = new Uint32Array(sum(index, (d) => d.length)); @@ -581,49 +603,49 @@ function excludeIndex(index) { return ex; } -// Returns the facet groups, and possibly fx and fy channels, associated to the -// top-level facet options {data, x, y} -function topFacetRead(facet) { +// Returns the facet groups, and possibly fx and fy channels, associated with +// the top-level facet option {data, x, y}. +function maybeTopFacet(facet) { if (facet == null) return; const {x, y} = facet; if (x != null || y != null) { const data = arrayify(facet.data); - if (data == null) throw new Error(`missing facet data`); // TODO strict equality + if (data == null) throw new Error(`missing facet data`); const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; - const groups = facetGroups(range(data), {fx, fy}); + const groups = facetGroups(data, fx, fy); // If the top-level faceting is non-trivial, track the corresponding data // length, in order to compare it for the warning above. - const facetChannelLength = + const dataLength = groups.size > 1 || (fx && fy && groups.size === 1 && [...groups][0][1].size > 1) ? data.length : undefined; - return {groups, fx, fy, facetChannelLength}; + return {fx, fy, groups, data: facet.data, dataLength}; } } -// Returns the facet groups, and possibly fx and fy channels, associated to a -// mark, either through top-level faceting or mark-level facet options {fx, fy} -function facetRead(mark, facetOptions, topFacetInfo) { +// Returns the facet groups, and possibly fx and fy channels, associated with a +// mark, either through top-level faceting or mark-level facet options {fx, fy}. +function maybeMarkFacet(mark, topFacetState) { if (mark.facet === null) return; // This mark defines a mark-level facet. const {fx: x, fy: y} = mark; if (x != null || y != null) { const data = arrayify(mark.data); - if (data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); // TODO strict equality + if (data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; - return {groups: facetGroups(range(data), {fx, fy}), fx, fy}; + return {fx, fy, groups: facetGroups(data, fx, fy)}; } // This mark links to a top-level facet, if present. - if (topFacetInfo === undefined) return; + if (topFacetState === undefined) return; - const {groups, facetChannelLength} = topFacetInfo; - if (mark.facet !== "auto" || mark.data === facetOptions.data) return {groups}; + const {groups, data, dataLength} = topFacetState; + if (mark.facet !== "auto" || mark.data === data) return {groups}; - // Warn for the common pitfall of wanting to facet mapped data. See - // above for the initialization of facetChannelLength. - if (facetChannelLength !== undefined && arrayify(mark.data)?.length === facetChannelLength) { + // Warn for the common pitfall of wanting to facet mapped data. See above for + // the initialization of dataLength. + if (dataLength !== undefined && arrayify(mark.data)?.length === dataLength) { 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.` ); From df9098fd6a543eeb16d68382edcfd84650d11523 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 18:05:38 -0800 Subject: [PATCH 19/34] TODO re. applyScaleTransforms --- src/plot.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/plot.js b/src/plot.js index e841a7642a..5c06b4a443 100644 --- a/src/plot.js +++ b/src/plot.js @@ -37,13 +37,7 @@ export function plot(options = {}) { const facetStateByMark = new Map(); for (const mark of marks) { const facetState = maybeMarkFacet(mark, topFacetState); - if (facetState) { - const facetChannels = {}; - if (facetState.fx) facetChannels.fx = facetState.fx; - if (facetState.fy) facetChannels.fy = facetState.fy; - applyScaleTransforms(facetChannels, options); - facetStateByMark.set(mark, facetState); - } + if (facetState) facetStateByMark.set(mark, facetState); } // Const a Map from scale name to an array of associated channels, but only @@ -611,6 +605,7 @@ function maybeTopFacet(facet) { if (x != null || y != null) { const data = arrayify(facet.data); if (data == null) throw new Error(`missing facet data`); + // TODO Call applyScaleTransforms on the fx and fy channels. const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; const groups = facetGroups(data, fx, fy); @@ -632,6 +627,7 @@ function maybeMarkFacet(mark, topFacetState) { if (x != null || y != null) { const data = arrayify(mark.data); if (data == null) throw new Error(`missing facet data in ${mark.ariaLabel}`); + // TODO Call applyScaleTransforms on the fx and fy channels. const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; return {fx, fy, groups: facetGroups(data, fx, fy)}; From d271b2c64759ef2c133781d7cf3945154b628337 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 18:07:14 -0800 Subject: [PATCH 20/34] prettier --- src/plot.js | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/plot.js b/src/plot.js index 5c06b4a443..ebab896dd1 100644 --- a/src/plot.js +++ b/src/plot.js @@ -602,19 +602,18 @@ function excludeIndex(index) { function maybeTopFacet(facet) { if (facet == null) return; const {x, y} = facet; - if (x != null || y != null) { - const data = arrayify(facet.data); - if (data == null) throw new Error(`missing facet data`); - // TODO Call applyScaleTransforms on the fx and fy channels. - const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; - const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; - const groups = facetGroups(data, fx, fy); - // If the top-level faceting is non-trivial, track the corresponding data - // length, in order to compare it for the warning above. - const dataLength = - groups.size > 1 || (fx && fy && groups.size === 1 && [...groups][0][1].size > 1) ? data.length : undefined; - return {fx, fy, groups, data: facet.data, dataLength}; - } + if (x == null && y == null) return; + const data = arrayify(facet.data); + if (data == null) throw new Error(`missing facet data`); + // TODO Call applyScaleTransforms on the fx and fy channels. + const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; + const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; + const groups = facetGroups(data, fx, fy); + // If the top-level faceting is non-trivial, track the corresponding data + // length, in order to compare it for the warning above. + const dataLength = + groups.size > 1 || (fx && fy && groups.size === 1 && [...groups][0][1].size > 1) ? data.length : undefined; + return {fx, fy, groups, data: facet.data, dataLength}; } // Returns the facet groups, and possibly fx and fy channels, associated with a From b677fe74c8671742b04c3761a7a387db4276eedb Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 19:41:37 -0800 Subject: [PATCH 21/34] adopt InternMap; remove sorting --- src/plot.js | 94 ++++++++++++++++++++++------------------------------- 1 file changed, 38 insertions(+), 56 deletions(-) diff --git a/src/plot.js b/src/plot.js index ebab896dd1..477700803a 100644 --- a/src/plot.js +++ b/src/plot.js @@ -1,4 +1,4 @@ -import {ascending, cross, group, intersection, sum, select, sort} from "d3"; +import {cross, group, intersection, sum, select, sort, InternMap} from "d3"; import {Axes, autoAxisTicks, autoScaleLabels} from "./axes.js"; import {Channel, Channels, channelDomain, valueObject} from "./channel.js"; import {Context, create} from "./context.js"; @@ -40,30 +40,15 @@ export function plot(options = {}) { if (facetState) facetStateByMark.set(mark, facetState); } - // Const a Map from scale name to an array of associated channels, but only - // for the fx and fy scales and channels, which are evaluated specially. - const facetChannelsByScale = new Map(); - if (topFacetState) addFacetChannels(facetChannelsByScale, topFacetState); - for (const facetState of facetStateByMark.values()) addFacetChannels(facetChannelsByScale, facetState); - - // Construct the initial facet scales. - const facetScaleDescriptors = Scales(facetChannelsByScale, options); - const facetScales = ScaleFunctions(facetScaleDescriptors); - let {fx, fy} = facetScales; - - // All the possible facets are given by the domains of fx or fy, or the - // cross-product of these domains if we facet by both x and y. We sort them in - // order to apply the facet filters afterwards. - let fxDomain = fx?.domain(); - let fyDomain = fy?.domain(); - let facets = - fxDomain && fyDomain - ? cross(sort(fxDomain, ascending), sort(fyDomain, ascending)).map(([x, y]) => ({x, y})) - : fxDomain - ? sort(fxDomain, ascending).map((x) => ({x})) - : fyDomain - ? sort(fyDomain, ascending).map((y) => ({y})) - : undefined; + // Compute a Map from scale name to an array of associated channels. + const channelsByScale = new Map(); + if (topFacetState) addFacetChannels(channelsByScale, topFacetState); + for (const facetState of facetStateByMark.values()) addFacetChannels(channelsByScale, facetState); + + // All the possible facets are given by the domains of the fx or fy scales, or + // the cross-product of these domains if we facet by both x and y. We sort + // them in order to apply the facet filters afterwards. + let facets = Facets(channelsByScale, options); if (facets !== undefined) { const facetsIndex = topFacetState ? filterFacets(facets, topFacetState) : undefined; @@ -115,9 +100,6 @@ export function plot(options = {}) { } } - // A Map from scale name to an array of associated (non-facet) channels. - const channelsByScale = new Map(); - // If a scale is explicitly declared in options, initialize its associated // channels to the empty array; this will guarantee that a corresponding scale // will be created later (even if there are no other channels). Ignore facet @@ -145,29 +127,16 @@ export function plot(options = {}) { stateByMark.set(mark, {data, facets, channels}); } - // TODO Don’t initialize the fx and fy scales twice. - for (const [key, channels] of facetChannelsByScale) channelsByScale.set(key, channels); - - // Initalize the scales. + // Initalize the scales and axes. const scaleDescriptors = Scales(addScaleChannels(channelsByScale, stateByMark), options); const scales = ScaleFunctions(scaleDescriptors); - - // TODO Don’t initialize the fx and fy scales twice. - ({fx, fy} = scales); - fxDomain = fx?.domain(); - fyDomain = fy?.domain(); - - // Merge the non-facet scales with the previously-computed facet scales. - // Object.assign(scaleDescriptors, facetScaleDescriptors); - // Object.assign(scales, facetScales); - - // Intialize the axes. const axes = Axes(scaleDescriptors, options); const dimensions = Dimensions(scaleDescriptors, hasGeometry(stateByMark), axes, options); autoScaleRange(scaleDescriptors, dimensions); autoAxisTicks(scaleDescriptors, axes); + const {fx, fy} = scales; const fyMargins = fy && {marginTop: 0, marginBottom: 0, height: fy.bandwidth()}; const fxMargins = fx && {marginRight: 0, marginLeft: 0, width: fx.bandwidth()}; const subdimensions = {...dimensions, ...fxMargins, ...fyMargins}; @@ -260,8 +229,8 @@ export function plot(options = {}) { // Render (possibly faceted) marks. if (facets !== undefined) { - const fyDomain = fy && fy.domain(); - const fxDomain = fx && fx.domain(); + const fxDomain = fx?.domain(); + const fyDomain = fy?.domain(); const selection = select(svg); if (fy && axes.y) { const axis1 = axes.y, @@ -333,7 +302,7 @@ export function plot(options = {}) { const facetPosition = new Map(facets.map((f, j) => [f, j])); selection .selectAll() - .data(facetKeys(facets, {fx, fy})) + .data(facetKeys(facets, fx, fy)) .enter() .append("g") .attr("aria-label", "facet") @@ -543,14 +512,27 @@ function nolabel(axis) { : Object.assign(Object.create(axis), {label: undefined}); } -// Returns keys in order of the associated scale’s domains. -function facetKeys(facets, {fx, fy}) { - const fxI = fx && new Map(fx.domain().map((x, i) => [x, i])); - const fyI = fy && new Map(fy.domain().map((y, i) => [y, i])); - return sort( - facets, - ({x: xa, y: ya}, {x: xb, y: yb}) => (fxI && fxI.get(xa) - fxI.get(xb)) || (fyI && fyI.get(ya) - fyI.get(yb)) - ); +// Returns an array of {x?, y?} objects representing the facet domain. +function Facets(channelsByScale, options) { + const {fx, fy} = Scales(channelsByScale, options); + let fxDomain = fx?.scale.domain(); // TODO .sort(ascending)? + let fyDomain = fy?.scale.domain(); // TODO .sort(ascending)? + return fxDomain && fyDomain + ? cross(fxDomain, fyDomain).map(([x, y]) => ({x, y})) + : fxDomain + ? fxDomain.map((x) => ({x})) + : fyDomain + ? fyDomain.map((y) => ({y})) + : undefined; +} + +// Returns keys in order of the associated scale’s domains. (We don’t want to +// recompute the keys here because facets may already be filtered, and facets +// isn’t sorted because it’s constructed prior to the other mark channels.) +function facetKeys(facets, fx, fy) { + const fxI = fx && new InternMap(fx.domain().map((x, i) => [x, i])); + const fyI = fy && new InternMap(fy.domain().map((y, i) => [y, i])); + return sort(facets, (a, b) => (fxI && fxI.get(a.x) - fxI.get(b.x)) || (fyI && fyI.get(a.y) - fyI.get(b.y))); } // Returns a (possibly nested) Map of [[key1, index1], [key2, index2], …] @@ -649,8 +631,8 @@ function maybeMarkFacet(mark, topFacetState) { // Facet filter, by mark; for now only the "eq" filter is provided. function filterFacets(facets, {fx, fy}) { - const X = fx != null && fx.value; - const Y = fy != null && fy.value; + const X = fx?.value; + const Y = fy?.value; const index = range(X || Y); const gx = X && group(index, (i) => X[i]); const gy = Y && group(index, (i) => Y[i]); From 63480fbff13f278aa599fdb2aca760dc2dadeca6 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 19:51:18 -0800 Subject: [PATCH 22/34] nullish, not undefined --- src/plot.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/plot.js b/src/plot.js index 477700803a..242639d3ce 100644 --- a/src/plot.js +++ b/src/plot.js @@ -55,14 +55,12 @@ export function plot(options = {}) { // Compute a facet index for each mark, parallel to the facets array. for (const mark of marks) { - const {facet} = mark; - if (facet === null) continue; + if (mark.facet === null) continue; const facetInfo = facetStateByMark.get(mark); if (facetInfo === undefined) continue; // For mark-level facets, compute an index for that mark’s data and options. - const {fx, fy} = mark; - if (fx !== undefined || fy !== undefined) { + if (mark.fx != null || mark.fy != null) { facetInfo.facetsIndex = filterFacets(facets, facetInfo); } From a46f447d377ff8297c667efcd8a8ef7db8581987 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 19:53:23 -0800 Subject: [PATCH 23/34] style --- src/plot.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/plot.js b/src/plot.js index 242639d3ce..ca8fe27f9a 100644 --- a/src/plot.js +++ b/src/plot.js @@ -56,20 +56,20 @@ export function plot(options = {}) { // Compute a facet index for each mark, parallel to the facets array. for (const mark of marks) { if (mark.facet === null) continue; - const facetInfo = facetStateByMark.get(mark); - if (facetInfo === undefined) continue; + const facetState = facetStateByMark.get(mark); + if (facetState === undefined) continue; // For mark-level facets, compute an index for that mark’s data and options. if (mark.fx != null || mark.fy != null) { - facetInfo.facetsIndex = filterFacets(facets, facetInfo); + facetState.facetsIndex = filterFacets(facets, facetState); } // Otherwise, link to the top-level facet information. else if (topFacetState !== undefined) { - facetInfo.facetsIndex = facetsIndex; + facetState.facetsIndex = facetsIndex; const {fx, fy} = topFacetState; - if (fx !== undefined) facetInfo.fx = fx; - if (fy !== undefined) facetInfo.fy = fy; + if (fx !== undefined) facetState.fx = fx; + if (fy !== undefined) facetState.fy = fy; } } @@ -82,11 +82,11 @@ export function plot(options = {}) { // from the nested index in each mark. const nonEmpty = new Set(); for (const {facetsIndex} of facetStateByMark.values()) { - if (facetsIndex) { - facetsIndex.forEach((index, i) => { - if (index?.length > 0) nonEmpty.add(i); - }); - } + facetsIndex?.forEach((index, i) => { + if (index?.length > 0) { + nonEmpty.add(i); + } + }); } if (0 < nonEmpty.size && nonEmpty.size < facets.length) { facets = facets.filter((_, i) => nonEmpty.has(i)); From 8e328950a42f3d456dbd62e3d9d0bc99ad62e17a Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 1 Dec 2022 20:05:14 -0800 Subject: [PATCH 24/34] remove TODO --- src/plot.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plot.js b/src/plot.js index ca8fe27f9a..c56a19388d 100644 --- a/src/plot.js +++ b/src/plot.js @@ -180,7 +180,6 @@ export function plot(options = {}) { Object.assign(scales, newScales); } - // TODO Pass facetChannelsByScale here, separately? autoScaleLabels(channelsByScale, scaleDescriptors, axes, dimensions, options); // Compute value objects, applying scales and projection as needed. From 173af3cee9d1a919ab03f2ab543f31be5d8b8b16 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Fri, 2 Dec 2022 08:14:37 -0800 Subject: [PATCH 25/34] =?UTF-8?q?fx=20and=20fy=20aren=E2=80=99t=20in=20mar?= =?UTF-8?q?k.channels?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/channel.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/channel.js b/src/channel.js index f6c74b703f..6adcb1715d 100644 --- a/src/channel.js +++ b/src/channel.js @@ -17,11 +17,7 @@ export function Channel(data, {scale, type, value, filter, hint}) { } export function Channels(descriptors, data) { - return Object.fromEntries( - Object.entries(descriptors) - .filter(([name]) => name !== "fx" && name !== "fy") // exclude facet channels - .map(([name, channel]) => [name, Channel(data, channel)]) - ); + return Object.fromEntries(Object.entries(descriptors).map(([name, channel]) => [name, Channel(data, channel)])); } // TODO Use Float64Array for scales with numeric ranges, e.g. position? From 6092cf7c3e6201d9c0b7ae1977357fb014d61fb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 8 Dec 2022 11:31:19 +0100 Subject: [PATCH 26/34] remove todo --- src/plot.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plot.js b/src/plot.js index f0cdea4821..ea58806e0d 100644 --- a/src/plot.js +++ b/src/plot.js @@ -533,8 +533,8 @@ function nolabel(axis) { // Returns an array of {x?, y?} objects representing the facet domain. function Facets(channelsByScale, options) { const {fx, fy} = Scales(channelsByScale, options); - let fxDomain = fx?.scale.domain(); // TODO .sort(ascending)? - let fyDomain = fy?.scale.domain(); // TODO .sort(ascending)? + const fxDomain = fx?.scale.domain(); + const fyDomain = fy?.scale.domain(); return fxDomain && fyDomain ? cross(fxDomain, fyDomain).map(([x, y]) => ({x, y})) : fxDomain From 7406b795192f4b8e6ed43708d3ddec293593485b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 8 Dec 2022 11:31:28 +0100 Subject: [PATCH 27/34] fix comment --- src/plot.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plot.js b/src/plot.js index ea58806e0d..1cb60deafc 100644 --- a/src/plot.js +++ b/src/plot.js @@ -609,8 +609,8 @@ function maybeTopFacet(facet) { const fx = x != null ? Channel(data, {value: x, scale: "fx"}) : undefined; const fy = y != null ? Channel(data, {value: y, scale: "fy"}) : undefined; const groups = facetGroups(data, fx, fy); - // If the top-level faceting is non-trivial, track the corresponding data - // length, in order to compare it for the warning above. + // When the top-level facet option generated several frames, track the + // corresponding data length in order to compare it for the warning above. const dataLength = groups.size > 1 || (fx && fy && groups.size === 1 && [...groups][0][1].size > 1) ? data.length : undefined; return {fx, fy, groups, data: facet.data, dataLength}; From 7954d0c5a4496c486742a033fe13ac26cc9f1682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 8 Dec 2022 11:31:48 +0100 Subject: [PATCH 28/34] use groups --- src/plot.js | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/plot.js b/src/plot.js index 1cb60deafc..f2b349435f 100644 --- a/src/plot.js +++ b/src/plot.js @@ -1,4 +1,4 @@ -import {cross, group, intersection, sum, select, sort, InternMap} from "d3"; +import {cross, group, sum, select, sort, InternMap} from "d3"; import {Axes, autoAxisTicks, autoScaleLabels} from "./axes.js"; import {Channel, Channels, channelDomain, valueObject} from "./channel.js"; import {Context, create} from "./context.js"; @@ -648,15 +648,10 @@ function maybeMarkFacet(mark, topFacetState) { } // Facet filter, by mark; for now only the "eq" filter is provided. -function filterFacets(facets, {fx, fy}) { - const X = fx?.value; - const Y = fy?.value; - const index = range(X || Y); - const gx = X && group(index, (i) => X[i]); - const gy = Y && group(index, (i) => Y[i]); - return X && Y - ? facets.map(({x, y}) => arrayify(intersection(gx.get(x) ?? [], gy.get(y) ?? []))) - : X - ? facets.map(({x}) => gx.get(x) ?? []) - : facets.map(({y}) => gy.get(y) ?? []); +function filterFacets(facets, {fx, fy, groups}) { + return fx && fy + ? facets.map(({x, y}) => groups.get(x)?.get(y) ?? []) + : fx + ? facets.map(({x}) => groups.get(x) ?? []) + : facets.map(({y}) => groups.get(y) ?? []); } From 39bb3d424137b99edec11e965422ab4962926a77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 8 Dec 2022 11:31:02 +0100 Subject: [PATCH 29/34] tighten state --- src/plot.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/plot.js b/src/plot.js index f2b349435f..3f7c73bccb 100644 --- a/src/plot.js +++ b/src/plot.js @@ -119,9 +119,8 @@ export function plot(options = {}) { // Initialize the marks’ state. for (const mark of marks) { if (stateByMark.has(mark)) throw new Error("duplicate mark; each mark must be unique"); - const facetState = facetStateByMark.get(mark) || {}; - const facetsIndex = mark.facet === "exclude" ? excludeIndex(facetState.facetsIndex) : facetState.facetsIndex; - const {data, facets, channels} = mark.initialize(facetsIndex, facetState); // TODO Only pass {fx, fy}, not all facetState. + const {facetsIndex: I, ...state} = facetStateByMark.get(mark) || {}; + const {data, facets, channels} = mark.initialize(mark.facet === "exclude" ? excludeIndex(I) : I, state); applyScaleTransforms(channels, options); stateByMark.set(mark, {data, facets, channels}); } @@ -396,7 +395,7 @@ export class Mark { if (facets === undefined && data != null) facets = [range(data)]; if (this.transform != null) ({facets, data} = this.transform(data, facets)), (data = arrayify(data)); const channels = Channels(this.channels, data); - if (this.sort != null) channelDomain(channels, facetChannels, data, this.sort); + if (this.sort != null) channelDomain(channels, facetChannels, data, this.sort); // mutates facetChannels! return {data, facets, channels}; } filter(index, channels, values) { From 01bcd784ccede3cb18aab9d35439afe29c379812 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 8 Dec 2022 07:26:43 -0800 Subject: [PATCH 30/34] sparse facetsIndex --- src/plot.js | 22 ++++++++++++++++------ test/output/penguinFacetAnnotated.svg | 2 -- test/output/penguinFacetAnnotatedX.svg | 2 -- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/plot.js b/src/plot.js index 3f7c73bccb..e3e7f63fa6 100644 --- a/src/plot.js +++ b/src/plot.js @@ -34,7 +34,7 @@ export function plot(options = {}) { // fx - a channel to add to the fx scale // fy - a channel to add to the fy scale // groups - a possibly-nested map from facet values to indexes in the data array - // facetsIndex - a nested array of indices corresponding to the valid facets + // facetsIndex - a sparse nested array of indices corresponding to the valid facets const facetStateByMark = new Map(); for (const mark of marks) { const facetState = maybeMarkFacet(mark, topFacetState); @@ -313,14 +313,24 @@ export function plot(options = {}) { .attr("transform", facetTranslate(fx, fy)) .each(function (key) { for (const [mark, {channels, values, facets}] of stateByMark) { - const facet = facets ? mark.filter(facets[facetPosition.get(key)] ?? facets[0], channels, values) : null; + let facet = null; + if (facets) { + facet = facets[facetPosition.get(key)] ?? facets[0]; + if (!facet) continue; + facet = mark.filter(facet, channels, values); + } const node = mark.render(facet, scales, values, subdimensions, context); if (node != null) this.appendChild(node); } }); } else { for (const [mark, {channels, values, facets}] of stateByMark) { - const facet = facets ? mark.filter(facets[0], channels, values) : null; + let facet = null; + if (facets) { + facet = facets[0]; + if (!facet) continue; + facet = mark.filter(facet, channels, values); + } const node = mark.render(facet, scales, values, dimensions, context); if (node != null) svg.appendChild(node); } @@ -649,8 +659,8 @@ function maybeMarkFacet(mark, topFacetState) { // Facet filter, by mark; for now only the "eq" filter is provided. function filterFacets(facets, {fx, fy, groups}) { return fx && fy - ? facets.map(({x, y}) => groups.get(x)?.get(y) ?? []) + ? facets.map(({x, y}) => groups.get(x)?.get(y)) : fx - ? facets.map(({x}) => groups.get(x) ?? []) - : facets.map(({y}) => groups.get(y) ?? []); + ? facets.map(({x}) => groups.get(x)) + : facets.map(({y}) => groups.get(y)); } diff --git a/test/output/penguinFacetAnnotated.svg b/test/output/penguinFacetAnnotated.svg index a7500b4bbd..22ea2d9faf 100644 --- a/test/output/penguinFacetAnnotated.svg +++ b/test/output/penguinFacetAnnotated.svg @@ -89,7 +89,6 @@ - @@ -100,7 +99,6 @@ - diff --git a/test/output/penguinFacetAnnotatedX.svg b/test/output/penguinFacetAnnotatedX.svg index a3ae5fed67..0544b94b53 100644 --- a/test/output/penguinFacetAnnotatedX.svg +++ b/test/output/penguinFacetAnnotatedX.svg @@ -77,7 +77,6 @@ - @@ -88,7 +87,6 @@ - From d1e902aa2f79d28b890b51facc4f65b5c58597e6 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Fri, 9 Dec 2022 09:55:38 -0800 Subject: [PATCH 31/34] simpler facetsIndex initialization --- src/plot.js | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/plot.js b/src/plot.js index 2dc8740ec7..cdf5d5b765 100644 --- a/src/plot.js +++ b/src/plot.js @@ -31,8 +31,7 @@ export function plot(options = {}) { const topFacetState = maybeTopFacet(facet, options); // Construct a map from (faceted) Mark instance to facet state, including: - // fx - a channel to add to the fx scale - // fy - a channel to add to the fy scale + // channels - an {fx?, fy?} object to add to the fx and fy scale // groups - a possibly-nested map from facet values to indexes in the data array // facetsIndex - a sparse nested array of indices corresponding to the valid facets const facetStateByMark = new Map(); @@ -54,23 +53,14 @@ export function plot(options = {}) { if (facets !== undefined) { const topFacetsIndex = topFacetState ? filterFacets(facets, topFacetState) : undefined; - // Compute a facet index for each mark, parallel to the facets array. + // Compute a facet index for each mark, parallel to the facets array. For + // mark-level facets, compute an index for that mark’s data and options. + // Otherwise, use the top-level facet index. for (const mark of marks) { if (mark.facet === null) continue; const facetState = facetStateByMark.get(mark); if (facetState === undefined) continue; - - // For mark-level facets, compute an index for that mark’s data and options. - if (mark.fx != null || mark.fy != null) { - facetState.facetsIndex = filterFacets(facets, facetState); - } - - // Otherwise, link to the top-level facet information. - // TODO What is this doing, exactly? - else if (topFacetState !== undefined) { - facetState.facetsIndex = topFacetsIndex; - facetState.channels = topFacetState.channels; - } + facetState.facetsIndex = mark.fx != null || mark.fy != null ? filterFacets(facets, facetState) : topFacetsIndex; } // The cross product of the domains of fx and fy can include fx-fy @@ -632,7 +622,8 @@ function maybeTopFacet(facet, options) { function maybeMarkFacet(mark, topFacetState, options) { if (mark.facet === null) return; - // This mark defines a mark-level facet. + // This mark defines a mark-level facet. TODO There’s some code duplication + // here with maybeTopFacet that we could reduce. const {fx: x, fy: y} = mark; if (x != null || y != null) { const data = arrayify(mark.data); @@ -647,8 +638,9 @@ function maybeMarkFacet(mark, topFacetState, options) { // This mark links to a top-level facet, if present. if (topFacetState === undefined) return; - const {groups, data, dataLength} = topFacetState; - if (mark.facet !== "auto" || mark.data === data) return {groups}; + // TODO Can we link the top-level facet channels here? + const {channels, groups, data, dataLength} = topFacetState; + if (mark.facet !== "auto" || mark.data === data) return {channels, groups}; // Warn for the common pitfall of wanting to facet mapped data. See above for // the initialization of dataLength. From eb368683c4723638615fb742472b8db80bdd1001 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Fri, 9 Dec 2022 10:08:25 -0800 Subject: [PATCH 32/34] small consolidation --- src/plot.js | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/plot.js b/src/plot.js index cdf5d5b765..897213e1ce 100644 --- a/src/plot.js +++ b/src/plot.js @@ -225,6 +225,16 @@ export function plot(options = {}) { const fxDomain = fx?.domain(); const fyDomain = fy?.domain(); const selection = select(svg); + // When faceting by both fx and fy, this nested Map allows to look up the + // non-empty facets and draw the grid lines properly. + const fxy = + fx && fy && (axes.x || axes.y) + ? group( + facets, + ({x}) => x, + ({y}) => y + ) + : undefined; if (fy && axes.y) { const axis1 = axes.y, axis2 = nolabel(axis1); @@ -234,23 +244,13 @@ export function plot(options = {}) { : axis1.labelAnchor === "center" ? fyDomain.length >> 1 : 0; - - // When faceting by both fx and fy, this nested Map allows to look up the - // non-empty facets and draw the grid lines properly. - const cx = - fx && - group( - facets, - ({y}) => y, - ({x}) => x - ); selection .selectAll() .data(fyDomain) .enter() .append((ky, i) => (i === j ? axis1 : axis2).render( - cx && where(fxDomain, (kx) => cx.get(ky).has(kx)), + fx && where(fxDomain, (kx) => fxy.get(kx).has(ky)), scales, {...dimensions, ...fyMargins, offsetTop: fy(ky)}, context @@ -262,13 +262,6 @@ export function plot(options = {}) { axis2 = nolabel(axis1); const j = axis1.labelAnchor === "right" ? fxDomain.length - 1 : axis1.labelAnchor === "center" ? fxDomain.length >> 1 : 0; - const cy = - fy && - group( - facets, - ({x}) => x, - ({y}) => y - ); const {marginLeft, marginRight} = dimensions; selection .selectAll() @@ -276,7 +269,7 @@ export function plot(options = {}) { .enter() .append((kx, i) => (i === j ? axis1 : axis2).render( - cy && where(fyDomain, (ky) => cy.get(kx).has(ky)), + fy && where(fyDomain, (ky) => fxy.get(kx).has(ky)), scales, { ...dimensions, From 2ec8d083894ebd8eb77bd72e4a1f2dbed2b51ab7 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Fri, 9 Dec 2022 10:15:26 -0800 Subject: [PATCH 33/34] compute exclude facet index earlier --- src/plot.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/plot.js b/src/plot.js index 897213e1ce..367c184b62 100644 --- a/src/plot.js +++ b/src/plot.js @@ -86,6 +86,14 @@ export function plot(options = {}) { state.facetsIndex = facetsIndex.filter((_, i) => nonEmpty.has(i)); } } + + // For any mark using the “exclude” facet mode, invert the index. + for (const mark of marks) { + if (mark.facet === "exclude") { + const facetState = facetStateByMark.get(mark); + facetState.facetsIndex = excludeIndex(facetState.facetsIndex); + } + } } // If a scale is explicitly declared in options, initialize its associated @@ -108,8 +116,8 @@ export function plot(options = {}) { // Initialize the marks’ state. for (const mark of marks) { if (stateByMark.has(mark)) throw new Error("duplicate mark; each mark must be unique"); - const {facetsIndex: I, channels: facetChannels} = facetStateByMark.get(mark) || {}; - const {data, facets, channels} = mark.initialize(mark.facet === "exclude" ? excludeIndex(I) : I, facetChannels); + const {facetsIndex, channels: facetChannels} = facetStateByMark.get(mark) || {}; + const {data, facets, channels} = mark.initialize(facetsIndex, facetChannels); applyScaleTransforms(channels, options); stateByMark.set(mark, {data, facets, channels}); } @@ -572,7 +580,7 @@ function facetTranslate(fx, fy) { } // Returns an index that for each facet lists all the elements present in other -// facets in the original index. +// facets in the original index. TODO Memoize to avoid repeated work? function excludeIndex(index) { const ex = []; const e = new Uint32Array(sum(index, (d) => d.length)); From 4e9b4a97ef988a488f1b6b6de72e11e434b22cc9 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Fri, 9 Dec 2022 10:20:18 -0800 Subject: [PATCH 34/34] simpler facet channels collection --- src/plot.js | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/plot.js b/src/plot.js index 367c184b62..ecab470bc3 100644 --- a/src/plot.js +++ b/src/plot.js @@ -42,8 +42,8 @@ export function plot(options = {}) { // Compute a Map from scale name to an array of associated channels. const channelsByScale = new Map(); - if (topFacetState) addFacetChannels(channelsByScale, topFacetState); - for (const facetState of facetStateByMark.values()) addFacetChannels(channelsByScale, facetState); + if (topFacetState) addScaleChannels(channelsByScale, [topFacetState]); + addScaleChannels(channelsByScale, facetStateByMark); // All the possible facets are given by the domains of the fx or fy scales, or // the cross-product of these domains if we facet by both x and y. We sort @@ -496,25 +496,15 @@ function addScaleChannels(channelsByScale, stateByMark, filter = yes) { const channel = channels[name]; const {scale} = channel; if (scale != null && filter(scale)) { - addMapValue(channelsByScale, scale, channel); + const scaleChannels = channelsByScale.get(scale); + if (scaleChannels !== undefined) scaleChannels.push(channel); + else channelsByScale.set(scale, [channel]); } } } return channelsByScale; } -function addFacetChannels(channelsByScale, {channels}) { - for (const key in channels) { - addMapValue(channelsByScale, key, channels[key]); - } -} - -function addMapValue(map, key, value) { - const values = map.get(key); - if (values !== undefined) values.push(value); - else map.set(key, [value]); -} - function hasGeometry(stateByMark) { for (const {channels} of stateByMark.values()) { if (channels.geometry) return true;