-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix: custom category order was hard-coded #5000
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2147,6 +2147,8 @@ def process_dataframe_timeline(args): | |
|
|
||
|
|
||
| def process_dataframe_pie(args, trace_patch): | ||
| import numpy as np | ||
|
|
||
| names = args.get("names") | ||
| if names is None: | ||
| return args, trace_patch | ||
|
|
@@ -2159,12 +2161,12 @@ def process_dataframe_pie(args, trace_patch): | |
| uniques = df.get_column(names).unique(maintain_order=True).to_list() | ||
| order = [x for x in OrderedDict.fromkeys(list(order_in) + uniques) if x in uniques] | ||
|
|
||
| # Sort args['data_frame'] by column 'b' according to order `order`. | ||
| # Sort args['data_frame'] by column `names` according to order `order`. | ||
| token = nw.generate_temporary_column_name(8, df.columns) | ||
| args["data_frame"] = ( | ||
| df.with_columns( | ||
| nw.col("b") | ||
| .replace_strict(order, range(len(order)), return_dtype=nw.UInt32) | ||
| nw.col(names) | ||
| .replace_strict(order, np.arange(len(order)), return_dtype=nw.UInt32) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MarcoGorelli What's the reason for using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, @FBruzzesi had noted that it was generally more performant, and numpy is still required in plotly express anyway - no objections to changing this back of course! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, makes sense to me, thanks! |
||
| .alias(token) | ||
| ) | ||
| .sort(token) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,39 @@ def test_pie_like_px(): | |
| _compare_figures(trace, fig) | ||
|
|
||
|
|
||
| def test_pie_custom_category_order(constructor): | ||
| # https://github.com/plotly/plotly.py/issues/4999 | ||
| data = { | ||
| "status": ["On Route", "Pending", "Waiting Result", "Delivered"], | ||
| "count": [28, 10, 73, 8], | ||
| } | ||
| df = constructor(data) | ||
| custom_order = ["Pending", "Waiting Result", "On Route", "Delivered"] | ||
| result = px.pie( | ||
| data_frame=df, | ||
| values="count", | ||
| names="status", | ||
| category_orders={"status": custom_order}, | ||
| ) | ||
| assert list(result.to_dict()["data"][0]["labels"]) == [ | ||
| "Pending", | ||
| "Waiting Result", | ||
| "On Route", | ||
| "Delivered", | ||
| ] | ||
| values_ = np.array( | ||
| [ | ||
| x[0] | ||
| for x in sorted( | ||
| zip(data["count"], data["status"]), | ||
| key=lambda t: custom_order.index(t[1]), | ||
| ) | ||
| ] | ||
| ) | ||
| trace = go.Pie(values=values_, labels=custom_order) | ||
| _compare_figures(trace, result) | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was doing a similar fix 🙈 You could also add the comparison with values_ = np.array([ x[0] for x in sorted(zip(data["count"], data["status"]), key=lambda t: custom_order.index(t[1])) ]) trace = go.Pie( values=values_, labels=custom_order, ) _compare_figures(trace, fig) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup, thanks for reviewing! |
||
|
|
||
| def test_sunburst_treemap_colorscales(): | ||
| labels = ["Eve", "Cain", "Seth", "Enos", "Noam", "Abel", "Awan", "Enoch", "Azura"] | ||
| parents = ["", "Eve", "Eve", "Seth", "Seth", "Eve", "Eve", "Awan", "Eve"] | ||
|
|
||