NotificationsYou must be signed in to change notification settings
Fork2.7k
Star17.2k

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 ourterms of service andprivacy 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 intoplotly:masterfromMarcoGorelli:hardcoded-var

Jan 30, 2025

Merged

fix: custom category order was hard-coded#5000

emilykl merged 2 commits intoplotly:masterfromMarcoGorelli:hardcoded-var

Jan 30, 2025

Conversation

Copy link

Contributor

MarcoGorelli commentedJan 30, 2025•
edited
Loading

closes#4999

MarcoGorelli force-pushed thehardcoded-var branch fromcc8a1a1 tob2b0282Compare

January 30, 2025 08:49

fix: custom category order was hard-coded

3abd6d4

MarcoGorelli force-pushed thehardcoded-var branch fromb2b0282 to3abd6d4Compare

January 30, 2025 09:16

MarcoGorelli marked this pull request as ready for review

January 30, 2025 09:22

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

FBruzzesiJan 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 withgo.Pie:

values_=np.array([x[0]forxinsorted(zip(data["count"],data["status"]),key=lambdat:custom_order.index(t[1]))    ])trace=go.Pie(values=values_,labels=custom_order,    )_compare_figures(trace,fig)

Copy link

ContributorAuthor

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

add go.Pie comparison

9043fff

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

emilyklJan 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 usingnp.arange here rather thanrange? Is there a performance difference?

Copy link

ContributorAuthor

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