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.

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

ignore infinite values when setting a scale domain? #528

Closed
Fil opened this issue Sep 2, 2021 · 3 comments · Fixed by #644
Closed

ignore infinite values when setting a scale domain? #528

Fil opened this issue Sep 2, 2021 · 3 comments · Fixed by #644
Labels
enhancement New feature or request question Further information is needed

Comments

@Fil
Copy link
Contributor

Fil commented Sep 2, 2021

I was doing an error plot and because one of the values was infinite (log(0)), the y scale collapsed. it would be more useful in that case to ignore the infinite value (as NaN), and report the missing / invalid point when we have some way of logging warnings.

@Fil Fil added enhancement New feature or request good first issue question Further information is needed labels Sep 2, 2021
@Fil Fil changed the title ignore infinite values? ignore infinite values when setting a scale domain? Sep 2, 2021
@mbostock
Copy link
Member

mbostock commented Sep 7, 2021

If we want to do this, it should probably happen when we coerce numbers (#532):

plot/src/scales.js

Lines 189 to 194 in 916d04b

// Unlike Mark’s number, here we want to convert null and undefined to NaN,
// since the result will be stored in a Float64Array and we don’t want null to
// be coerced to zero.
function coerceNumber(x) {
return x == null ? NaN : +x;
}

For example, we could check that the value is finite:

function coerceNumber(x) {
  return x == null || !isFinite(x = +x) ? NaN : x;
}

However, I’m not sure that this is a good universal behavior… it feels convenient to treat infinite values as undefined to make sure that the chart renders correctly, but the infinite values are (technically) defined so it might be preferable to throw an error or show a warning instead. #536

@Fil
Copy link
Contributor Author

Fil commented Sep 7, 2021

The line is blurry: a mathematician would say that 1/0 and log(0) are undefined, but JavaScript wants them Infinite.

In my use case, the data was the magnitude of a computational error over 100 tests. When the error for one of the 100+ tests fell to 0 (a situation which I hadn't anticipated), the y axis crashed, which was not helpful.

This patch works very well; in this notebook, I tried several alternatives… in each case it means you have to understand why the chart collapsed, then think about a solution.

(I'd also want a warning, of course.)

@Fil
Copy link
Contributor Author

Fil commented Nov 23, 2021

This proposal is also consistent with #279.

In other words, this chart:

Plot.plot({ x: { type: "linear" }, marks: [
  Plot.dotX([0, 0.1, 1, 2, 10], Plot.map({x: d => d.map(d => Math.log2(d))}, {x: d => d}))
]});

should not fail, but behave as this one (which is our log-degenerate.js test case), only with log(d) on the x axis:

Plot.plot({ x: { type: "log" }, marks: [ Plot.dotX([0, 0.1, 1, 2, 10]) ] });

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants