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

a bandage solution to issue #2177 #2185

Closed
wants to merge 2 commits into from
Closed

a bandage solution to issue #2177 #2185

wants to merge 2 commits into from

Conversation

PandaTinker
Copy link

A naive attempt to fix issue #2177.
When the field is "custom_data", it ensures the input is a list.

@PandaTinker PandaTinker changed the title a bandage solution a bandage solution to issue #2177 Feb 12, 2020
@emmanuelle
Copy link
Contributor

@wornbb thank you for your contribution. There are pros and cons about this kind of "under-the-hood magic" fixes, so I'm a bit hesitant here. Let's wait maybe for other devs to comment? But we already do this in some parts of plotly.py, like for the data argument of a go.Figure.

At the moment we accept several kinds of iterables for custom_data (and hover_data, so if decide to go for your solution for hover_data, we would need the same for hover_data), like tuple, numpy arrays etc. For a tuple we also need to keep the argument as it is.

One tricky corner case is what to do with custom_data=np.arange([1, 2, 3]), since it could correspond either to column names (for a dataframe created from a numpy array) or to the custom_data themselves... Passing column names inside a numpy array is a really weird syntax but it works as of now, basically we can take everything which can be indexed.

Another solution can be to convert only when the type is str of pandas series.

To be discussed together! Thank you again @wornbb

@emmanuelle
Copy link
Contributor

@nicolaskruchten interested in your opinion here when you have the time :-).

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

Successfully merging this pull request may close these issues.

2 participants