Skip to content

Commit 4ff6504

Browse files
committed
Raise an error when a reducer expects numeric input but is given non-numeric input
Add tests for the normalize transform closes #473
1 parent 5670bbf commit 4ff6504

File tree

7 files changed

+87
-8
lines changed

7 files changed

+87
-8
lines changed

src/mark.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,3 +340,14 @@ export function isTemporal(values) {
340340
return value instanceof Date;
341341
}
342342
}
343+
344+
function isNumeric(values) {
345+
for (const value of values) {
346+
if (value == null) continue;
347+
return !isNaN(+value);
348+
}
349+
}
350+
351+
export function checkNumeric(S) {
352+
if (!isNumeric(S)) throw new Error("the reducer expects numeric input");
353+
}

src/transforms/group.js

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {group as grouper, sort, sum, deviation, min, max, mean, median, mode, variance, InternSet} from "d3";
22
import {firstof} from "../defined.js";
3-
import {valueof, maybeColor, maybeInput, maybeTransform, maybeTuple, maybeLazyChannel, lazyChannel, first, identity, take, labelof, range} from "../mark.js";
3+
import {valueof, maybeColor, maybeInput, maybeTransform, maybeTuple, maybeLazyChannel, lazyChannel, first, identity, take, labelof, range, checkNumeric} from "../mark.js";
44

55
// Group on {z, fill, stroke}.
66
export function groupZ(outputs, options) {
@@ -143,12 +143,12 @@ export function maybeReduce(reduce, value) {
143143
case "sum": return value == null ? reduceCount : reduceSum;
144144
case "proportion": return reduceProportion(value, "data");
145145
case "proportion-facet": return reduceProportion(value, "facet");
146-
case "deviation": return reduceAccessor(deviation);
146+
case "deviation": return reduceAccessor(deviation, true);
147147
case "min": return reduceAccessor(min);
148148
case "max": return reduceAccessor(max);
149-
case "mean": return reduceAccessor(mean);
150-
case "median": return reduceAccessor(median);
151-
case "variance": return reduceAccessor(variance);
149+
case "mean": return reduceAccessor(mean, true);
150+
case "median": return reduceAccessor(median, true);
151+
case "variance": return reduceAccessor(variance, true);
152152
case "mode": return reduceAccessor(mode);
153153
}
154154
throw new Error("invalid reduce");
@@ -170,9 +170,10 @@ function reduceFunction(f) {
170170
};
171171
}
172172

