update numAxisSplits to throw error instead of infinite loop#1117
update numAxisSplits to throw error instead of infinite loop#1117fastfrwrd wants to merge 1 commit intoleeoniya:masterfrom
Conversation
| if (scaleMin === scaleMax) | ||
| throw new RangeError("scaleMin cannot be equal to scaleMax"); | ||
|
|
||
| if (roundDec(scaleMin + foundIncr, numDec) === scaleMin) |
There was a problem hiding this comment.
the only sus thing from the perspective of performance I think. in reality, it's one extra round beyond the rounding the method already does as part of the below for loop, so it seems negligible for the benefit.
|
hey paul, nice to see you :)
since uPlot has to handle flat data (e.g. single data points), there is already Lines 206 to 271 in 3a56bc6 this is why splits finding functions [and everything downstream from the example data in the added test seems to work okay with the built-in ranger: https://jsfiddle.net/z0xaq398/ let opts = {
width: 400,
height: 170,
scales: {
x: {
time: false,
},
},
series: [
{},
{
stroke: "red",
},
],
};
let data = [
[0, 1, 2, 3, 4],
[2, 1.999999999999999, 2.000000000000001, 2, 2],
];
let u = new uPlot(opts, data, document.body);while i agree that crashing and infinite loop is not a great outcome, it should be the responsibility of the customized options to uphold the same contract as the built-in functions (the ranging util functions are also statically exported for this purpose). i wonder if uPlot should additionally expose some finer-grained helpers to avoid too much copy-paste into any customized scale.range callbacks. and/or provide some kind of conformance tests that allow someone to validate that their own range functions handle all the same situations that uPlot's default ones already handle. |
Yeah, that's exactly right. The Sparkline is doing its own range configuration, mainly because it's user configurable in some situations like Table, and we keep running into cases where this happens. I wonder if there's a way to do the kind of check we do for auto-range to a configured range, and then maybe adjust what was provided if it's going to be a problem.
I like that idea! |
|
one util that would be nice to have would be some way to get the implied |
|
@fastfrwrd could you minimally extend the above jsfiddle to show how an exported decimals lookup would be used in a custom |
We have had some instances recently where the min and max being equal crashes the browser in Grafana:
While we should not attempt to remediate this inside uPlot, it would be better to just throw if we can tell that's about to happen rather than running out of memory.