Skip to content
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

Add support for sankey links with arrows #6276

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Andy2003
Copy link

@Andy2003 Andy2003 commented Jul 25, 2022

This pull request adds a configuration to add arrows to the links in the sankey diagram.

Given the following configuration:

{
    "data": [
        {
            "type": "sankey",
            "node": {
                "pad": 5,
                "label": ["0", "1", "2", "3", "4", "5", "6"]
            },
            "link": {
                "arrowwidth": 20,
                "source": [
                    0, 0, 1, 2, 5, 4, 3
                ],
                "target": [
                    5, 3, 4, 3, 0, 2, 2
                ],
                "value": [
                    1, 2, 1, 1, 1, 1, 1
                ]
            }
        }],
    "layout": {
        "title": {"text": "Sankey with circular data and arrows"},
        "width": 800,
        "height": 800
    }
}

The following sankey diagram is generated:

sankey_circular_with_arrows

@Andy2003 Andy2003 force-pushed the feature/sanky-with-arrows branch from 4f3503a to b2e0190 Compare Jul 25, 2022
@alexcjohnson
Copy link
Contributor

@alexcjohnson alexcjohnson commented Jul 25, 2022

Thanks @Andy2003 - that's a nice effect 😎

I'd call it a length though, since it's along the direction of the link, rather than a width which is typically perpendicular to the direction of the object. So arrowlen (we prefer not to use length in attribute names because of the built-in .length)

Two cases I'm curious about:

  • Does this work for vertical Sankey diagrams (orientation: 'v')?
  • What happens to non-circular links if the space between two nodes is close to or even less than the arrow length? Maybe we need to limit each arrow to take no more than perhaps half the available space?

@Andy2003
Copy link
Author

@Andy2003 Andy2003 commented Jul 25, 2022

Does this work for vertical Sankey diagrams (orientation: 'v')?

Yes:

sankey_circular_with_arrows_vertical

What happens to non-circular links if the space between two nodes is close to or even less than the arrow length? Maybe we need to limit each arrow to take no more than perhaps half the available space?

Done:

sankey_x_y_with_arrows

* rename `arrowwidth` for sankey diagram to `arrowlen`
* add test vor vertical orientation
* check available space for arrows and apply max 50% of it to arrows
@Andy2003 Andy2003 force-pushed the feature/sanky-with-arrows branch from 56d2877 to 2e4b5a2 Compare Jul 25, 2022
@alexcjohnson
Copy link
Contributor

@alexcjohnson alexcjohnson commented Jul 25, 2022

Looks great! We'll need to figure out why the tests are failing, and let's see if @archmoj has any further comments.

@archmoj
Copy link
Contributor

@archmoj archmoj commented Jul 25, 2022

Please run the command below to commit the changes to plot-schema test.

npm run schema && git add test/plot-schema.json && git commit -m "update plot-schema diff"

There is a bug in our current image tests system.
To avoid that please rename those 3 new mocks and baselines so that they start with z-sankey_ and run in the end of the list.

Thanks.

@archmoj
Copy link
Contributor

@archmoj archmoj commented Jul 26, 2022

💃 💃
💃 💃 💃
💃 💃
LGTM.
We will merge this when we started adding features of 2.14.0.

draftlogs/6276_add.md Outdated Show resolved Hide resolved
Co-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants