FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-104003: Implement PEP 702 by JelleZijlstra · Pull Request #104004 · python/cpython · GitHub

/ cpython Public

gh-104003: Implement PEP 702 - #104004

Merged
JelleZijlstra merged 25 commits into
python:mainfrom
JelleZijlstra:pep702
Nov 29, 2023
Merged

gh-104003: Implement PEP 702#104004
JelleZijlstra merged 25 commits into
python:mainfrom
JelleZijlstra:pep702

Conversation

JelleZijlstra commented Apr 30, 2023
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member Author

This is copied from python/typing_extensions#105.

Comment thread Lib/test/test_typing.py Outdated
Comment thread Lib/test/test_typing.py Outdated

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM.

gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Waiting for the other reviewers to pipe in? This seems pretty straightforward.

Copy link
Copy Markdown
Member

Waiting for the other reviewers to pipe in? This seems pretty straightforward.

The PEP hasn't been accepted yet!

Copy link
Copy Markdown
Member Author

Planning to hit the merge button the moment the PEP is accepted :)

Copy link
Copy Markdown
Member Author

PEP 702 has been accepted, but it's changed since this PR. I'll update the PR soon to put the decorator in warnings and sync the implementation from typing-extensions.

Comment thread Doc/glossary.rst
Comment thread Doc/library/warnings.rst Outdated
Comment thread Doc/library/warnings.rst Outdated
Comment thread Doc/library/warnings.rst Outdated
Comment thread Doc/library/warnings.rst Outdated
Comment thread Doc/whatsnew/3.13.rst Outdated
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

Viicos commented Nov 26, 2023

Copy link
Copy Markdown
Contributor

Doesn't seem to be get_origin's primary use (maybe docs should be updated), but that would avoid completely refactoring the current implementation!

AlexWaygood commented Nov 26, 2023
edited
Loading

Copy link
Copy Markdown
Member

Doesn't seem to be get_origin's primary use

It's already sorta awkwardly overloaded in what it means tbh — the __origin__ attribute of ParamSpecArgs instances doesn't really have the same semantics as the __origin__ attribute of generic aliases :)

Zac-HD commented Nov 26, 2023

Copy link
Copy Markdown
Contributor

A class seems somewhat more elegant to me, but yeah, that would work.

Copy link
Copy Markdown
Member Author

I'm OK with making it a class. It's not what the object is meant for, but it's a reasonable extension.

Using __origin__ feels hacky and wouldn't provide an easy way to get the deprecation message out.

Comment thread Lib/warnings.py Outdated
Comment thread Lib/warnings.py
Comment thread Doc/whatsnew/3.13.rst Outdated
Comment on lines +338 to +340
* 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Despite static typing being the original motivation for the feature, I would actually put the runtime effect first here:

Suggested change
* 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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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!

Comment thread Doc/library/warnings.rst Outdated
Comment thread Doc/library/warnings.rst Outdated

Copy link
Copy Markdown
Member Author

Thanks @AlexWaygood for the review! I pushed some changes.

Comment thread Doc/library/warnings.rst
Comment thread Lib/test/test_warnings/__init__.py Outdated
Comment thread Lib/test/test_warnings/__init__.py Outdated
Comment thread Lib/warnings.py Outdated

AlexWaygood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Comment thread Lib/warnings.py Outdated
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
JelleZijlstra merged commit d4a6229 into python:main Nov 29, 2023
JelleZijlstra deleted the pep702 branch November 29, 2023 17:38
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants


Back | FazBrowse Home | New Git URL