leeoniya / uPlot

📈 A small, fast chart for time series, lines, areas, ohlc & bars
MIT License
8.48k stars 370 forks source link

1.6.28 broke plot zooming in mode 2 #914

Closed davidlougheed closed 3 months ago

davidlougheed commented 3 months ago

Hello,

Thanks for your work on uPlot. As of 1.6.28, I get an error when zooming just the x-axis of a plot in mode 2. The error occurs in the following line, because wsc is undefined: https://github.com/leeoniya/uPlot/blob/master/src/uPlot.js#L1263

Here is a fiddle demonstrating the error:

https://jsfiddle.net/3L7qx1tr/

leeoniya commented 3 months ago

oof, i knew this would bite me :sweat_smile:. those changes cut pretty deep.

leeoniya commented 3 months ago

i probably should mention that for a dataset like this you don't need mode 2 :)

davidlougheed commented 3 months ago

this is a reduced example from a scatter plot component where I was using one of the demos for reference where mode is set to 2, i'm not sure I understand exactly what it does (and in our production codebase I have an old comment beside it saying // (?) haha)

leeoniya commented 3 months ago

in the beginning, all of uplot's built-in pathbuilders expected an aligned / timeseries-like data structure, which requires ascending and non-repeating x values. that makes it fundamentally incompatible with rendering scatter plots that can have arbitrary points and ordering, and often no special meaning for the x axis, since you're just correlating two numeric dimensions. mode 2 was introduced to disable some internal timeseries data shape assumptions, auto scaling, and cursor internals. you can plug your own renderer for each series and DIY.

since then, i have updated the built-in pathbuilders to support the mode: 2 data shape. i'm not really sure that x-only zoom makes sense in mode 2 / scatter plots.

davidlougheed commented 3 months ago

I'm using uplot in a bioinformatics context to build an interactive Manhattan plot with the x-axis being a linear chromosome position, and the y-axis being -log10(p) values, so for my (fairly niche) purpose it makes sense, since even when zoomed in we want to keep the global 'more significant or less significant p' y-range context:

image

it's true that this could allow y-zooming as well and it probably wouldn't harm things too much!

(edit: also I guess this has an ordered x-axis, so it isn't exactly arbitrarily ordered, but it isn't a quite a time series either)