d3 / d3-interpolate

Interpolate numbers, colors, strings, arrays, objects, whatever!
https://d3js.org/d3-interpolate
ISC License
494 stars 69 forks source link

Interpolate typed array folded #74

Closed Fil closed 4 years ago

Fil commented 4 years ago

This variant of https://github.com/d3/d3-interpolate/pull/65 folds interpolateTypedArray into interpolateArray.

The only part I find weird is in README, where we explain that no defensive copy of the template array is made, nor, in the case of typed array, do we have defensive copies of a and b. Not sure if this is intelligible at all.

mbostock commented 4 years ago

My main objection is now the name.

“interpolateTypedArray” will happily interpolate (generic) arrays of numbers; there’s no requirement that the arrays be typed. So I argue that “interpolateNumberArray” is a better name.

(It’s still fine that d3.interpolate detects typed arrays and uses that to dispatch to interpolateNumberArray instead of interpolateArray; that’s just a heuristic for detecting arrays of numbers. And one that’s cheaper than walking through the entire array and checking the type of every element.)

Also, interpolateTypedArray doesn’t work with all typed arrays. Specifically: BigInt64Array and BigUint64Array. You’ll get a TypeError: Cannot mix BigInt and other types, use explicit conversions. If we use interpolateNumberArray, this is more reasonable because the typeof is bigint and not number, and we could have an interpolateBigIntArray if we wanted.

Fil commented 4 years ago

Implies interpolateNumberArray will be exposed, so we can call it directly when we know we have only numbers? I'm fine with that (and will start over in a new branch).

I think it would also be nice to detect the situation in which an array contains only numbers and switch to interpolateNumberArray. Checking b.every(i => typeof i == "number") is probably reasonable? A small price when false, and a big gain when true.

mbostock commented 4 years ago

Yep, interpolateNumberArray would be exposed.

I dunno, do you think we need to do the type check on elements in the generic interpolate? I feel like if you care about the performance and it’s ambiguous you can just opt-in by calling interpolateNumberArray directly.