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

sankey: introducing attributes (node|link).customdata #4621

Merged
merged 5 commits into from
Mar 10, 2020

Conversation

antoinerg
Copy link
Contributor

Closes #4243 by introducing attributes node.customdata and link.customdata as suggested in #4243 (comment).

@etpinard
Copy link
Contributor

Nice!

I see that customdata is currently part of the sankey root-level attributes:

image

Maybe we should remove it, like we currently do for parcats:

// Hide unsupported top-level properties from plot-schema
customdata: undefined,

@emmanuelle
Copy link
Contributor

Thank you @antoinerg for the great reactivity :-). Would it make sense to add a test where the node or link customdata has more than one dimensions, to check that it's possible to display several values from the customdata in the hovertemplate?

@antoinerg
Copy link
Contributor Author

Maybe we should remove it, like we currently do for parcats

Done in 63ef3e9

@antoinerg
Copy link
Contributor Author

Thank you @antoinerg for the great reactivity :-). Would it make sense to add a test where the node or link customdata has more than one dimensions, to check that it's possible to display several values from the customdata in the hovertemplate?

@emmanuelle what do you mean by more than one dimension? Right now, I made it so that if customdata is an array, accessing customdata from an hovertemplate will return the value at this point's index (ie. customdata[pointNumber]). Is that what you're after?

@emmanuelle
Copy link
Contributor

@antoinerg yes. I just wanted to be sure that if customdata = [['a', 'b'], ['c', 'd']], it is possible to access either a with customdata[0] in the template, or b with customdata[1], etc.

@antoinerg
Copy link
Contributor Author

@antoinerg yes. I just wanted to be sure that if customdata = [['a', 'b'], ['c', 'd']], it is possible to access either a with customdata[0] in the template, or b with customdata[1], etc.

Thanks for the clarification! I'm not sure this is handled properly. I'll add a test :)

@antoinerg
Copy link
Contributor Author

@emmanuelle tested to work properly in c43873a

@archmoj archmoj added this to the v1.53.0 milestone Mar 10, 2020
@archmoj archmoj requested a review from alexcjohnson March 10, 2020 20:40
valType: 'data_array',
editType: 'calc',
description: [
'Assigns extra data each node.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
'Assigns extra data each node.'
'Assigns extra data to each node.'

valType: 'data_array',
editType: 'calc',
description: [
'Assigns extra data each link.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
'Assigns extra data each link.'
'Assigns extra data to each link.'

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Looks good! 💃

@antoinerg antoinerg merged commit 8f049fd into master Mar 10, 2020
@antoinerg antoinerg deleted the sankey-customdata branch March 10, 2020 21:30
@jzamalloa1
Copy link

Thank you very much all for working on this much needed feature! Do you know if this (node|customdata, link|customdata) will be pushed to the plotly sankey python library? That would be extremely useful, thanks!

@archmoj
Copy link
Contributor

archmoj commented Mar 15, 2020

I think so.

@alexcjohnson
Copy link
Collaborator

@jzamalloa1 you mean the sankey trace type in the plotly package? Yes, the next release of the package - due out within the next week I would expect - will include this feature.

@xanderwallace85
Copy link

@jzamalloa1 you mean the sankey trace type in the plotly package? Yes, the next release of the package - due out within the next week I would expect - will include this feature.

Great! Same for the python library?

@antoinerg
Copy link
Contributor Author

Great! Same for the python library?

Yes, the Python library will also get the improvements very shortly.

@xanderwallace85
Copy link

Great! Same for the python library?

Yes, the Python library will also get the improvements very shortly.

Any news about the updated Python library? :(

@PGrothaus
Copy link

It's in the new python release 4.6.0. Thanks a lot everyone involved, it's immensely useful!

@xanderwallace85
Copy link

It's in the new python release 4.6.0. Thanks a lot everyone involved, it's immensely useful!

Hi @PGrothaus,
I believe the problem is not completely solved in plotly. For instance, regarding the sunburst, the case to follow doesn't work properly:

customdata=np.dstack((a, b)),
hovertemplate='<br>%{customdata[0]:.3f} <br> %{customdata[1]:.3f} ',

However, if I have simply

customdata=a,
hovertemplate='<br>%{customdata:.3f} <br>',

then everything works smoothly.

@antoinerg
Copy link
Contributor Author

@xanderwallace85 I think it would be better if you opened a separate issue for the sunburst case you're describing!

Thank you for using plotly.js :)

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

Successfully merging this pull request may close these issues.

[Request] Allow customdata attribute to be accessed by hovertemplate using sankey,
8 participants