autoSpec

[tophtucker]

Factors out the parts of the auto mark that fill in its options. Produces an object that you can pass as a fully-specified Plot.auto options object.

As an optimization, there are two versions:

• the internal spec function optionally returns the materialized columns, so that Plot.auto doesn't have to re-materialize them in order to determine the implementation of the marks
• the exported autoSpec function, which calls spec but doesn't return the materialized columns

So, calling Plot.auto will still only materialize columns once; calling Plot.auto(data, Plot.autoSpec(data, options)) will materialize them twice.

Optional to-dos, I'm not sure they're necessary for now:

• Investigate making all undefineds into nulls
• More tests. I have a couple just for autoSpec but am still mostly relying on the usual mark snapshots
• Bring zero and z assignments up from auto to autoSpec?

[Fil]

I'd suggest we make the tests much more stringent, by comparing their serialized version to a template (which can be in a separate file on disk, or in the unit test page).

For example instead of testing

it("Plot.autoSpec makes a histogram from a quantitative dimension", () => {
  const data = [{value: 1}, {value: 1}, {value: 38}];
  const A = Plot.autoSpec(data, {x: "value"});
  assert.strictEqual(A.y.reduce, "count");
  assert.strictEqual(A.mark, "bar");
});

we do:

it("Plot.autoSpec makes a histogram from a quantitative dimension", () => {
  assert.strictEqual(JSON.stringify(Plot.autoSpec([{value: 1}, {value: 1}, {value: 38}], {x: "value"})), `{ expected spec }`);
});

of course this could be a nice function if we want to keep it readable.

[mbostock]

Why not use assert.deepStrictEqual?

[mbostock]

What is this change doing?

[tophtucker]

At one point in refactoring, I didn't have a reference to sizeValue here, and it seemed inconsistent to refer to x and y (not xValue and yValue) below, but sizeValue (not size) here, so I changed it. Upon further reflection, if sizeValue is undefined / null, then size will be undefined / null, so it doesn't matter. I'll revert this.

[mbostock]

I think it’ll be clearer if we store the materialized columns as a separate array, rather than redefining local variables/function arguments. I’ll incorporate into this into a commit I’m about to push.

[mbostock]

A different way we can do this (without requiring an internal option and separate function) is to materialize the values as arrays, and then pass them into autoSpec; this way valueof does nothing (it returns the existing arrays). I’ll take a crack at this and push a commit if it works.

[mbostock]

I think it might be clearer to return null here to fully realize the defaults. I’ll try that and see.

[tophtucker]

TIL "??=". Thanks for simplifying back to one function and for tackling the "return something that's defined one way or the other" thing!