Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork18.5k
ENH: Implement PDEP-17#61468
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
ENH: Implement PDEP-17#61468
Conversation
pandas/errors/__init__.py Outdated
| classPandasChangeWarning(Warning): | ||
| """ | ||
| Warning raised for any an upcoming change. | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Not in love with this name, butPandasDeprecationWarning is taken below. Bikeshedding here is most welcome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We could also make this internal if we want to not have it be public.
Seems like the label |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think there should be separate docs about the deprecation policy and these classes, aside from what is in the whatsnew document.
| @@ -24,6 +24,12 @@ Enhancement1 | |||
| Enhancement2 | |||
| ^^^^^^^^^^^^ | |||
| New Deprecation Policy | |||
| ^^^^^^^^^^^^^^^^^^^^^^ | |||
| pandas 3.0.0 introduces a new 3-stage deprecation policy: using ``DeprecationWarning`` initially, then switching to ``FutureWarning`` for broader visibility in the last minor version before the next major release, and then removal of the deprecated functionality in the major release. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Shouldn't we say that these could be tested via the classesPandasDeprecationWarning andPandasFutureWarning ?
| classPandas4Warning(PandasDeprecationWarning): | ||
| """ | ||
| Warning raised for an upcoming change that will be enforced in pandas 4.0. | ||
| See Also | ||
| -------- | ||
| errors.Pandas4Warning : Class for deprecations to be enforced in pandas 4.0. | ||
| Examples | ||
| -------- | ||
| >>> pd.errors.Pandas4Warning | ||
| <class 'pandas.errors.Pandas4Warning'> | ||
| """ | ||
| classPandas5Warning(PandasPendingDeprecationWarning): | ||
| """ | ||
| Warning raised for an upcoming change that will be enforced in pandas 5.0. | ||
| See Also | ||
| -------- | ||
| errors.Pandas4Warning : Class for deprecations to be enforced in pandas 4.0. | ||
| Examples | ||
| -------- | ||
| >>> pd.errors.Pandas5Warning | ||
| <class 'pandas.errors.Pandas5Warning'> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I wonder if we should have an abstract base classPandasWarning that has an abstract class methodversion(), and then bothPandas4Warning andPandas5Warning inherit from that class as well, and defineversion() to return 4 and 5 respectively.
Then indeprecate_nonkeyword_arguments(), instead of checking for the class, you just check forversion(). See comment there.
| frompandas.errorsimport ( | ||
| Pandas4Warning, | ||
| Pandas5Warning, | ||
| ) | ||
| ifklassisPandas4Warning: | ||
| version="4.0" | ||
| elifklassisPandas5Warning: | ||
| version="5.0" | ||
| else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If you create the ABCPandasWarning as suggested above, then you could just do
ifissubclass(klass,PandasWarning):version=klass.version()else:raiseAssertionError(f"{type(klass)=} must be a versioned warning")
| @@ -51,6 +51,12 @@ Exceptions and warnings | |||
| errors.OptionError | |||
| errors.OutOfBoundsDatetime | |||
| errors.OutOfBoundsTimedelta | |||
| errors.PandasChangeWarning | |||
| errors.Pandas4Warning | |||
| errors.Pandas5Warning | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Aside from the tests,Pandas5Warning isn't used. Is the idea that we will use it in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes.
@Dr-Irv - Thanks for the review! I'm negative on the name |
Maybe a different name would solve that? Maybe |
Uh oh!
There was an error while loading.Please reload this page.
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.Continuation of#58169
Still needs tests and some cleanup with the docs and decorator arguments, but I'd like to get a first look.
cc@Dr-Irv@Aloqeely