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

Jump to bottom

Deprecate passing args as positional in DataFrame/Series.clip #41511

Merged
merged 8 commits into from May 26, 2021
Merged

Deprecate passing args as positional in DataFrame/Series.clip #41511

merged 8 commits into from May 26, 2021

Conversation

MarcoGorelli
Copy link
Member

@MarcoGorelli MarcoGorelli commented May 16, 2021 •
edited

@MarcoGorelli MarcoGorelli added the Deprecate Functionality to remove in pandas label May 16, 2021
@MarcoGorelli MarcoGorelli force-pushed the deprecate-nonkeyword-args-clip branch from cd733ac to a40bad1 Compare May 16, 2021 14:01
@MarcoGorelli MarcoGorelli force-pushed the deprecate-nonkeyword-args-clip branch from a40bad1 to 0345df0 Compare May 16, 2021 14:11
simonjayhawkins
simonjayhawkins reviewed May 17, 2021
View reviewed changes
pandas/tests/frame/methods/test_clip.py Outdated
df = DataFrame({"a": [1, 2, 3]})
msg = (
r"Starting with Pandas version 2\.0 all arguments of clip except "
r"for the arguments 'self', 'lower' and 'upper' will be keyword-only"
Copy link
Member

Choose a reason for hiding this comment

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

I think deprecate_nonkeyword_arguments should be updated first to give a clearer message for methods (i.e. not mention self and maybe also mention the class)

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, agreed, I'll make a PR to update that first

Copy link
Contributor

Choose a reason for hiding this comment

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

ping on that PR :->

@jorisvandenbossche jorisvandenbossche changed the title deprecate non-keywordargs in clip Deprecate passing args as positional in DataFrame/Series.clip May 17, 2021
@MarcoGorelli MarcoGorelli mentioned this pull request May 17, 2021
Merged
@MarcoGorelli MarcoGorelli marked this pull request as draft May 18, 2021 08:13
@MarcoGorelli MarcoGorelli marked this pull request as ready for review May 18, 2021 20:15
@MarcoGorelli MarcoGorelli force-pushed the deprecate-nonkeyword-args-clip branch from b4ad229 to 37faa10 Compare May 18, 2021 20:37
@jreback jreback added this to the 1.3 milestone May 19, 2021
@jreback
Copy link
Contributor

jreback commented May 19, 2021

can you rebase

This was referenced May 19, 2021
Closed
Merged
@MarcoGorelli MarcoGorelli force-pushed the deprecate-nonkeyword-args-clip branch from e14c5dd to b4dec9e Compare May 19, 2021 07:17
@MarcoGorelli MarcoGorelli force-pushed the deprecate-nonkeyword-args-clip branch from a81e7b4 to a6d64d1 Compare May 19, 2021 19:45
@MarcoGorelli MarcoGorelli requested a review from jreback May 20, 2021 13:14
@simonjayhawkins
Copy link
Member

@MarcoGorelli it looks like deprecate_nonkeyword_arguments is not preserving the type annotations. maybe should fix before too many of these deprecations are merged.

@MarcoGorelli
Copy link
Member Author

Hey @simonjayhawkins - unfortunately, I don't think that's possible. I think we need to choose between:

  • making these arguments keyword-only without a warning
  • not making these arguments keyword-only and putting up with tons of overloads
  • making these arguments keyword-only, but temporarily (until 1.3 is out - we can then remove the deprecate_nonkeyword_arguments decorator on master) lose some internal type checking

and I'd like to make the case that the third one is the least undesirable

@simonjayhawkins
Copy link
Member

not tried but cannot do -> Callable[[_F], _F]?

@MarcoGorelli
Copy link
Member Author

I'll try again, hopefully I spoke prematurely

@MarcoGorelli
Copy link
Member Author

I'll try again, hopefully I spoke prematurely

Indeed - #41608 - thanks for having pushed me on this 🙏

@MarcoGorelli MarcoGorelli marked this pull request as draft May 23, 2021 20:36
@MarcoGorelli MarcoGorelli marked this pull request as ready for review May 24, 2021 07:40
@jreback jreback mentioned this pull request May 26, 2021
Closed
@jreback jreback merged commit b880cf9 into pandas-dev:master May 26, 2021
@MarcoGorelli MarcoGorelli deleted the deprecate-nonkeyword-args-clip branch May 26, 2021 08:05
TLouf pushed a commit to TLouf/pandas that referenced this pull request Jun 1, 2021
@MarcoGorelli @TLouf
Deprecate passing args as positional in DataFrame/Series.clip (pandas…
84bcc11 
…-dev#41511)
JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
@MarcoGorelli @JulianWgs
Deprecate passing args as positional in DataFrame/Series.clip (pandas…
1f85092 
…-dev#41511)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback Awaiting requested review from jreback

@simonjayhawkins simonjayhawkins Awaiting requested review from simonjayhawkins

Assignees
No one assigned
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@MarcoGorelli @jreback @simonjayhawkins