173-
function reduceAccessor(f) {
173+
function reduceAccessor(f, check) {
174174
return {
175175
reduce(I, X) {
176+
if (check) checkNumeric(X);
176177
return f(I, i => X[i]);
177178
}
178179
};
@@ -212,7 +213,7 @@ const reduceDistinct = {
212213
}
213214
};
214215

215-
const reduceSum = reduceAccessor(sum);
216+
const reduceSum = reduceAccessor(sum, true);
216217

217218
function reduceProportion(value, scope) {
218219
return value == null

src/transforms/normalize.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {extent, mean, median, sum} from "d3";
22
import {defined} from "../defined.js";
3-
import {take} from "../mark.js";
3+
import {checkNumeric, take} from "../mark.js";
44
import {mapX, mapY} from "./map.js";
55

66
export function normalizeX({basis, ...options} = {}) {
@@ -28,6 +28,7 @@ function normalize(basis) {
2828
function normalizeBasis(basis) {
2929
return {
3030
map(I, S, T) {
31+
checkNumeric(S);
3132
const b = +basis(I, S);
3233
for (const i of I) {
3334
T[i] = S[i] === null ? NaN : S[i] / b;
@@ -38,6 +39,7 @@ function normalizeBasis(basis) {
3839

3940
const normalizeExtent = {
4041
map(I, S, T) {
42+
checkNumeric(S);
4143
const [s1, s2] = extent(I, i => S[i]), d = s2 - s1;
4244
for (const i of I) {
4345
T[i] = S[i] === null ? NaN : (S[i] - s1) / d;

src/transforms/window.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {mapX, mapY} from "./map.js";
22
import {deviation, max, min, median, variance} from "d3";
3+
import {checkNumeric} from "../mark.js";
34

45
export function windowX({k, reduce, shift, ...options} = {}) {
56
return mapX(window(k, reduce, shift), options);
@@ -45,6 +46,7 @@ function maybeReduce(reduce = "mean") {
4546
function reduceSubarray(f) {
4647
return (k, s) => ({
4748
map(I, S, T) {
49+
checkNumeric(S);
4850
const C = Float64Array.from(I, i => S[i] === null ? NaN : S[i]);
4951
let nans = 0;
5052
for (let i = 0; i < k - 1; ++i) if (isNaN(C[i])) ++nans;
@@ -60,6 +62,7 @@ function reduceSubarray(f) {
6062
function reduceSum(k, s) {
6163
return {
6264
map(I, S, T) {
65+
checkNumeric(S);
6366
let nans = 0;
6467
let sum = 0;
6568
for (let i = 0; i < k - 1; ++i) {
@@ -95,6 +98,7 @@ function reduceMean(k, s) {
9598
function reduceDifference(k, s) {
9699
return {
97100
map(I, S, T) {
101+
checkNumeric(S);
98102
for (let i = 0, n = I.length - k; i < n; ++i) {
99103
const a = S[I[i]];
100104
const b = S[I[i + k - 1]];
@@ -107,6 +111,7 @@ function reduceDifference(k, s) {
107111
function reduceRatio(k, s) {
108112
return {
109113
map(I, S, T) {
114+
checkNumeric(S);
110115
for (let i = 0, n = I.length - k; i < n; ++i) {
111116
const a = S[I[i]];
112117
const b = S[I[i + k - 1]];

test/transforms/normalize-test.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import * as Plot from "@observablehq/plot";
2+
import assert from "assert";
3+
4+
it("normalizeX normalizes as expected", () => {
5+
const data = [2, 10, 8];
6+
testNormalize(data, undefined, [1, 5, 4]);
7+
testNormalize(data, "first", [1, 5, 4]);
8+
testNormalize(data, "last", [0.25, 1.25, 1]);
9+
testNormalize(data, "mean", [0.3, 1.5, 1.2]);
10+
testNormalize(data, "sum", [0.1, 0.5, 0.4]);
11+
});
12+
13+
it("normalizeX throws on non-numeric values", () => {
14+
const data = ["A", 10, 8];
15+
testNormalizeThrows(data);
16+
testNormalizeThrows(data, "first");
17+
testNormalizeThrows(data, "last");
18+
testNormalizeThrows(data, "mean");
19+
testNormalizeThrows(data, "sum");
20+
});
21+
22+
function testNormalize(data, basis, r) {
23+
const mark = Plot.dot(data, Plot.normalizeX({x: data, basis}));
24+
const c = new Map(mark.initialize().channels);
25+
assert.deepStrictEqual(c.get("x").value, r);
26+
}
27+
28+
function testNormalizeThrows(data, basis) {
29+
const mark = Plot.dot(data, Plot.normalizeX({x: data, basis}));
30+
assert.throws(() => mark.initialize());
31+
}

test/transforms/reduce-test.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ it("baked-in reducers reduce as expected", () => {
1010
testReducer(data, "min", 0);
1111
testReducer(data, "sum", 21);
1212
testReducer(data, "variance", 10.7);
13+
testReducer(data, "mode", 0);
14+
});
15+
16+
it("baked-in non-numeric reducers throw on non-numeric data", () => {
17+
const data = ["A", "B", "C", "B"];
18+
testReducer(data, "min", "A");
19+
testReducer(data, "max", "C");
20+
testReducer(data, "mode", "B");
1321
});
1422

1523
it("function reducers reduce as expected", () => {
@@ -18,8 +26,22 @@ it("function reducers reduce as expected", () => {
1826
testReducer(data, v => v.join(", "), "0, 1, 2, 4, 5, 9");
1927
});
2028

29+
it("baked-in numeric reducers throw on non-numeric data", () => {
30+
const data = ["A", "B", "C", "A"];
31+
testReducerThrows(data, "deviation");
32+
testReducerThrows(data, "mean");
33+
testReducerThrows(data, "median");
34+
testReducerThrows(data, "sum");
35+
testReducerThrows(data, "variance");
36+
});
37+
2138
function testReducer(data, x, r) {
2239
const mark = Plot.dot(data, Plot.groupZ({x}, {x: d => d}));
2340
const c = new Map(mark.initialize().channels);
2441
assert.deepStrictEqual(c.get("x").value, [r]);
2542
}
43+
44+
function testReducerThrows(data, x) {
45+
const mark = Plot.dot(data, Plot.groupZ({x}, {x: d => d}));
46+
assert.throws(() => mark.initialize());
47+
}

test/transforms/windowMean-test.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,10 @@ it("movingAverage respects shift", () => {
4747
assert.deepStrictEqual(mt.x.transform(), [,, 1, 2, 3, 4]);
4848
});
4949

50+
it("movingAverage throws on non-numeric data", () => {
51+
const data = ["A", 1, 2, 3, 4, 5];
52+
const mc = Plot.windowX({k: 3, x: d => d});
53+
assert.throws(() => {
54+
mc.transform(data, [d3.range(data.length)]);
55+
});
56+
});

0 commit comments

Comments
 (0)