Separate x and y in sankey

[Kuluum]

The idea is to separate x and y parameters of sankey's node to make it independent. Now if the user wants to force set only x (or y) position he or she must provide another parameter too.
I faced this problem when I made a graph with about 300 nodes and I wanted to make several columns. I wanted to force place each node in some column, so I need to set x positions, but I don't want to bother about y position, because plotly have some good logic to place nodes around, and to save plotly's y positions I should repeat this logic.

[Kuluum]

@etpinard Please look at this plotly PR

[etpinard]

Hi @Kuluum - thanks for the PR.

Can you share an example (ideally via codepen) of the old vs the new behavior?

[Kuluum]

@etpinard codepen
You can switch <script> tag in html box between current plotly's cdn and my built version.
The goal is to place all B nodes in the central column.

On current version, you need to add y list to do that (for example, y: [0,0,0,0,0.1,0,0] - for me it looks strange too, but it's not the point).

[etpinard]

I'll cc @antoinerg who worked on this problem before. What do you think @antoinerg ?

[etpinard]

By the way @Kuluum - have you ran the image tests off your branch? If so, did it lead to any failures?

[Kuluum]

@etpinard No, I miss this moment, just didn't see this link before. I will do it asap. Maybe it's a good idea to add information about running tests in Pull request guide?

[Kuluum]

@etpinard sankey tests are ok

node tasks/test_image.js "sankey*"

Running image comparison tests using build/plotly.js from 2019-11-20 13:59:14

nw1: stopped
nw1: started


TAP version 13
testing mocks in batch
ok 1 sankey_circular_simple should be pixel perfect
ok 2 sankey_circular_simple2 should be pixel perfect
ok 3 sankey_circular_process should be pixel perfect
ok 4 sankey_circular should be pixel perfect
ok 5 sankey_circular_large should be pixel perfect
ok 6 sankey_large_padding should be pixel perfect
ok 7 sankey_energy_dark should be pixel perfect
ok 8 sankey_energy should be pixel perfect
ok 9 sankey_groups should be pixel perfect
ok 10 sankey_link_concentration should be pixel perfect
ok 11 sankey_subplots_circular should be pixel perfect
ok 12 sankey_messy should be pixel perfect
ok 13 sankey_subplots should be pixel perfect
ok 14 sankey_x_y should be pixel perfect

1..14
tests 14
pass  14

ok

I also tried to run all test, but it crashes on different test cases once at 493 another one at 431

[Kuluum]

@antoinerg ping

[antoinerg]

@etpinard @Kuluum I just laid my eyes on this one for the first time. Sorry for the delay.

This is an interesting change. It seems like it's working alright for the simple case you provided in the Codepen but I would need to see how it behaves when there are circular links (or way more nodes). I will take a better look at it tomorrow.

[Kuluum]

@antoinerg kind reminder =)

[etpinard]

@Kuluum we appreciate your enthusiasm, but unfortunately plotly.js devs are a little busy at the moment with end-of-year requirements to finish. Reviewing community PRs is often not a high-priority for us unfortunately. We'll attempt to take a in-depth look at your work before our next minor release coming at the start of the new year. Thank you.

[Kuluum]

@etpinard Maybe I should make this PR to main repository so you can see it there instead of visiting my repo?

[Kuluum]

@antoinerg @etpinard Is anybody here? :)

[antoinerg]

Sorry @Kuluum for being inactive here. I was sucked into working on other codebases.

As I said, I would need to see examples of how it behaves. I suspect that setting only x or y will lead to nodes being poorly laid out. I would also like to see an example with circular links.

Could you add a new mock/baseline showcasing the feature with and without circular links?

Thank you @Kuluum !

[Kuluum]

@antoinerg
https://codepen.io/kuluum/pen/vYEPyrp <- with cycle
https://codepen.io/kuluum/pen/yLLGZvQ <- without cycle

You can switch <script> tag in html box between current plotly's cdn and my built version.

In the current plotly version if you comment just one of x or y line the plot will layout the same way as without both x and y.
If you use my version, you can set only x list then x positions will change, same with only y.
It doesnt look that changing only one coordinate, instead of two can break the cycle.

P.S. If you set x coordinate of C node to 0.8 x: [0.2, 0.1, 0.8, 0.7, 0.3, 0.5] or just greater than D's node x, then cycle will break. But it is so in both current plotly and my version.
just noticed that.

[Kuluum]

@antoinerg I still here)