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.

Sign up for GitHub

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

DEPR: Remove unnecessary kwargs in groupby ops #50483

Closed
wants to merge 16 commits into from
Closed

DEPR: Remove unnecessary kwargs in groupby ops #50483

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7ebf822
Depr: unnecessary kwargs in groupby ops
ramvikrams Dec 29, 2022
88ab270
update
ramvikrams Dec 29, 2022
aff453d
added the whatsnew entey and modified the code
ramvikrams Dec 29, 2022
4f6548b
added the decorator above the functions
ramvikrams Dec 31, 2022
2a680be
update
ramvikrams Dec 31, 2022
6569006
Update groupby.py
ramvikrams Dec 31, 2022
3a150f6
Update generic.py
ramvikrams Dec 31, 2022
519d9da
Update v2.0.0.rst
ramvikrams Dec 31, 2022
20c8606
added tests
ramvikrams Jan 3, 2023
b026a74
Update v2.0.0.rst
ramvikrams Jan 3, 2023
dfed625
Update test_deprecate_nonkeyword_arguments.py
ramvikrams Jan 3, 2023
1676d19
Update test_deprecate_nonkeyword_arguments.py
ramvikrams Jan 3, 2023
b3ca2f2
Update test_deprecate_nonkeyword_arguments.py
ramvikrams Jan 3, 2023
90fb8dc
Update test_deprecate_nonkeyword_arguments.py
ramvikrams Jan 3, 2023
fe8f551
corrected the tests
ramvikrams Jan 9, 2023
e45c8ff
Update test_deprecate_nonkeyword_arguments.py
ramvikrams Jan 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ Other API changes
Deprecations
~~~~~~~~~~~~
- Deprecated argument ``infer_datetime_format`` in :func:`to_datetime` and :func:`read_csv`, as a strict version of it is now the default (:issue:`48621`)
- Deprecate unused argument ``**kwargs`` from :meth:`cummax`, :meth:`cummin`, :meth:`cumsum`, :meth:`cumprod`, :meth:`take` and :meth:`skew` (:issue:`50483`)

.. ---------------------------------------------------------------------------

Expand Down
32 changes: 16 additions & 16 deletions pandas/core/groupby/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
from pandas.util._decorators import (
Appender,
Substitution,
deprecate_nonkeyword_arguments,
doc,
)

Expand Down Expand Up @@ -893,6 +894,9 @@ def fillna(
return result

@doc(Series.take.__doc__)
@deprecate_nonkeyword_arguments(
version="3.0", allowed_args=["self", "axis", "indices"]
)
Copy link
Member

Choose a reason for hiding this comment

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

Having another look, looks like deprecate_nonkeyword_arguments doesn't deprecate the argument, but makes it usable only via a keyword argument (e.g. foo(1) would produce the warning, but foo(val=1) wouldn't). If that's the case, doesn't seem like we've got a decorator to simply deprecate an argument. In that case, you'll have to produce it manually.

