Skip to content

REF: _cython_operation handle values.ndim==1 case up-front #40672

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 2 commits into from
Mar 29, 2021

Conversation

jbrockmendel
Copy link
Member

  • closes #xxxx
  • tests added / passed
  • Ensure all linting tests pass, see here for how to run them
  • whatsnew entry

@jreback jreback added Groupby Refactor Internal refactoring of code labels Mar 29, 2021
@jreback jreback added this to the 1.3 milestone Mar 29, 2021
@jreback
Copy link
Contributor

jreback commented Mar 29, 2021

neutralish on perf?

@jbrockmendel
Copy link
Member Author

Yes

@jreback jreback merged commit 3c3589b into pandas-dev:master Mar 29, 2021
@jbrockmendel jbrockmendel deleted the ref-gbops-1dim branch March 29, 2021 18:42
sthagen added a commit to sthagen/pandas-dev-pandas that referenced this pull request Mar 29, 2021
REF: _cython_operation handle values.ndim==1 case up-front (pandas-dev#40672)
@jorisvandenbossche
Copy link
Member

This will give a slowdown for ArrayManager, because you are calling itself (_cython_operation) again, which means that everything in the beginning of _cython_operation is repeated, and thus done twice per column (check for numeric dtype, for disallow_invalid_ops, check for extension arrays -> those are already costly when done repeatedly in wide dataframes).

@jbrockmendel
Copy link
Member Author

and thus done twice per column (check for numeric dtype, for disallow_invalid_ops, check for extension arrays -> those are already costly when done repeatedly in wide dataframes).

could move these to after the 1d check

@jorisvandenbossche
Copy link
Member

I am not sure that's possible with the dispatching to _ea_wrap_cython_operation, which needs the 1D EA as is, and not reshaped to a 2D array

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Groupby Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants