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

REF: implement groupby std, min, max as EA methods #51116

Closed
wants to merge 15 commits into from

Conversation

jbrockmendel
Copy link
Member

xref #51003 does the same thing for quantile
xref #43682 asking for something similar (but not for these particular reductions)

Allows EA authors to implement performant implementations (including us for e.g. pyarrow dtypes).

@mroeschke mroeschke added Groupby Reduction Operations sum, mean, min, max, etc. labels Feb 2, 2023
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

There any performance impact here?

pandas/core/arrays/masked.py Outdated Show resolved Hide resolved
@jbrockmendel
Copy link
Member Author

There any performance impact here?

Not directly, but a big part of the idea is to make it easier to implement performant implementations for our pyarrow arrays (and in my daydreams, for 3rd party authors to do Distributed/GPU EA implementations)

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Both the direction this moves toward and the implementation look good to me. cc @mroeschke @jorisvandenbossche

@mroeschke
Copy link
Member

Does any/all/std for sure need a different API then groupby_op in #51166?

@jbrockmendel
Copy link
Member Author

Does any/all/std for sure need a different API then groupby_op in #51166?

No. #52053 moves std to use WrappedCythonOp, so it would no longer need a separate method here. #52043 is a step in the same direction for any/all

@mroeschke
Copy link
Member

Okay so IIUC, _groupby_any_all and _groupby_std is a bridge to _groupby_op in #51166?

@jbrockmendel
Copy link
Member Author

Okay so IIUC, _groupby_any_all and _groupby_std is a bridge to _groupby_op in #51166?

Yes. Though at this point I'm almost ready to close this.

@jbrockmendel
Copy link
Member Author

Closing in favor of #52089

@jbrockmendel jbrockmendel deleted the ref-ea-gb-reductions branch March 20, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Groupby Reduction Operations sum, mean, min, max, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants