Fix date histograms#1186
Conversation
and test calc of uniform AND nonuniform bins
| size: xbins.size | ||
| }; | ||
| } | ||
| if(!nonuniformBinsY && ya.type === 'date') { |
There was a problem hiding this comment.
Why do we need this here? Lib.findBin is called below.
There was a problem hiding this comment.
Described above:
At its root, the issue is that
Lib.findBindoesn't know anything about the axis it's binning onto, so needs bins specified entirely by numbers, either{start, end, size}or[edge0, edge1, edge2, ... edgeN], but in the new date system,startandendare date strings. For performance it's important that these be turned into numbers only once, so rather than hack date conversion intofindBinwe convert the bins object everywhere it gets called.
We could refactor findBin to be a higher-order function:
var findBin = Lib.binFinder(bins, ax);
for(i...) findBin(x[i]);
That would actually have some performance benefits, but it's a little bit of a bigger project because of all the places this gets called.
There was a problem hiding this comment.
Got it. Sorry I thought that first patched file ⏫ was lib/search.js. Time to grab another ☕
|
Looks good 💃 |
fixes https://github.com/plotly/streambed/issues/8512 - date histograms with small bins failed. Turns out it was any uniform date bins, ie intervals smaller than 1 month, that were broken.
Note that there are still some issues including and relating to #1151 uncovered in the course of this fix, some TODOs added describing these.
At its root, the issue is that
Lib.findBindoesn't know anything about the axis it's binning onto, so needs bins specified entirely by numbers, either{start, end, size}or[edge0, edge1, edge2, ... edgeN], but in the new date system,startandendare date strings. For performance it's important that these be turned into numbers only once, so rather than hack date conversion intofindBinwe convert the bins object everywhere it gets called.In addition to histograms, this routine gets called in:
traces/box/calc.js
traces/heatmap/convert_column_xyz.js
traces/heatmap/hover.js
traces/heatmap/plot.js
But as far as I can tell these callers can only provide numeric data for the bins so they should be safe.