Skip to content

fixed bug in hover data #2544

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

Merged
merged 2 commits into from
Jun 22, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions packages/python/plotly/plotly/express/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This works because the default for args["labels"] is set to {} but probably we should use None instead, what do you think @nicolaskruchten ?

Copy link
Contributor

Choose a reason for hiding this comment

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

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... ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created #2562 to talk about this issue since it already exists before this PR.

try:
return reversed_labels[column]
except Exception:
return column


def _is_continuous(df, col_name):
return df[col_name].dtype.kind in "ifc"

Expand Down Expand Up @@ -434,11 +442,12 @@ 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():
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

might want to add a comment here explaining... it took me 4 tries and 10min to figure this one out :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok :-). Yes, that's what you said.

if k_args in args["hover_data"]:
if args["hover_data"][k_args][0]:
if isinstance(args["hover_data"][k_args][0], str):
mapping_labels_copy[k] = v.replace(
"}", "%s}" % args["hover_data"][k][0]
"}", "%s}" % args["hover_data"][k_args][0]
)
else:
_ = mapping_labels_copy.pop(k)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@ def test_newdatain_hover_data():
)


def test_formatted_hover_and_labels():
df = px.data.tips()
fig = px.scatter(
df,
x="tip",
y="total_bill",
hover_data={"total_bill": ":.1f"},
labels={"total_bill": "Total bill"},
)
assert ":.1f" in fig.data[0].hovertemplate


def test_fail_wrong_column():
with pytest.raises(ValueError) as err_msg:
px.scatter(
Expand Down