Skip to content

Correct bug when color and values correspond to the same color in treemap or sunburst #2591

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 10 commits into from
Jun 26, 2020
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- 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)).
- Made the Plotly Express `trendline` argument more robust and made it work with datetime `x` values ([#2554](https://github.com/plotly/plotly.py/pull/2554))
- Fixed bug in `px.sunburst` and `px.treemap`: when the `color` and `values`
arguments correspond to the same column, a different aggregation function has
to be used for the two arguments ([#2591](https://github.com/plotly/plotly.py/pull/2591))
- Plotly Express wide mode now accepts mixed integer and float columns ([#2598](https://github.com/plotly/plotly.py/pull/2598))
- Plotly Express `range_(x|y)` should not impact the unlinked range of marginal subplots ([#2600](https://github.com/plotly/plotly.py/pull/2600))
- `px.line` now sets `line_group=<variable>` in wide mode by default ([#2599](https://github.com/plotly/plotly.py/pull/2599))
Expand Down
18 changes: 11 additions & 7 deletions packages/python/plotly/plotly/express/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,11 +474,10 @@ def make_trace_kwargs(args, trace_spec, trace_data, mapping_labels, sizeref):
# We need to invert the mapping here
k_args = invert_label(args, k)
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_args][0]
)
formatter = args["hover_data"][k_args][0]
if formatter:
if isinstance(formatter, str):
mapping_labels_copy[k] = v.replace("}", "%s}" % formatter)
else:
_ = mapping_labels_copy.pop(k)
hover_lines = [k + "=" + v for k, v in mapping_labels_copy.items()]
Expand Down Expand Up @@ -1507,7 +1506,9 @@ def aggfunc_discrete(x):

if args["color"]:
if args["color"] == args["values"]:
aggfunc_color = "sum"
new_value_col_name = args["values"] + "_sum"
df[new_value_col_name] = df[args["values"]]
args["values"] = new_value_col_name
Copy link
Contributor

Choose a reason for hiding this comment

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

so like this, the <col>_total appears in the hoverlabel as the value. This is what we want? Not sure what would be better...

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we want <col>_sum instead? or <col>_weighted_average for the other one? not sure

Copy link
Contributor

Choose a reason for hiding this comment

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

<col>_color for the other one maybe?

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'm fine changing total to sum. I put total like branchvalues='total'. We could also put a suffix for the colors column but if we want to minimize the occurrence of such suffixes, we'd better put it on the values column (only in the case where it's the same as the colors column).

Copy link
Contributor

Choose a reason for hiding this comment

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

ok let's do sum and merge this thing :)

count_colname = args["values"]
else:
# we need a count column for the first groupby and the weighted mean of color
Expand All @@ -1526,7 +1527,7 @@ def aggfunc_discrete(x):
if not _is_continuous(df, args["color"]):
aggfunc_color = aggfunc_discrete
discrete_color = True
elif not aggfunc_color:
else:

def aggfunc_continuous(x):
return np.average(x, weights=df.loc[x.index, count_colname])
Expand Down Expand Up @@ -1584,6 +1585,9 @@ def aggfunc_continuous(x):
if args["color"]:
if not args["hover_data"]:
args["hover_data"] = [args["color"]]
elif isinstance(args["hover_data"], dict):
if not args["hover_data"].get(args["color"]):
args["hover_data"][args["color"]] = (True, None)
else:
args["hover_data"].append(args["color"])
return args
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,6 @@ def test_sunburst_treemap_with_path():
fig = px.sunburst(df, path=path, values="values")
assert fig.data[0].branchvalues == "total"
assert fig.data[0].values[-1] == np.sum(values)
# Continuous colorscale
fig = px.sunburst(df, path=path, values="values", color="values")
assert "coloraxis" in fig.data[0].marker
assert np.all(np.array(fig.data[0].marker.colors) == np.array(fig.data[0].values))
# Error when values cannot be converted to numerical data type
df["values"] = ["1 000", "3 000", "2", "4", "2", "2", "1 000", "4 000"]
msg = "Column `values` of `df` could not be converted to a numerical data type."
Expand All @@ -162,6 +158,12 @@ def test_sunburst_treemap_with_path():
path = [df.total, "regions", df.sectors, "vendors"]
fig = px.sunburst(df, path=path)
assert fig.data[0].branchvalues == "total"
# Continuous colorscale
df["values"] = 1
fig = px.sunburst(df, path=path, values="values", color="values")
assert "coloraxis" in fig.data[0].marker
assert np.all(np.array(fig.data[0].marker.colors) == 1)
assert fig.data[0].values[-1] == 8


def test_sunburst_treemap_with_path_and_hover():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,3 +151,15 @@ def test_fail_wrong_column():
"Ambiguous input: values for 'c' appear both in hover_data and data_frame"
in str(err_msg.value)
)


def test_sunburst_hoverdict_color():
df = px.data.gapminder().query("year == 2007")
fig = px.sunburst(
df,
path=["continent", "country"],
values="pop",
color="lifeExp",
hover_data={"pop": ":,"},
)
assert "color" in fig.data[0].hovertemplate