Skip to content

CLN: Catch more specific exceptions in groupby #27909

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 5 commits into from
Sep 3, 2019

Conversation

jbrockmendel
Copy link
Member

Working hypothesis is that this will help turn up the problem underlying #27902. Regardless catching more specific exceptions is worthwhile.

@jreback jreback added this to the 1.0 milestone Aug 15, 2019
@jbrockmendel
Copy link
Member Author

Thoughts here? Conflicts are going to start building up with local branches addressing catching-too-much

Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

Looks good - more specific is certainly better with these

@jreback
Copy link
Contributor

jreback commented Sep 2, 2019

can you run groupby perf benchamarks. some of the code you changes was a sore point for perf recently.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

rebase & run groupby asvs

@jbrockmendel
Copy link
Member Author

Rebased. asvs look like noise, which makes sense.

asv continuous master HEAD -E virtualenv -f 1.1 -b groupby
[...]
       before           after         ratio
     [91e5b85a]       [0c8ae4a9]
     <master>         <winapply>
+         111±3μs          170±2μs     1.53  groupby.GroupByMethods.time_dtype_as_field('float', 'shift', 'transformation')
+      10.0±0.7ms       12.9±0.2ms     1.28  groupby.Categories.time_groupby_nosort
+         121±4μs          135±6μs     1.12  groupby.GroupByMethods.time_dtype_as_group('object', 'any', 'direct')
-         174±4μs          121±4μs     0.70  groupby.GroupByMethods.time_dtype_as_group('datetime', 'any', 'transformation')

e.g. re-running time_groupby_nosort comes back with NOT SIGNIFICANTLY CHANGED.

@TomAugspurger TomAugspurger merged commit 9cb5de0 into pandas-dev:master Sep 3, 2019
@TomAugspurger
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants