From 3a3d54a99d3627b52208a5bfc6c61b0c8f393caf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Fri, 2 Jun 2023 13:38:30 +0200 Subject: [PATCH] normalizes the calls to valueof to three arguments: *element*, *index*, and *data* previously *data* was present in some cases (when going through the *arraytype*.map code path) but not others (e.g. when doing type conversion) Addresses this comment in https://github.com/observablehq/plot/pull/1665#issuecomment-1572742907 --- docs/features/transforms.md | 2 +- src/channel.d.ts | 2 +- src/options.js | 16 ++++++++++++---- test/options-test.js | 9 +++++++++ 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/docs/features/transforms.md b/docs/features/transforms.md index 8da856d77b..8c70d6d65e 100644 --- a/docs/features/transforms.md +++ b/docs/features/transforms.md @@ -216,7 +216,7 @@ Plot.valueof(aapl, "Close") Given an iterable *data* and some *value* accessor, returns an array (a column) of the specified *type* with the corresponding value of each element of the data. The *value* accessor may be one of the following types: * a string - corresponding to the field accessor (`(d) => d[value]`) -* an accessor function - called as *type*.from(*data*, *value*) +* an accessor function - called on each element as *value*(*element, *index*, *data*) * a number, Date, or boolean — resulting in an array uniformly filled with the *value* * an object with a **transform** method — called as *value*.transform(*data*) * an array of values - returning the same diff --git a/src/channel.d.ts b/src/channel.d.ts index e88bb77c2a..ecc8ff69e0 100644 --- a/src/channel.d.ts +++ b/src/channel.d.ts @@ -137,7 +137,7 @@ export type ChannelValue = | number // constant | boolean // constant | null // constant - | ((d: any, i: number) => any) // function of data + | ((d: any, i: number, values: any[]) => any) // function of data | ChannelTransform; // function of data /** diff --git a/src/options.js b/src/options.js index 0a1603eaa7..1a087778d3 100644 --- a/src/options.js +++ b/src/options.js @@ -35,7 +35,7 @@ function maybeTypedArrayify(data, type) { } function floater(f) { - return (d, i) => coerceNumber(f(d, i)); + return (d, i, v) => coerceNumber(f(d, i, v)); } export const singleton = [null]; // for data-less decoration marks, e.g. frame @@ -127,10 +127,18 @@ export function arrayify(data) { return data == null || data instanceof Array || data instanceof TypedArray ? data : Array.from(data); } -// An optimization of type.from(values, f): if the given values are already an -// instanceof the desired array type, the faster values.map method is used. +// A generalization of values.map(f) with type conversion: if the given values +// are already an instanceof the desired array type, the faster values.map +// method is used, otherwise type.from is used. If f accepts a third argument, +// it receives the values. export function map(values, f, type = Array) { - return values == null ? values : values instanceof type ? values.map(f) : type.from(values, f); + return values == null + ? values + : values instanceof type + ? values.map(f) + : f.length === 3 + ? type.from(values, (d, i) => f(d, i, values)) + : type.from(values, f); } // An optimization of type.from(values): if the given values are already an diff --git a/test/options-test.js b/test/options-test.js index 4b96b06372..4b699a7d58 100644 --- a/test/options-test.js +++ b/test/options-test.js @@ -122,6 +122,15 @@ it("valueof returns the given array value", () => { assert.strictEqual(valueof([], a), a); }); +it("valueof passes the data as a third argument to function accessors", () => { + const a = (d, i, e) => e.length; + assert.deepStrictEqual(valueof([1, 2], a, Float64Array), Float64Array.of(2, 2)); + assert.deepStrictEqual(valueof(Float64Array.of(2, 2), a), [2, 2]); + assert.deepStrictEqual(valueof(Float64Array.of(2, 2), a, Uint8Array), Uint8Array.of(2, 2)); + assert.deepStrictEqual(valueof([1, 2], a, Uint8Array), Uint8Array.of(2, 2)); + assert.deepStrictEqual(valueof([1, 2], a, Array), [2, 2]); +}); + it("valueof accepts complicated data with the proper accessor", () => { const m = [(d) => d, new Promise(() => {})]; assert.deepStrictEqual(valueof(m, String), ["(d) => d", "[object Promise]"]);