Ensure axis autorange is computed after transform operations #1260
Conversation
- one loop for relinking calcdata - one loop for transform ops - one loop for regular _module.calc in an effort to make transform less intrusive.
- so that axis autorange is computed after transfroms are made.
| cd = []; | ||
|
|
||
| // If traces were specified and this trace was not included, then transfer it over from | ||
| // the old calcdata: | ||
| if(Array.isArray(traces) && traces.indexOf(i) === -1) { | ||
| calcdata[i] = oldCalcdata[i]; |
There was a problem hiding this comment.
I know this part isn't new here, but... it isn't used anywhere, right? Looks to me like it was already broken by transforms (calcdata index and trace index aren't the same) and will be broken MORE by this PR (any trace not included will be dropped from autorange) so I'd remove it until we decide to do it right AND use it.
There was a problem hiding this comment.
Ah, is that a bug to be using that as an array index like that? If so, that may well be my fault. Unless I'm wrong, the intention was to reduce the work when animating. Use case: two traces. One contains 10MB of data, the other has two points. When animating the two points, it's best to avoid touching the 10MB trace. Perhaps we need to fix this.
There was a problem hiding this comment.
right, makes a lot of sense to want to do that. I guess for autorange we just need to store the various _min and _max entries in a different way so we can selectively replace only the ones for the traces we're recalculating. And trace index, depending on how this gets used (it's not yet, right?) you just have to be careful about what you mean by index, before or after transforms potentially insert new traces.
|
LGTM. Great tests! nonblocking comment, 💃 |
|
🎉 |
|
will be part of |
| // clear stuff ... | ||
| for(i = 0; i < axList.length; i++) { | ||
| axList[i]._min = []; | ||
| axList[i]._max = []; |
There was a problem hiding this comment.
note to self: maybe we should clear _categories too to fix #1181 ?
There was a problem hiding this comment.
Cool - note though that this only fixes the first part of 1181, not the variant of it mentioned in #1181 (comment)
- so that only post-transform categories are displayed on graph
|
nice. 💃 |
To fix the bug, we need to clear the
ax._minand_ax._maxarrays (generated inAxes.expanddone in the_module.calccall before thecalcTransformloop) before the finalmodule.calccall.To do so in the least intrusive way possible, this PR splits
Plots.doCalcdataoperations into separate loops - which hopefully will also help readability.