d3-sankey-circular #3406

antoinerg · 2019-01-07T02:43:35Z

Initial integration of d3-sankey-circular to close #2636

This branch is based off PR #3355 which brings d3-sankey to version 0.7.1.

When detecting circular links, we swap from d3-sankey(https://github.com/d3/d3-sankey) to d3-sankey-circular (https://github.com/tomshanley/d3-sankey-circular). This is possible because d3-sankey-circular has a similar API to d3-sankey 0.7.x and this is why PR #3355 was an important stepping stone.

Here are example baselines:

To do:

Add dragging and a force model
Links are not rectangles anymore but path (so we don't have a fill property, but only stroke)
Style link on hover

antoinerg · 2019-01-07T22:56:30Z

package.json

    "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",


Minimal changes over latest d3-sankey d3/d3-sankey@master...antoinerg:fix-large-padding

Hmm. So we no longer need https://github.com/plotly/d3-sankey ? I thought i read that we made changes in our @plotly/d3-sankey that got rejected by d3/d3-sankey.

Those changes were minimal. One of the biggest was that we generated the SVG path for the links in it. As you found out in this PR, we can do this operation in plotly.js instead.

Ok that make sense. Do you think we have a shot at getting

d3/d3-sankey@master...antoinerg:fix-large-padding

merged into d3/d3-sankey?

There hasn't been anything merged into master in d3/d3-sankey since Jul 13, 2017 so I'm not sure... I opened a PR almost a month ago d3/d3-sankey#63 and there has been no activity since then.

antoinerg · 2019-01-07T22:57:40Z

package.json

    "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",
+    "d3-sankey-circular": "git://github.com/antoinerg/d3-sankey-circular.git#a298acf674f0a9c7158243d45814cd8060dad728",


Minimal changes over latest d3-sankey-circular tomshanley/d3-sankey-circular@master...antoinerg:support-update

Those changes got merged in 🚀 !

antoinerg · 2019-01-08T23:05:35Z

Here's a Codepen to generate random mocks with circular links: https://codepen.io/anon/pen/LMBZbK

You can play around with N, L and targetOffset in the source code.

AryehGielchinsky · 2019-01-13T21:36:39Z

Hi @antoinerg, thank you for your work bringing circular Sankey to plotly. I'm beginner/intermediate at python, beginner at plotly, and don't know any JavaScript. Is there any way I could help?

antoinerg · 2019-01-14T17:01:09Z

Hello @AryehGielchinsky and thank you for your interest in plotly.js. Sankey is undergoing majors changes as we're implementing several new features. Getting feedback from the community is always useful.

What's your use case for circular Sankey? Do you have interesting datasets with circularity that we could use as examples?

antoinerg · 2019-01-18T23:02:31Z

hovers appear in the center of circular loops.

Good ! For forward links the bounding box center is always on the link, but for circular links it would be great to move the hover box anchor to the center of the back stretch.

@alexcjohnson Is the following what you had in mind? I am not sure what part is the "back stretch".

alexcjohnson · 2019-01-18T23:05:30Z

@alexcjohnson Is the following what you had in mind? I am not sure what part is the "back stretch".

Yep, you got it exactly as I had in mind! https://en.wikipedia.org/wiki/Backstretch

src/traces/sankey/render.js

AryehGielchinsky · 2019-01-21T20:22:47Z

Hi @antoinerg unfortunately I wont be able to share Sankey diagrams based on my company's datasets.

I look forward to the completion of this project, I think it will make plotly's sankey diagrams much more useful. Keep up the good work!

etpinard · 2019-01-21T21:44:38Z

@antoinerg this thing is starting to look great. Here's an updated TODO list:

ask the d3-sankey-circular author to publish a new version to npm, update our deps accordingly
merge the changes from your d3-sankey fork to @plotly/d3-sankey, update our deps accordingly
add a few jasmine tests to 🔒 down sankey.update()

antoinerg · 2019-01-22T20:23:50Z

In commit 9b36090, I 🔒 down sankey.update() and fix a little bug that I discovered in its implementation in d3-sankey-circular: it wouldn't force circular links to go from top to bottom on the first invocation of update(). Because we make the call to update() in the animation loop over and over, this error on the first call was not perceivable to the user. Anyway, this is now fixed.

I will submit PR upstream (edit: done here tomshanley/d3-sankey-circular#29). BTW, its maintainer @tomshanley said he could publish an npm package as soon as he gets back from holiday (tomshanley/d3-sankey-circular#28 (comment)).

antoinerg · 2019-01-24T00:41:06Z

@etpinard Tomorrow I intend to create/update our own forks for both d3-sankey and d3-sankey-circular and publish npm packages for them. Was updating our deps the only remaining thing blocking this PR?

etpinard · 2019-01-24T00:47:12Z

Was updating our deps the only remaining thing blocking this PR?

Yes. Your new tests look good!

etpinard · 2019-01-24T00:56:40Z

test/image/mocks/sankey_subplots_circular.json

@@ -0,0 +1,65 @@
+{
+    "data": [{


etpinard · 2019-01-24T00:57:20Z

test/jasmine/tests/sankey_test.js

-        expect(sankey.nodePadding()).toEqual(10, 'incorrect nodePadding');
-    });
+        // Update links
+        var updatedGraph = sankey.update(graph);


Actually, can we make these tests use restyle instead of (the internal) sankey.update?

Actually, can we make these tests use restyle instead of (the internal) sankey.update?

I don't think we can do that right now because we don't have any attributes to control the position of the nodes (so no restyle call can change the position of the nodes). Right now, sankey.update is only called when nodes are dragged around with the mouse.

Maybe tests using restyle could be included in a future PR implementing attributes node.x and node.y? Or maybe I should add those attributes here right now?

Ha I see. Good point.

Can you add (just) one restyle test that update circular to non-circular node and links data? Or better yet, one react test that goes from one circular mock to a non-circular one?

One last TODO ⬆️

handle large circular sankey diagram

etpinard · 2019-02-14T21:06:15Z

Amazing work!

💃 💃 💃

antoinerg added 9 commits December 12, 2018 17:49

merge @Plotly/d3-sankey into plotly.js (es6 -> es5, tests in jasmine)

03a3cb5

fix header for d3-sankey.js

2504dc3

move data conversion into its own function and run in calc() phase

fd19907

refactor d3-sankey to look more similar to upstream

bde2a79

fix indentation

5f89d78

updating to d3-sankey 0.7.1

1ad9586

generate link path appropriately, reattach mouse events

ee7088a

fix dragging, snapping, pushing and pass all tests

5418d25

initial rough integration of d3-sankey-circular

be1acc8

antoinerg added feature something new status: in progress labels Jan 7, 2019

antoinerg added 3 commits January 7, 2019 15:27

implement dragging

067869f

fallback to d3-sankey if no circularity is present

f8bf7fe

Merge remote-tracking branch 'origin' into sankey-circular, fix lint

54523db

antoinerg changed the base branch from sankey-d3-sankey-0.7 to master January 7, 2019 22:47

antoinerg commented Jan 7, 2019

View reviewed changes

antoinerg added 3 commits January 8, 2019 14:28

testing features of d3-sankey-circular

ed87295

change stroke-opacity on hover

c1404fb

use index as unique identifier for nodes

d268b73

antoinerg mentioned this pull request Jan 10, 2019

d3-sankey-circular with grouping #3426

Closed

antoinerg added 4 commits January 14, 2019 12:23

delete tests dealing with removal of circular nodes

09d2aa6

on update, change circular link type based on nodes' position

3e5b160

circular-sankey, sort links per node on update

3ed9ba5

change node's identifier

00e5d28

antoinerg removed the status: discussion needed label Jan 14, 2019

combine all sankey tests into a single file

4539d28

position hover label above circular links

225d310

antoinerg commented Jan 21, 2019

View reviewed changes

src/traces/sankey/render.js Show resolved Hide resolved

update d3-sankey-circular and test update() API

9b36090

sankey-circular: add baseline with subplots and vertical orientation

48a8b1b

etpinard reviewed Jan 24, 2019

View reviewed changes

test/image/mocks/sankey_subplots_circular.json

@@ -0,0 +1,65 @@

{

"data": [{

Copy link

Contributor

etpinard Jan 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool mock!

etpinard reviewed Jan 24, 2019

View reviewed changes

etpinard added this to the v1.45.0 milestone Jan 24, 2019

antoinerg mentioned this pull request Jan 31, 2019

Cleanup prior to releasing 0.7.2 plotly/d3-sankey#8

Merged

antoinerg added 6 commits January 31, 2019 14:51

update Sankey deps

777dc97

nest API tests for d3-sankey-circular in its own describe block

ebe613b

d3-sankey-circular test large dataset

04d53ff

remove gap between circular links

003704c

update d3-sankey-circular and associated baselines

3625234

update d3-sankey-circular deps

59dbef4

antoinerg mentioned this pull request Feb 11, 2019

handle large circular sankey diagram #3535

Merged

antoinerg added 3 commits February 14, 2019 11:56

update dep of d3-sankey-circular

4ed7634

Merge pull request #3535 from plotly/pr-large-sankey-circular

ba2a485

handle large circular sankey diagram

test switching to circular Sankey on Plotly.react

063784a

antoinerg merged commit 30eaa3a into master Feb 14, 2019

azu mentioned this pull request Feb 14, 2019

update plot.ly for d3-sankey-circular textlint/textstat#3

Open

antoinerg deleted the sankey-circular branch March 26, 2019 01:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

d3-sankey-circular #3406

d3-sankey-circular #3406

antoinerg commented Jan 7, 2019 •

edited

Loading

antoinerg Jan 7, 2019

etpinard Jan 15, 2019

antoinerg Jan 15, 2019 •

edited

Loading

etpinard Jan 15, 2019

antoinerg Jan 17, 2019

antoinerg Jan 7, 2019

antoinerg Jan 18, 2019

antoinerg commented Jan 8, 2019

AryehGielchinsky commented Jan 13, 2019 •

edited

Loading

antoinerg commented Jan 14, 2019 •

edited

Loading

antoinerg commented Jan 18, 2019

alexcjohnson commented Jan 18, 2019

AryehGielchinsky commented Jan 21, 2019

etpinard commented Jan 21, 2019 •

edited by antoinerg

Loading

antoinerg commented Jan 22, 2019 •

edited

Loading

antoinerg commented Jan 24, 2019

etpinard commented Jan 24, 2019

etpinard Jan 24, 2019

etpinard Jan 24, 2019 •

edited

Loading

antoinerg Jan 24, 2019

etpinard Jan 24, 2019

etpinard Feb 14, 2019

etpinard commented Feb 14, 2019

d3-sankey-circular #3406

d3-sankey-circular #3406

Conversation

antoinerg commented Jan 7, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoinerg Jan 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoinerg commented Jan 8, 2019

AryehGielchinsky commented Jan 13, 2019 • edited Loading

antoinerg commented Jan 14, 2019 • edited Loading

antoinerg commented Jan 18, 2019

alexcjohnson commented Jan 18, 2019

AryehGielchinsky commented Jan 21, 2019

etpinard commented Jan 21, 2019 • edited by antoinerg Loading

antoinerg commented Jan 22, 2019 • edited Loading

antoinerg commented Jan 24, 2019

etpinard commented Jan 24, 2019

Choose a reason for hiding this comment

etpinard Jan 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard commented Feb 14, 2019

antoinerg commented Jan 7, 2019 •

edited

Loading

antoinerg Jan 15, 2019 •

edited

Loading

AryehGielchinsky commented Jan 13, 2019 •

edited

Loading

antoinerg commented Jan 14, 2019 •

edited

Loading

etpinard commented Jan 21, 2019 •

edited by antoinerg

Loading

antoinerg commented Jan 22, 2019 •

edited

Loading

etpinard Jan 24, 2019 •

edited

Loading