Skip to content
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

TST: add ASV check for groupby with uint64 col #41859

Merged
merged 2 commits into from
Jun 8, 2021

Conversation

smithto1
Copy link
Member

@smithto1 smithto1 commented Jun 7, 2021

The performance regression reported in the original issue was fixed somewhere between 0.25.1 and 1.2.4. This PR just adds an ASV test to monitor for the same regression in the future.

@smithto1 smithto1 changed the title GH #28122 add ASV check for groupby with uint64 col TST: add ASV check for groupby with uint64 col Jun 7, 2021
@WillAyd
Copy link
Member

WillAyd commented Jun 8, 2021

Thanks for the PR - can you post the result to be sure? Also do you think we could just parametrize GroupByMethods to include this instead? That may be slightly more comprehensive than this one-off class, though noted that you used a similar class for Int as inspiration

@WillAyd WillAyd added Benchmark Performance (ASV) benchmarks Groupby labels Jun 8, 2021
@smithto1
Copy link
Member Author

smithto1 commented Jun 8, 2021

Thanks for the PR - can you post the result to be sure? Also do you think we could just parametrize GroupByMethods to include this instead? That may be slightly more comprehensive than this one-off class, though noted that you used a similar class for Int as inspiration

@WillAyd I prefer your suggestion to run uint through GroupByMethods. I've implemented this way and removed the one-off class.

Results attached.

asv_groupby.GroupByMethods.txt

@jreback jreback added this to the 1.3 milestone Jun 8, 2021
@jreback jreback merged commit 5abb02e into pandas-dev:master Jun 8, 2021
@jreback
Copy link
Contributor

jreback commented Jun 8, 2021

thanks @smithto1

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Benchmark Performance (ASV) benchmarks Groupby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Performance regression uint64 groupby().min() in 0.25.1 vs. 0.24.2
3 participants