-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
ENH: New proposed deprecation policy #58169
Conversation
One thing to note is that when using |
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
Gentle ping @lithomas1. I would like this to move forward so I don't have to update it every time a new deprecation is added |
Thanks for the reminder (and sorry that this slipped off my radar). |
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 left some more comments.
I wouldn't be comfortable merging this in by myself. Let's see what some other core devs think.
cc @pandas-dev/pandas-core for other thoughts on this.
@@ -24,6 +24,10 @@ 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.
It might be good to add some reasoning to this
(e.g. This was done to give downstream packages more time to adjust to pandas deprecations, which should reducing the amount of warnings that an user gets from code that isn't theirs.)
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 we'd have to also say that there would always be a minimal amount of time (6 months, maybe more??) between the last minor version before the next 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.
a minimal amount of time (6 months, maybe more??) between the last minor version before the next major release.
Is this consistent? 3.0 was planned to be released within 6 months of releasing 2.2.0 (#57064)
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.
a minimal amount of time (6 months, maybe more??) between the last minor version before the next major release.
Is this consistent? 3.0 was planned to be released within 6 months of releasing 2.2.0 (#57064)
In your last commit, you wrote "This was done to give downstream packages more time to adjust to pandas deprecations". So if we are going to have a policy about this, then we have to decide how much time is sufficient for "more time".
One thing we also could consider doing is decoupling the test changes from this PR, and just introducing That way there's less chances of this creating conflicts and getting out of sync. |
I think switching to Future warning only one release before the release that will break is too short. My motion on this is that it would be only 6 months or so, and sometimes less. Often end users report issues to projects through future warnings. This then requires downstream to make the changes and make a release before the next pandas, or face a break (unless strictly capping, which is not common). Producing a FutureWarning 2 releases prior to breaking things seems like a more balanced approach. |
This reverts commit f4e0d00.
That would mean any deprecations released in the last 2 minor releases will immediately be FutureWarnings which means downstream libraries won't have any time to handle/suppress these warnings. Had we implemented your idea earlier, v2.1 deprecations would immediately be FutureWarning (there are a lot of them, see #50578), one of them being the cause of #54970 being opened in the first place. I guess if we think some deprecation needs attention by end users, we can leave it as a FutureWarning and postpone enforcing the deprecation to a later major release. |
Pandas4DeprecationWarning = DeprecationWarning | ||
Pandas5DeprecationWarning = DeprecationWarning | ||
CurrentDeprecationWarning = Pandas4DeprecationWarning |
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 this should be implemented as a Generic
class with a parameter corresponding to the version number. So you'd have PandasDeprecationWarning[4]
and PandasDeprecationWarning[5]
, etc. This would allow people to write code that would catch a PandasDeprecationWarning
without having to worry about the particular version number.
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.
How will the class decide whether to inherit from FutureWarning
or DeprecationWarning
?
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.
How will the class decide whether to inherit from
FutureWarning
orDeprecationWarning
?
You could make the base class a parameter of the Generic
declaration.
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.
You would also need to update https://github.com/pandas-dev/pandas/blob/main/doc/source/development/policies.rst as part of this PR
@Aloqeely we had a discussion about our deprecation policy at the bi-weekly developers meeting, and we think that a PDEP should be created that would formalize the policy and allow for greater discussion. Are you willing to write up a PDEP with a proposed policy? |
Sure, I can give it a go. |
xref #54970
Use
DeprecationWarning
until the last minor version before a major release then we switch toFutureWarning