Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

null, undefined, NaN and -Infinity create holes #61

Merged
merged 9 commits into from
Jan 8, 2023
Merged

null, undefined, NaN and -Infinity create holes #61

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
80daf12
null, undefined, NaN and -Infinity create holes. Ignore infinite valu…
Fil Jul 7, 2022
b429650
Update src/contours.js
Fil Jul 11, 2022
d8d8f3f
post-review
Fil Jul 11, 2022
a1fa540
revert unrelated style changes
mbostock Jan 8, 2023
3b45fd2
nullish
mbostock Jan 8, 2023
7b1f992
extract closure
mbostock Jan 8, 2023
877511b
a few comments
mbostock Jan 8, 2023
15e0cff
another comment
mbostock Jan 8, 2023
e0cc18b
another comment
mbostock Jan 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 22 additions & 12 deletions src/contours.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export default function() {

// Convert number of thresholds into uniform thresholds.
if (!Array.isArray(tz)) {
const e = extent(values), ts = tickStep(e[0], e[1], tz);
const e = extent(values, d => isFinite(d) ? d : null), ts = tickStep(e[0], e[1], tz);
tz = ticks(Math.floor(e[0] / ts) * ts, Math.floor(e[1] / ts - 1) * ts, tz);
} else {
tz = tz.slice().sort(ascending);
Expand Down Expand Up @@ -80,25 +80,30 @@ export default function() {
fragmentByEnd = new Array,
x, y, t0, t1, t2, t3;

function above(index) {
const x = values[index];
return x == null || isNaN(x) ? false : x >= value;
Fil marked this conversation as resolved.
Show resolved Hide resolved
}

// Special case for the first row (y = -1, t2 = t3 = 0).
x = y = -1;
t1 = values[0] >= value;
t1 = above(0);
cases[t1 << 1].forEach(stitch);
while (++x < dx - 1) {
t0 = t1, t1 = values[x + 1] >= value;
t0 = t1, t1 = above(x + 1);
cases[t0 | t1 << 1].forEach(stitch);
}
cases[t1 << 0].forEach(stitch);

// General case for the intermediate rows.
while (++y < dy - 1) {
x = -1;
t1 = values[y * dx + dx] >= value;
t2 = values[y * dx] >= value;
t1 = above(y * dx + dx);
t2 = above(y * dx);
cases[t1 << 1 | t2 << 2].forEach(stitch);
while (++x < dx - 1) {
t0 = t1, t1 = values[y * dx + dx + x + 1] >= value;
t3 = t2, t2 = values[y * dx + x + 1] >= value;
t0 = t1, t1 = above(y * dx + dx + x + 1);
t3 = t2, t2 = above(y * dx + x + 1);
cases[t0 | t1 << 1 | t2 << 2 | t3 << 3].forEach(stitch);
}
cases[t1 | t2 << 3].forEach(stitch);
Expand All @@ -109,7 +114,7 @@ export default function() {
t2 = values[y * dx] >= value;
cases[t2 << 2].forEach(stitch);
while (++x < dx - 1) {
t3 = t2, t2 = values[y * dx + x + 1] >= value;
t3 = t2, t2 = above(y * dx + x + 1);
cases[t2 << 2 | t3 << 3].forEach(stitch);
}
cases[t2 << 3].forEach(stitch);
Expand Down Expand Up @@ -167,18 +172,23 @@ export default function() {
xt = x | 0,
yt = y | 0,
v0,
v1 = values[yt * dx + xt];
if (x > 0 && x < dx && xt === x) {
v0 = values[yt * dx + xt - 1];
v1 = low(values[yt * dx + xt]);
if (x > 0 && x < dx && xt === x) {
v0 = low(values[yt * dx + xt - 1]);
if (v0 == null || isNaN(v0) || !isFinite(v0)) v0 = -1e52;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This…

  • is redundant with low(…)
  • treats positive infinity as negative infinity
  • uses a magic number when we can handle infinity exactly

The passed-in value should be coerced like this (outside this function, probably in contour as I mentioned in the other comment):

value = value === null ? NaN : +value;
if (isNaN(value)) throw new Error(`invalid value: ${value}`);

Values we read from values should be coerced like this:

v0 = v0 === null ? NaN : +v0;
v1 = v1 === null ? NaN : +v1;

Given that, maybe something like this?

const a = value - v0;
const b = v1 - v0;
const d = isFinite(a) && isFinite(b) ? a / b : Math.sign(a) / Math.sign(b);
point[0] = isNaN(d) ? x : x + d - 0.5;

point[0] = x + (value - v0) / (v1 - v0) - 0.5;
}
if (y > 0 && y < dy && yt === y) {
v0 = values[(yt - 1) * dx + xt];
v0 = low(values[(yt - 1) * dx + xt]);
point[1] = y + (value - v0) / (v1 - v0) - 0.5;
}
});
}

function low(v) {
return v == null || isNaN(v) || !isFinite(v) ? -1e52 : v;
}

contours.contour = contour;

contours.size = function(_) {
Expand Down
32 changes: 32 additions & 0 deletions test/contours-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,35 @@ it("contours(values) returns the expected thresholds", () => {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0
]).map(d => d.value), [0, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.55, 0.6, 0.65, 0.7, 0.75, 0.8, 0.85, 0.9, 0.95]);
});

it("contours(values) ignores infinite values when computing the thresholds", () => {
const c = contours().size([10, 10]).thresholds(20);
assert.deepStrictEqual(c([
0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, -Infinity, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 1, 1, 1, 0, 1, 1, 1, 0, 0,
0, 1, 0, 1, 0, 1, 0, 1, 0, 0,
0, 1, 1, 1, 0, 1, 1, 1, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0, Infinity, 0,
0, 0, 0, 0, 0, 0, 0, 0, 0, 0
]).map(d => d.value), [0, 0.05, 0.1, 0.15, 0.2, 0.25, 0.3, 0.35, 0.4, 0.45, 0.5, 0.55, 0.6, 0.65, 0.7, 0.75, 0.8, 0.85, 0.9, 0.95]);
});

it("contours(values) treats null, undefined, NaN and -Infinity as holes", () => {
const c = contours().size([10, 10]);
assert.deepStrictEqual(c.contour([
1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
1, -Infinity, 1, 1, 1, 1, 1, 1, 1, 1,
1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
1, 1, 1, null, 1, 1, 1, 1, 1, 1,
1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
1, 1, 1, 1, 1, 1, 2, 2, 2, 1,
1, 1, NaN, 1, 1, 1, 2, -Infinity, 2, 1,
1, 1, 1, 1, 1, 1, 2, 2, 2, 1,
1, 1, 1, 1, 1, 1, 1, 1, 1, 1
], 0), {"type":"MultiPolygon","value":0,"coordinates":[[[[10,9.5],[10,8.5],[10,7.5],[10,6.5],[10,5.5],[10,4.5],[10,3.5],[10,2.5],[10,1.5],[10,0.5],[9.5,0],[8.5,0],[7.5,0],[6.5,0],[5.5,0],[4.5,0],[3.5,0],[2.5,0],[1.5,0],[0.5,0],[0,0.5],[0,1.5],[0,2.5],[0,3.5],[0,4.5],[0,5.5],[0,6.5],[0,7.5],[0,8.5],[0,9.5],[0.5,10],[1.5,10],[2.5,10],[3.5,10],[4.5,10],[5.5,10],[6.5,10],[7.5,10],[8.5,10],[9.5,10],[10,9.5]],[[1.5,2.5],[0.5,1.5],[1.5,0.5],[2.5,1.5],[1.5,2.5]],[[3.5,5.5],[2.5,4.5],[3.5,3.5],[4.5,4.5],[3.5,5.5]],[[2.5,8.5],[1.5,7.5],[2.5,6.5],[3.5,7.5],[2.5,8.5]],[[7.5,8.5],[6.5,7.5],[7.5,6.5],[8.5,7.5],[7.5,8.5]]]]});
});