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.

Sign up for GitHub

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

fixed bug in hover data #2544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ed1ecc
fixed bug in hover data
emmanuelle Jun 5, 2020
23ef3b8
addressed review comments
emmanuelle Jun 12, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
### Fixed

- Fixed special cases with `px.sunburst` and `px.treemap` with `path` input ([#2524](https://github.com/plotly/plotly.py/pull/2524))
- Fixed bug in `hover_data` argument of `px` functions, when the column name is changed with labels and `hover_data` is a dictionary setting up a specific format for the hover data ([#2544](https://github.com/plotly/plotly.py/pull/2544)).

## [4.8.1] - 2020-05-28

Expand Down
22 changes: 18 additions & 4 deletions packages/python/plotly/plotly/express/_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,18 @@ def get_label(args, column):
return column


def invert_label(args, column):
"""Invert mapping.
Find key corresponding to value column in dict args["labels"].
Returns `column` if the value does not exist.
"""
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 +446,13 @@ 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):
# We need to invert the mapping here
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# 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"]

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
12 changes: 12 additions & 0 deletions packages/python/plotly/plotly/tests/test_core/test_px/test_px_hover.py
View file
Open in desktop
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