def take(
self,
indices: TakeIndexer,
Expand All @@ -903,6 +907,9 @@ def take(
return result

@doc(Series.skew.__doc__)
@deprecate_nonkeyword_arguments(
version="3.0", allowed_args=["self", "axis", "skipna", "numeric_only"]
)
def skew(
self,
axis: Axis | lib.NoDefault = lib.no_default,
Expand All @@ -911,11 +918,7 @@ def skew(
**kwargs,
) -> Series:
result = self._op_via_apply(
"skew",
axis=axis,
skipna=skipna,
numeric_only=numeric_only,
**kwargs,
"skew", axis=axis, skipna=skipna, numeric_only=numeric_only, **kwargs
)
return result

Expand Down Expand Up @@ -2272,16 +2275,17 @@ def fillna(
return result

@doc(DataFrame.take.__doc__)
def take(
self,
indices: TakeIndexer,
axis: Axis | None = 0,
**kwargs,
) -> DataFrame:
@deprecate_nonkeyword_arguments(
version="3.0", allowed_args=["self", "indices", "axis"]
)
def take(self, indices: TakeIndexer, axis: Axis | None = 0, **kwargs) -> DataFrame:
result = self._op_via_apply("take", indices=indices, axis=axis, **kwargs)
return result

@doc(DataFrame.skew.__doc__)
@deprecate_nonkeyword_arguments(
version="3.0", allowed_args=["self", "axis", "skipna", "numeric_only"]
)
def skew(
self,
axis: Axis | None | lib.NoDefault = lib.no_default,
Expand All @@ -2290,11 +2294,7 @@ def skew(
**kwargs,
) -> DataFrame:
result = self._op_via_apply(
"skew",
axis=axis,
skipna=skipna,
numeric_only=numeric_only,
**kwargs,
"skew", axis=axis, skipna=skipna, numeric_only=numeric_only, **kwargs
)
return result

Expand Down
9 changes: 9 additions & 0 deletions pandas/core/groupby/groupby.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class providing the base-class of operations.
Appender,
Substitution,
cache_readonly,
deprecate_nonkeyword_arguments,
doc,
)

Expand Down Expand Up @@ -3411,6 +3412,7 @@ def rank(
@final
@Substitution(name="groupby")
@Appender(_common_see_also)
@deprecate_nonkeyword_arguments(version="3.0", allowed_args=["self", "axis"])
def cumprod(self, axis: Axis = 0, *args, **kwargs) -> NDFrameT:
"""
Cumulative product for each group.
Expand All @@ -3429,6 +3431,7 @@ def cumprod(self, axis: Axis = 0, *args, **kwargs) -> NDFrameT:
@final
@Substitution(name="groupby")
@Appender(_common_see_also)
@deprecate_nonkeyword_arguments(version="3.0", allowed_args=["self", "axis"])
def cumsum(self, axis: Axis = 0, *args, **kwargs) -> NDFrameT:
"""
Cumulative sum for each group.
Expand All @@ -3447,6 +3450,9 @@ def cumsum(self, axis: Axis = 0, *args, **kwargs) -> NDFrameT:
@final
@Substitution(name="groupby")
@Appender(_common_see_also)
@deprecate_nonkeyword_arguments(
version="3.0", allowed_args=["self", "axis", "numeric_only"]
)
def cummin(
self, axis: AxisInt = 0, numeric_only: bool = False, **kwargs
) -> NDFrameT:
Expand All @@ -3472,6 +3478,9 @@ def cummin(
@final
@Substitution(name="groupby")
@Appender(_common_see_also)
@deprecate_nonkeyword_arguments(
version="3.0", allowed_args=["self", "axis", "numeric_only"]
)
def cummax(
self, axis: AxisInt = 0, numeric_only: bool = False, **kwargs
) -> NDFrameT:
Expand Down
24 changes: 24 additions & 0 deletions pandas/tests/util/test_deprecate_nonkeyword_arguments.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@
from pandas.util._decorators import deprecate_nonkeyword_arguments

import pandas._testing as tm
from pandas.core.groupby.generic import (
skew,
take,
)
from pandas.core.groupby.groupby import (
cummax,
cummin,
cumprod,
cumsum,
)


@deprecate_nonkeyword_arguments(
Expand Down Expand Up @@ -155,3 +165,17 @@ def test_class():
)
with tm.assert_produces_warning(FutureWarning, match=msg):
Foo().baz("qux", "quox")


def test_deprecated_keywords():
Copy link
Member

Choose a reason for hiding this comment

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

This file is for testing the decorator itself, these tests should go in pandas/tests/groupby/test_function.

msg = (
"In the future version of pandas the arguments args"
"and kwargs will be deprecated for these functions"
)
with tm.assert_produces_warning(FutureWarning, match=msg):
assert cummax
Copy link
Member

Choose a reason for hiding this comment

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

assert [statement] evaluates the truthiness of the statement. For example, [] will evaluate to False (and hence raise if used in an assert) whereas [1] will evaluate to True. Functions are always truthy - they evaluate to True.

Instead, this needs to call the function itself, e.g.

df = DataFrame({'a': [1, 1, 2], 'b': [3, 4, 5]})
gb = df.groupby('a')
gb.cummax(kwargs=1)

Copy link
Contributor Author

@ramvikrams ramvikrams Jan 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh thanks I'll do the changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i did the changes but still the tests are failing, don't know why

assert cummin
assert cumprod
assert cumsum
assert skew
assert take