[emmanuelle]

closes #1751

[emmanuelle]

The way it works:

• the index is always added to customdata of the trace
• then the columns specified in custom_data are added to customdata
• then the ones of hover_data
• the hover template uses the only the indices of custom_data provided by hover_data

I did not implement deduplication because this will behave correctly even if the user provides the same column in custom_data and hover_data, (at the cost of unnecessary data copy).

[nicolaskruchten]

How hard would it be to implement deduplication?

[nicolaskruchten]

basically it would be a matter of, when processing hover_data, to see if the data's already there... I think we already do something like this in this step in case the data is already in x or y or something

[emmanuelle]

@nicolaskruchten no it wouldn't be hard, I can do it if you prefer. Much easier than debugging the CI problem with chart_studio anyway :-/. At the moment I'm adding nottest decorators to isolate the failing tests but of course it is only temporary

[nicolaskruchten]

hehe ok why don't you add the dedupe code and I'll try to crack this Chart Studio thing :)

[emmanuelle]

Regarding deduplication, on second thoughts it might be confusing to users. Let's say a user supplies custom_data=['sepal_length', 'sepal_width'] and hover_data=['petal_length', 'sepal_width']. In a Dash callback, they would expect to access sepal_width at customdata[2] (2 and not 1 because of the index, which is already confusing...). But if we remove duplicates but sill want to preserve the order of hover items then the index will change. I''ll look for a workaround but not sure there is one. I'm also a bit worried by the fact that if customdata[0] is the index it changes the indices of the user-provided columns. Should index be instead customdata[-1] (quite different from the Pandas convention of course, but maybe less confusing).

[nicolaskruchten]

So with custom_data=['A', 'B'] and hover_data=['C', 'B'] I would expect the items in customdata to be [<index>, A, B, C] but the order in the hoverlabel to remain C, B... this seems doable, no?

[nicolaskruchten]

I agree that putting the index in the 0-position shifts things over 1 and makes things a bit confusing, but the value of having the index always available in the same position is pretty high.

[emmanuelle]

this is why I suggested -1, which can be argued to be always at the same position, starting from the end.

[emmanuelle]

I wonder which solution (index at 0 and then increment other positions by 1, or keep positions and have index at -1) will confuse users less... Any thoughts @jonmmease ?

[emmanuelle]

and yes what you suggested is doable.

[nicolaskruchten]

Really in an ideal world we would use the ids attribute, but that has special meaning in the context of animations :(

My worry about putting it in the last position is that it's easy for a user to say "oh it's here in position N" and then hardcode N in an app instead of hardcoding it as -1 or 0.

[nicolaskruchten]

Come to think of it, I don't think there's a restriction on the type of customdata so we could store an object in there like {"index_value": "whatever", "data": ["bla", "bli", "blo"]}

[emmanuelle]

Deduplication done. As soon as the percy PR is merged I'll rebase to compare, and then we can converge on the format of customdata.

[nicolaskruchten]

How about this: let's just not automatically insert the index!

Once #1767 is done, the user can simply say custom_data=[df.index] and put it wherever they like, if they like :)

[emmanuelle]

ok I removed the index. This should be ready, I'll work on a demo now.

[nicolaskruchten]

This is actually a NamedTuple

[emmanuelle]

ok

[nicolaskruchten]

Code looks fine, but this is probably a case where a simple test case would be nice, especially during review, for stuff that's invisible like this :)

[nicolaskruchten]

Maybe a note that this data is not user-visible, but is included in events emitted by the figure?

[emmanuelle]

done

[emmanuelle]

@nicolaskruchten about a simple test, definitely yes. I still need to work on #1754 there were some version conflicts in particular to be solved.

[emmanuelle]

See https://gist.github.com/emmanuelle/fba6f0a2c387100ac1cc027b4b37e20a for a Dash app showcasing the use of custom_data.

[emmanuelle]

The good thing is that Percy did not detect changes.

[nicolaskruchten]

Great! Yeah Percy is pretty good, sometimes throws false positives but not enough to want to ditch it :)

So next steps on this PR are to land #1754 onto master, then rebase this branch onto it, add a test and merge it?

[emmanuelle]

yes ! #1754 might be a bit tricky because of version management / conflicts, but let's see how it goes.

[nicolaskruchten]

💃 very nice, thank you :)