Hover format or skip

[emmanuelle]

Closes #1774

Still need to add doc (and probably more tests), but early feedback is welcome.

[emmanuelle]

I'm a bit surprised by the test failure on python 2.7 (why would a hovertemplate be different with python 2.7 and python 3?)

[nicolaskruchten]

The Python 2.7 error is just an ordering problem due to dict order (in)stability...

[emmanuelle]

ahhh thank you :-). I'll change the test then. Python 3 users won't have the problem since dicts are insertion-ordered, and py2 users can always use an OrderedDict instead.

[nicolaskruchten]

So this approach looks pretty good. I don't love the string manipulations, although they seem pretty safe!

One wrinkle: we support two different kinds of formatting... d3-format for numbers and d3-time-format for dates, and the delimiter between the attribute name and the formatting string is different (: vs |) ... how can we account for this? I think we should force the user to include the first character, at least in the d3-time-format case.

[nicolaskruchten]

Example for date axes:

[emmanuelle]

Sure, let's add the delimiter. As for string manipulations, maybe there's a cleaner way to do it, this was a first pass to see if it worked.

[emmanuelle]

@nicolaskruchten I think this one is ready for review.

[nicolaskruchten]

Can we also accept something like hover_data = dict(new_col=[1,2,3,4,5]), to be treated the same as hover_data=dict(new_col=(True, [1,2,3,4,5])) ?

[emmanuelle]

Can we also accept something like hover_data = dict(new_col=[1,2,3,4,5]), to be treated the same as hover_data=dict(new_col=(True, [1,2,3,4,5])) ?

I could maybe be convinced, but I liked the idea that the value is a formatting information, except when it's a tuple, in which case it's (formatting, data). It's also the logics which I have used in the code, converting everything to tuples internally.

[nicolaskruchten]

Right, but I feel like if we've already got a dict here (and we're already doing label manipulation), we might as well give the user a shortcut to hover_data=[ <list> ], labels=dict(hover_data_0="name") in the form of hover_data={"name": <list> }. We can internally convert <list> to (True, <list>) for them.

[nicolaskruchten]

Not quite done the review, but it looks great so far! We need to merge this in before #2336, as I think I'll have an easier time rebasing :)

[nicolaskruchten]

Huh, it took me a while to understand how the dataframe loop was working, and then I realized that this happens:

[nicolaskruchten]

So when we iterate through an "array attrable" and it's a dict, it sort of "just works" :) nice!

[emmanuelle]

So when we iterate through an "array attrable" and it's a dict, it sort of "just works" :) nice!

Yes when you iterate through a dict it iterates through its keys. Simple and elegant ;-).

[nicolaskruchten]

OK, this is awesome :)

💃

[emmanuelle]

Thanks for the 💃 let me just add the changelog entry and I'll merge.