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

BUG: Fix agg ingore arg/kwargs when given list like func #50863

Merged

Conversation

luke396
Copy link
Contributor

@luke396 luke396 commented Jan 19, 2023

Previous Pull Request #50725, for multiple elements-func list this time.

I would appreciate any feedback or suggestions on this draft pull request to help improve it. Thank you in advance for your help and cooperation.

@luke396 luke396 marked this pull request as ready for review January 20, 2023 11:04
@mroeschke mroeschke added the Apply Apply, Aggregate, Transform, Map label Jan 20, 2023
pandas/core/apply.py Outdated Show resolved Hide resolved
@luke396
Copy link
Contributor Author

luke396 commented Jan 29, 2023

cc @rhshadrach, I've reworked the implementation, any suggestions for improvement?

pandas/core/apply.py Outdated Show resolved Hide resolved
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.

Looks good. Could you also add an entry in the whatsnew for 2.0.0 under the bug fixes for reshaping.

pandas/core/apply.py Outdated Show resolved Hide resolved
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.

Looks good! Since this code is used by a variety of objects, I think some tests beyond just DataFrame should be added here.

doc/source/whatsnew/v2.0.0.rst Outdated Show resolved Hide resolved
def test__dataframe_groupy_agg_list_like_func_with_args():
# GH 50624
df = DataFrame({"x": [1, 2, 3], "y": ["a", "b", "c"]})
df = df.groupby("y")
Copy link
Member

Choose a reason for hiding this comment

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

Can you call this gb instead; calling it df is confusing.

def test__series_groupy_agg_list_like_func_with_args():
# GH 50624
data = Series([1, 2, 3])
data = data.groupby(data)
Copy link
Member

Choose a reason for hiding this comment

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

Same.

Comment on lines 648 to 652
msg = r"foo1\(\) got an unexpected keyword argument 'b'"
with pytest.raises(TypeError, match=msg):
df.agg([foo1, foo2], 0, 3, b=3, c=4)

result = df.agg([foo1, foo2], 0, 3, c=4)
Copy link
Member

Choose a reason for hiding this comment

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

This test should be using resample.

@@ -1317,7 +1317,7 @@ Reshaping
- Clarified error message in :func:`merge` when passing invalid ``validate`` option (:issue:`49417`)
- Bug in :meth:`DataFrame.explode` raising ``ValueError`` on multiple columns with ``NaN`` values or empty lists (:issue:`46084`)
- Bug in :meth:`DataFrame.transpose` with ``IntervalDtype`` column with ``timedelta64[ns]`` endpoints (:issue:`44917`)
- Bug in :func:`agg` would ignore arguments when passed a list of functions (:issue:`50863`)
- Bug in :meth:`DataFrame.agg`, :meth:`Series.agg`, :meth:`DataFrameGroupBy.agg`, :meth:`SeriesGroupBy.agg`, and :meth:`Resampler.agg` would ignore arguments when passed a list of functions (:issue:`50863`)
Copy link
Member

Choose a reason for hiding this comment

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

I should have mentioned - this should be split up with the groupby and resampler part going under their section within Bugfixes.

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.

lgtm

@rhshadrach rhshadrach added this to the 2.0 milestone Feb 12, 2023
@rhshadrach rhshadrach merged commit 9764f3d into pandas-dev:main Feb 12, 2023
@rhshadrach
Copy link
Member

Thanks @luke396!

@luke396 luke396 deleted the fix-agg-ingore-arg-given-list-like-func branch February 13, 2023 03:40
@luke396
Copy link
Contributor Author

luke396 commented Feb 13, 2023

Thanks @luke396!

This really couldn't have been done if it wasn't for your great help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Apply Apply, Aggregate, Transform, Map
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: agg ignores args/kwargs when giving a list-like argument
3 participants