-
Notifications
You must be signed in to change notification settings - Fork 159
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
(chore): convert DeprecationWarnings
to FutureWarnings
where possible
#1874
base: main
Are you sure you want to change the base?
Conversation
DeprecationWarnings
to FutureWarnings
where poss…DeprecationWarnings
to FutureWarnings
where possible
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1874 +/- ##
==========================================
- Coverage 86.11% 83.67% -2.44%
==========================================
Files 40 40
Lines 6242 6242
==========================================
- Hits 5375 5223 -152
- Misses 867 1019 +152
|
@@ -359,14 +359,14 @@ def warn_once(msg: str, category: type[Warning], stacklevel: int = 1): | |||
|
|||
def deprecated( | |||
new_name: str, | |||
category: type[Warning] = DeprecationWarning, | |||
category: type[Warning] = FutureWarning, |
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.
Maybe we remove this altogether and always force FutureWarning
?
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.
works for me!
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.
PendingDeprecationWarning
s are even weaker than DeprecationWarning
s: They are completely ignored by default. So I say we should upgrade these too.
Yes I guess my point is that this is basically misused. One use is a parent class for |
I'm not sure about
PendingDeprecationWarning
(which remain untouched - if you look at the codebase, there are some strange uses of them which I think is a separate issue) but otherwise this PR removesDeprecationWarning
. I'm marking as 0.12 so as not to break people's CI. In that case I would probably put concrete timelines on this like "Next minor release i.e., 0.13" we will remove these.