More robust Sankey #2629
More robust Sankey #2629
Conversation
| @@ -437,6 +437,8 @@ function recurseIntoValObject(valObject, parts, i) { | |||
| return valObject; | |||
| } | |||
|
|
|||
| // note: this is different from Lib.isIndex, this one doesn't accept numeric | |||
| // strings, only actual numbers. | |||
There was a problem hiding this comment.
I initially made isIndex into a Lib function thinking I could reuse it here... but in this context it's in principle important that an index is actually a number, to distinguish an array index from an object property that happens to be a number. That said I don't think it's possible to use a number as an object property in nestedProperty...
There was a problem hiding this comment.
Oh we could use this for validating selectedpoints items as done here:
Lines 497 to 499 in a132b85
There was a problem hiding this comment.
... by this I mean Lib.isIndex
| links[i].source = nodeIndices[links[i].source]; | ||
| links[i].target = nodeIndices[links[i].target]; | ||
| } | ||
| } |
There was a problem hiding this comment.
For the most part it didn't cause problems to have extra unused nodes in the list, but sometimes the layout algorithm would reserve extra space for these nodes (leaving extra margins or making the whole diagram S-shaped, for example) so it's better to remove them. The sankey_messy mock includes a whole bunch of extra nodes both before (to test this link reassignment) and after the real ones to test this.
src/traces/sankey/calc.js
Outdated
| @@ -14,13 +14,16 @@ var wrap = require('../../lib/gup').wrap; | |||
|
|
|||
| function circularityPresent(nodeList, sources, targets) { | |||
|
|
|||
| var nodes = nodeList.map(function() {return [];}); | |||
| var nodes = nodeList.map(function() { return []; }); | |||
There was a problem hiding this comment.
Would be nice to reuse:
Lines 13 to 17 in a132b85
| describe('No warnings for missing nodes', function() { | ||
| // we used to warn when some nodes were not used in the links | ||
| // not doing that anymore, it's not really consistent with | ||
| // the rest of our data processing. |
|
Great PR 💃 |
|
... my comments are non-blocking. |
Make Sankey more accepting of messy data (fixing #2480)
supplyDefaults, so was called frequently, and had a fairly slow implementation so could be a drag on large Sankey traces.cc @etpinard