Bug fix parcoords when all the data is zero #4082
Conversation
src/plots/cartesian/axes.js
Outdated
| @@ -839,7 +839,7 @@ function autoTickRound(ax) { | |||
| // 2 digits past largest digit of dtick | |||
| ax._tickround = 2 - Math.floor(Math.log(dtick) / Math.LN10 + 0.01); | |||
|
|
|||
| var maxend = Math.max(Math.abs(rng[0]), Math.abs(rng[1])); | |||
| var maxend = Math.max(Math.abs(rng[0]), Math.abs(rng[1])) || 1; | |||
There was a problem hiding this comment.
Hmm. I'm not a fan of this. We shouldn't have to patch this deep in axes.js. If cartesian axes don't need this || 1 fallback, parcoords shouldn't need it.
Could you help understand why we need to log a range value of 0?
There was a problem hiding this comment.
I could simply remove this from here. But I thought we should check for that to avoid log(0).
There was a problem hiding this comment.
Ha I see, fde0d32 is enough to fix the problem. Sorry for the confusion.
There was a problem hiding this comment.
Yes. That should be enough. Do you want me to undo 765ca5a?
There was a problem hiding this comment.
If maxend is 0 here, it leads to a pretty serious malfunction for this routine. Just mocking it to 1 isn't really the behaviour we want. In fact, the fallback is just there to not make Math.log blowup. Cartesian (and polar and maybe other axes) ranges are sanatised during ax.cleanRange, so maxend=0 never happens and the fallback is never used, so I'd be ok with dropping the fallback.
That said, since we're starting to reuse the more of the Axes logic, I think it would be best to log and error using Lib.error - similar to
plotly.js/src/plots/cartesian/axes.js
Line 935 in 38cd9e2
So @archmoj you could just drop it, or drop it and add a Lib.error when maxend === 0
|
Thanks very much !! 💃 💃 |
Fixes #4080 for
parcoords, alsoadds a condition check in
Axesto avoid encounteringlog(0)in computing range-exponent.@plotly/plotly_js