-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
gh-104003: Implement PEP 702 #104004
gh-104003: Implement PEP 702 #104004
Conversation
JelleZijlstra
commented
Apr 30, 2023
•
edited by bedevere-bot
Loading
edited by bedevere-bot
- Issue: Implement PEP 702: Marking deprecations using the type system #104003
This is copied from python/typing_extensions#105. |
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.
LGTM.
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.
Waiting for the other reviewers to pipe in? This seems pretty straightforward.
The PEP hasn't been accepted yet! |
Planning to hit the merge button the moment the PEP is accepted :) |
PEP 702 has been accepted, but it's changed since this PR. I'll update the PR soon to put the decorator in |
Misc/NEWS.d/next/Library/2023-04-29-20-49-13.gh-issue-104003.-8Ruk2.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
The issue is when using |
Got it. What if we did something like this? diff --git a/Lib/warnings.py b/Lib/warnings.py
index 36da0e75c6..fc8a10e71f 100644
--- a/Lib/warnings.py
+++ b/Lib/warnings.py
@@ -621,6 +621,7 @@ def wrapper(*args, **kwargs):
f"a class or callable, not {arg!r}"
)
+ decorator.__origin__ = deprecated
return decorator Then third-party code could do checks like this: from typing import get_origin
from warnings import decorated
if get_origin(obj) is deprecated:
... # it was a wrapper function returned by `warnings.deprecated("message")` |
Doesn't seem to be |
It's already sorta awkwardly overloaded in what it means tbh — the |
A class seems somewhat more elegant to me, but yeah, that would work. |
I'm OK with making it a class. It's not what the object is meant for, but it's a reasonable extension. Using |
Doc/whatsnew/3.13.rst
Outdated
* The new :func:`warnings.deprecated` decorator provides a way to communicate | ||
deprecations to :term:`static type checkers <static type checker>` and | ||
to warn on usage of deprecated classes and functions. |
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.
Despite static typing being the original motivation for the feature, I would actually put the runtime effect first here:
* The new :func:`warnings.deprecated` decorator provides a way to communicate | |
deprecations to :term:`static type checkers <static type checker>` and | |
to warn on usage of deprecated classes and functions. | |
* The new :func:`warnings.deprecated` decorator provides an ergonomic way to | |
mark a class or function as deprecated. A deprecation warning will be | |
emitted whenever a decorated function or class is used at runtime. The | |
decorator is also understood by | |
:term:`static type checkers <static type checker>`, which will emit warnings | |
if they identify a decorated function or class being used. |
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 still want to put the type checker effect first because that's the unique part. You can write a decorator that generates runtime warnings yourself; the new and exciting part is that this decorator is also understood by static type checkers.
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 can do, sure, but do people? It was possible to reimplement itertools.batched
in a few lines of code before Python 3.12, but lots of people were still very excited by its inclusion in the stdlib in Python 3.12.
I think this new decorator here could prove pretty popular with people who don't use static typing! But, I don't feel strongly; it looks fine to me now :)
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.
Sure, there are lots of third-party deprecation decorators. The Deprecated
library, I think Flask has one internally (David Lord mentioned it in PEP 702 discussions), I know at Quora we have one internally.
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'm not sure if that's meant to be a point in agreement or in disagreement with the point I'm making that this decorator could prove pretty popular with people who don't use static typing. Anyway, as I say, I'm happy with the docs now!
Thanks @AlexWaygood for the review! I pushed some changes. |
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.
LGTM
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com> Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>