Skip to content

composable transform - #193

Merged
mbostock merged 31 commits into
mainfrom
mbostock/compose-transform
Mar 8, 2021
Merged

mbostock merged 31 commits into
mainfrom
mbostock/compose-transform

Conversation

@mbostock

@mbostock mbostock commented Mar 5, 2021

Copy link
Copy Markdown
Member

Quick sketch of how a transform could return channels to override, so that it can be more easily composed with other transforms. Still a work in progress…

Fixes #192.
Fixes #184.
Fixes #183.
Fixes #201.

@mbostock
mbostock requested a review from Fil March 5, 2021 00:02
@mbostock

mbostock commented Mar 5, 2021

Copy link
Copy Markdown
Member Author

Note to self: I also want to pass [I] as the facets argument to transforms so that they don’t have to be conditional on whether the data is faceted. In other words let’s treat the no-facet case as a single facet that contains all the data.

@Fil

Fil commented Mar 5, 2021

Copy link
Copy Markdown
Contributor

I'm a bit lost in the code, with the Map<->Object conversions.

I've found ways to compose transforms like this:

Plot.line([[0,1], [0,2], [1,3], [3, 4]], {
      transform: PlotTransforms([
        data => data.map((d,i) => d.map((u,j) => u + i/10 + j/100)), // modify
        data => data.filter((d,i) => i != 1), // filter
        (data, facets, channels) => { mutable debug = {data, facets, channels}; }, // log
        data => d3.shuffle(data.slice()) // sort
      ])
  })

and the transformed are executed in the given order.

