Skip to content

REF: Move part of groupby.agg to apply #39311

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 1 commit into from
Jan 22, 2021

Conversation

rhshadrach
Copy link
Member

  • tests added / passed
  • Ensure all linting tests pass, see here for how to run them

pre-commit adjusted to account for the false positive on Union[SeriesGroupBy, DataFrameGroupBy].

Moves part of groupby.agg implementation from pandas.core.aggregation to pandas.core.apply. The only difference in the code is that in apply, the error message raised is different when the argument is a string (e.g. "sum") and the kwarg axis=1 is provided. Added a test to check for error message.

After this will be resample and rolling, then much of the implementation code in core.aggregation code can be moved into apply.

@rhshadrach rhshadrach added Apply Apply, Aggregate, Transform, Map Groupby Refactor Internal refactoring of code labels Jan 21, 2021
@@ -203,6 +203,18 @@ def test_aggregate_str_func(tsframe, groupbyfunc):
tm.assert_frame_equal(result, expected)


def test_agg_str_with_kwarg_axis_1_raises(df, reduction_func):
gb = df.groupby(level=0)
if reduction_func in ("idxmax", "idxmin"):
Copy link
Contributor

Choose a reason for hiding this comment

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

is this new?

Copy link
Member Author

Choose a reason for hiding this comment

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

With regards to idxmax/idxmin, no. Those are the two reduction_func that support axis=1 (whereas all others do not). However, they fail here because df has string and numeric columns.

@jreback jreback added this to the 1.3 milestone Jan 22, 2021
@jreback jreback merged commit 5d9e37d into pandas-dev:master Jan 22, 2021
@jreback
Copy link
Contributor

jreback commented Jan 22, 2021

thanks @rhshadrach

@rhshadrach rhshadrach deleted the groupby_agg_via_apply branch January 28, 2021 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Apply Apply, Aggregate, Transform, Map Groupby Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants