fixed bug in hover data #2544
fixed bug in hover data #2544
Conversation
| if k in args["hover_data"]: | ||
| if args["hover_data"][k][0]: | ||
| if isinstance(args["hover_data"][k][0], str): | ||
| k_args = invert_label(args, k) |
There was a problem hiding this comment.
OK so the problem is that k here is the renamed label? so if labels=dict(a="A") then k would be "A" and we need to find a?
There was a problem hiding this comment.
might want to add a comment here explaining... it took me 4 tries and 10min to figure this one out :)
There was a problem hiding this comment.
ok :-). Yes, that's what you said.
|
💃 once a changelog entry is added :) |
| @@ -116,6 +116,14 @@ def get_label(args, column): | |||
| return column | |||
|
|
|||
|
|
|||
| def invert_label(args, column): | |||
| reversed_labels = {value: key for (key, value) in args["labels"].items()} | |||
There was a problem hiding this comment.
This works because the default for args["labels"] is set to {} but probably we should use None instead, what do you think @nicolaskruchten ?
There was a problem hiding this comment.
Yes we should stop using {} as defaults :)
Here btw there's a potential issue if people use the same label for two things... this is a bad idea for a user to do but... ?
There was a problem hiding this comment.
I created #2562 to talk about this issue since it already exists before this PR.
| @@ -442,6 +446,7 @@ def make_trace_kwargs(args, trace_spec, trace_data, mapping_labels, sizeref): | |||
| mapping_labels_copy = OrderedDict(mapping_labels) | |||
| if args["hover_data"] and isinstance(args["hover_data"], dict): | |||
| for k, v in mapping_labels.items(): | |||
| # We need to invert the mapping here | |||
There was a problem hiding this comment.
| # We need to invert the mapping here | |
| # We need to invert the mapping here | |
| # k here is either the column name or remapped from arg["labels"] |
|
💃 |
Closes #2529