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

Jump to bottom

Merged

emilykl merged 2 commits into plotly:master from MarcoGorelli:hardcoded-var

Jan 30, 2025

Merged

fix: custom category order was hard-coded #5000

emilykl merged 2 commits into plotly:master from MarcoGorelli:hardcoded-var

Jan 30, 2025

+38 −3

Conversation

@MarcoGorelli

Copy link

Contributor

@MarcoGorelli MarcoGorelli commented Jan 30, 2025 •

edited

Loading

closes #4999

@MarcoGorelli MarcoGorelli force-pushed the hardcoded-var branch from cc8a1a1 to b2b0282 Compare

January 30, 2025 08:49

@MarcoGorelli


 fix: custom category order was hard-coded

3abd6d4

@MarcoGorelli MarcoGorelli force-pushed the hardcoded-var branch from b2b0282 to 3abd6d4 Compare

January 30, 2025 09:16

@MarcoGorelli MarcoGorelli marked this pull request as ready for review

January 30, 2025 09:22

FBruzzesi

FBruzzesi reviewed

Jan 30, 2025

View reviewed changes

packages/python/plotly/plotly/tests/test_optional/test_px/test_px_functions.py

"On Route",

"Delivered",

]

Copy link

Contributor

@FBruzzesi FBruzzesi Jan 30, 2025

Choose a reason for hiding this comment

The 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 go.Pie:

 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)

Copy link

Contributor Author

@MarcoGorelli MarcoGorelli Jan 30, 2025

Choose a reason for hiding this comment

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

yup, thanks for reviewing!

@MarcoGorelli


 add go.Pie comparison

9043fff

emilykl

emilykl reviewed

Jan 30, 2025

View reviewed changes

packages/python/plotly/plotly/express/_core.py

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)

Copy link

Contributor

@emilykl emilykl Jan 30, 2025

Choose a reason for hiding this comment

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

@MarcoGorelli What's the reason for using np.arange here rather than range? Is there a performance difference?

Copy link

Contributor Author

@MarcoGorelli MarcoGorelli Jan 30, 2025

Choose a reason for hiding this comment

The 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!