Skip to content

Sankey issue #3140

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nicolaskruchten opened this issue Oct 23, 2018 · 4 comments
Closed

Sankey issue #3140

nicolaskruchten opened this issue Oct 23, 2018 · 4 comments
Assignees
Labels
bug something broken

Comments

@nicolaskruchten
Copy link
Contributor

Here is the codepen for the reported bug: https://codepen.io/rbakker/pen/gBjXde
The pen shows two plotly plots with Sankey diagram with the same data.
The only difference is that the first plot uses node.pad = 10, the second uses node.pad = 2.
In the first plot the nodes are not drawn at all.

@nicolaskruchten
Copy link
Contributor Author

(reported via support channels)

@alexcjohnson
Copy link
Collaborator

Interesting... what's happening is we need to generate a node/link thickness scaling, and we do so by taking the height of a column, subtracting off n-1 paddings (for n nodes in that column), and scaling all nodes so the sum of their thicknesses fills the remaining space.

The problem is that the requested padding is more than the available space, so the scaling factor goes negative, and apparently we never considered this possibility before. The way we appear to draw it, negatively scaled nodes get zero height, and negatively scaled links get drawn with the absolute value of the scaling (but misaligned because we weren't expecting this)

I suppose the solution would be something like limit the padding to at most occupy perhaps half the space in any given column? 2/3?

@alexcjohnson alexcjohnson added the bug something broken label Oct 23, 2018
@antoinerg
Copy link
Contributor

@alexcjohnson Should we give a warning (Lib.warn) if the user specified value is too big?

@alexcjohnson
Copy link
Collaborator

Mmm yeah, good idea - Lib.warn if the actual padding is less than requested. 👍 That's probably better (and certainly easier) than trying to push the calculated value back into fullData (in a way that's compatible with Plotly.react...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

No branches or pull requests

3 participants