However I am at a loss when I want to compose Plot.stackY and Plot.binX (as in https://next.observablehq.com/@data-workflows/parallel-binning-183-3#binXstackY ), which would need to access channels and modify the index?

pass [I] as the facets

Agree it would simplify things a lot! It's been a recurring point when writing plugins. Here also I thought it would be simple but for some reason I failed to find where to apply the logic.

Maybe I need more coffee, or better sleep. In the meantime I'll try and tackle simpler tasks :)

@mbostock

mbostock commented Mar 5, 2021

Copy link
Copy Markdown
Member Author

I added a normalize transform and support for transform composition, and incorporated this new technique into the index chart example which now looks like this:

Plot.plot({
  y: {
    type: "log",
    grid: true,
    label: "↑ Change in price (%)"
  },
  marks: [
    Plot.ruleY([1]),
    Plot.line(stocks, {
      transform: Plot.normalizeY(),
      x: "Date",
      y: "Close",
      stroke: "Symbol"
    }),
    Plot.text(stocks, {
      transform: [Plot.normalizeY(), Plot.selectLast()],
      x: "Date",
      y: "Close",
      z: "Symbol",
      text: "Symbol",
      textAnchor: "start",
      dx: 3
    })
  ]
})

I haven’t updated the other transforms (stack, bin, group) yet so many things are still broken.

The main idea here is that transforms now take the same arguments that they return: data, index, channels. This means that you can compose them by running a series of transforms iteratively. The index argument is the old facets argument, except in the case where there are no facets, the value is [range(data)] i.e. a single facet with all the data. The channels argument is an object from channel name to channel value. This allows transforms to reference the channel definitions, to greedily compute channel values using the supplied data, and to return new channel definitions that are then passed to marks to render (or to subsequent transforms).

Since transforms now have now read and write channel definitions, we don’t need to wrap the mark options anymore; instead of specify the transform as the transform option. You can use either a single transform function or an array of transform functions.

I removed support for “basic” (non-facet-aware) transforms so as to simplify the code, and because these seemed error-prone when channel definitions are provided as precomputed arrays of values: such a data transform cannot be expressed as a mapping from data ↦ data. Transforms must provide an index, or they must return new channel definitions as appropriate. See the rewritten select transform as an example. We can also provide helper methods such as Plot.map and Plot.filter for some simple transform cases that will do the right thing while preserving index semantics.

@mbostock

mbostock commented Mar 6, 2021

Copy link
Copy Markdown
Member Author

This is going pretty well, but I’m now left with the two aggregation transforms, bin and group. These are trickier because what you do with channel values that are specified as an array? Since the data is aggregated, specifying a channel fill: data.map(d => d.foo) is not the same as fill: d => d.foo (or fill: "foo"): the former is computed before on the input data, while the latter is computed after on the aggregated bins. Perhaps we don’t need to worry about this, but I think these transforms may want to rewrite (take) any input channel arrays and pull out an arbitrary (likely the first?) value for each bin. Or I suppose we could throw an error, or reset these channels to be undefined. If we do nothing, then when rendering we’ll pull arbitrary values from these channel arrays which could be misleading.

@mbostock

mbostock commented Mar 6, 2021

Copy link
Copy Markdown
Member Author

Just ran into a little wall. The following won’t work because Plot.rectY thinks the x1, x2, y1, and y2 channels are missing (because those channels are defined by the transform, which happens on mark initialization, after construction) and it ignores the x channel (because Plot.rectY doesn’t support that channel — it’s specific to Plot.binX):

Plot.rectY(data, {transform: Plot.binX({normalize: true}), x: d => Math.log10(d.Volume)})

So, I think we want to go back to the previous approach (which is shorter besides)…

Plot.rectY(data, Plot.binX({x: d => Math.log10(d.Volume), normalize: true}))

but, with the addition that Plot.binX and all the other transforms will support a transform option to support composition. So if you want multiple transforms, you just do nested calls, e.g.,

Plot.text(stocks, Plot.selectLast(Plot.normalizeY({
  x: "Date",
  y: "Close",
  z: "Symbol",
  text: "Symbol",
  textAnchor: "start",
  dx: 3
})))

If we do want to separate mark options from transform options, we could always pass a second options object to the transform. But at least for now that doesn’t feel necessary.

Hopefully I’ll have time to try this tomorrow.

@mbostock

mbostock commented Mar 6, 2021

Copy link
Copy Markdown
Member Author

Okay, I’ve mostly reverted back to the previous design, but added transform composition and tidied up the non-faceted case. I’m feeling good about this. Still need to add support for transform composition to the aggregation transforms and decide how to handle channel value arrays.

@Fil

Fil commented Mar 6, 2021

Copy link
Copy Markdown
Contributor

http://localhost:8008/?test=mobyDickLetterPosition crashes because there are two keys in the group (x, y).

This patch works (but a bit ugly, so I don't push it):

diff --git a/src/transforms/group.js b/src/transforms/group.js
index f5a599b..9663d6b 100644
--- a/src/transforms/group.js
+++ b/src/transforms/group.js
@@ -59,7 +59,7 @@ function regroup(groups, facets) {

 function subset(facet) {
   const f = new Set(facet);
-  return ([key, group]) => [key, group.filter(i => f.has(i))];
+  return ([key, group1, group2]) => [key, group2 ? group1 : group1.filter(i => f.has(i)), group2 && group2.filter(i => f.has(i))];
 }

@mbostock

mbostock commented Mar 6, 2021

Copy link
Copy Markdown
Member Author

Yep, things should work now though. Marking this as ready.

@mbostock
mbostock marked this pull request as ready for review March 6, 2021 20:47
@mbostock mbostock mentioned this pull request Mar 6, 2021
@Fil

Fil commented Mar 6, 2021

Copy link
Copy Markdown
Contributor

Is plot.BinX accepting z? I couldn't make it work. (I'm trying to do https://observablehq.com/@data-workflows/parallel-binning-183-3 with this branch: i.e. binX with z => stackY => rectY.)

work notebook: https://observablehq.com/d/52395f0dc21003b5

@mbostock

mbostock commented Mar 6, 2021

Copy link
Copy Markdown
Member Author

No, I haven’t implemented z for the bin transform yet, but thanks for the reminder. I will do that.

@mbostock

mbostock commented Mar 7, 2021

Copy link
Copy Markdown
Member Author

So, I’m running into some problems composing bin + stack.

  1. The binX transform computes x1, x2, and y channels, but stackY needs an x channel to determine the stack locations. An easy fix for this to have the stackY channel fallback to the x1 channel if x is not defined. Alternatively, the binX transform could return an x channel that is equal to x1 (or the average of x1 and x2 if we want to be fancy).

  2. The rectY mark is not usable with the output of stackY, since the rectY mark takes x1, x2, and y channels as input, but stackY produces x, y1, y2 (passing through x1 and x2). You can workaround this by using the rect mark instead of rectY, but I think rectY should behave the same as rect if y1 and y2 are specified explicitly rather than rectY forcing y1 to always be zero. I’ve fixed this in 3026473.

  3. If the fill (or any z) channel is used to partition bins prior to stacking, the bin partition must compute the aggregated fill channel so that it is available to rendering. If the fill channel is not materialized, then we’ll try to compute it from the bins (e.g., bins.map(d => d.species)) which will typically return undefined (or even crash). So, the bin transform will need to know how the bins are being partitioned, and materialize the channels accordingly. Probably any of z, fill, and stroke need to be materialized (and if more than one is defined, I guess they’re all computed on the raw data, and the first value is chosen for the bin, similar to how we compute stroke for lines if z is also specified).

This last issue is the hardest to fix, but I think it’s doable.

@mbostock

mbostock commented Mar 7, 2021

Copy link
Copy Markdown
Member Author

(We’ll need to do this for the group transform, too.)

* z bin

* z, x1 inheritance

* stacked bin test

* use rectY

* more TODO

* x1, y1 inheritance

* bins as grouped data

* fix x inheritance

* bin1 normalize

* simplify

* reorder

* bin-z for 2D bins (#198)

* generealize z-bin1 to z-bin2
example http://localhost:8008/?test=penguinSexMassCulmenSpecies

remove unused functions

* allow separate thresholds_x and thresholds_y
(most useful when we want to specify them as arrays of values)

* a grid to match the binning

* tweaks

Co-authored-by: Mike Bostock <mbostock@gmail.com>

Co-authored-by: Philippe Rivière <fil@rezo.net>
@mbostock

mbostock commented Mar 8, 2021

Copy link
Copy Markdown
Member Author

Okay! Almost ready! Just need to rewrite the group transform to match the new bin transform, and add a test for group + stack.

@mbostock
mbostock merged commit 8f9332b into main Mar 8, 2021
@mbostock
mbostock deleted the mbostock/compose-transform branch March 8, 2021 22:40
@Fil

Fil commented Mar 9, 2021

Copy link
Copy Markdown
Contributor

In https://observablehq.com/@data-workflows/plot-line#highlight we now have to write
transform: data => ({data: d3.sort(data, highlight), index: [d3.range(data.length)] })

instead of transform: data => d3.sort(data, highlight) previously.

@mbostock

mbostock commented Mar 9, 2021

Copy link
Copy Markdown
Member Author

I’ll make a TODO. That could also be a top-level sort option #180, but technically you only need to sort the series here, not the individual data points… Hmm.

@Fil

Fil commented Mar 9, 2021

Copy link
Copy Markdown
Contributor

I was just mentioning it since that example in Plot.line had crashed. We could also create a Plot.sort helper that would sort the facet's index if necessary. I'm more interested in adding a {filter} option (or a Plot.filter(…) helper), as discussed in #194.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A domain option for the group transform? Normalization transform? Composable transforms? Stacked bins (Plot.stackY + Plot.binX + Plot.rectY)?

2 participants