Updating to d3-sankey 0.7.1 #3355
Conversation
| "alpha-shape": "^1.0.0", | ||
| "array-range": "^1.0.1", | ||
| "canvas-fit": "^1.5.0", | ||
| "color-normalize": "^1.3.0", | ||
| "convex-hull": "^1.0.3", | ||
| "country-regex": "^1.1.0", | ||
| "d3": "^3.5.12", | ||
| "d3-sankey": "git://github.com/antoinerg/d3-sankey.git#4f37ed8d3578b545a8569ecd74583f373768e900", |
There was a problem hiding this comment.
The only appreciable change we have with d3-sankey is:
d3/d3-sankey@master...antoinerg:fix-large-padding#diff-968964796da46b424b314b4359ccde31
There was a problem hiding this comment.
The link above give me zero diff!
Thought the link one the line below may be the correct one?
d3/d3-sankey@master...antoinerg:fix-large-padding
There was a problem hiding this comment.
Yes, thank you @archmoj! The only changes I made are are in src/sankey.js.
| @@ -218,22 +107,50 @@ function linkModel(uniqueKeys, d, l) { | |||
| }; | |||
| } | |||
|
|
|||
| function nodeModel(uniqueKeys, d, n) { | |||
| function linkPath() { | |||
There was a problem hiding this comment.
This path generator for links used to lived in our fork of d3-sankey and was added in this commit:
plotly/d3-sankey@80c33c3 . It doesn't need to live in there and could be moved back in here.
This and the fix for large node padding are the only improvements that we had over d3-sankey v0.4.2.
There was a problem hiding this comment.
To substantiate this last comment, you can review the changes of our forked d3-sankey has compared to v0.4.2.
| var removedNodes = false; | ||
| var nodeIndices = {}; | ||
|
|
||
| for(i = 0; i < nodeCount; i++) { |
There was a problem hiding this comment.
Non-blocking comment: when deleting from a list one may consider reversing the loop count and simply drop (pop) items from the array.
|
@antoinerg Great work! I was wondering if the status of this PR should be changed to review-able? |
|
Thanks @archmoj! This is by no means urgent and I feel like it might be a bit too early for a review. There is still the possibility that we get rid of d3-sankey altogether if it doesn't fit our requirements. I'll give you an update about this soon. |
|
@antoinerg Looks like all the commits here are now part of #3406 Ok if we close this one? |
Yes, it's all part of #3406 and I should have closed this one. Sorry about that! |
This will be the starting point to migrate to https://github.com/tomshanley/d3-sankey-circular which closely ressembles d3-sankey v0.7.1 API.
The biggest difference with the previous version of d3-sankey is a change of coordinate (x, dx) -> (x0, x1).
This is still a WIP although all tests are passing and only one baseline had to be slightly updated.