Skip to content
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-101561: Add typing.override decorator #101564

Merged
merged 10 commits into from
Feb 27, 2023

Conversation

stroxler
Copy link
Contributor

@stroxler stroxler commented Feb 4, 2023

The code is pulled almost unchanged from typing_extensions:

Question that came up was where to put the type variable and what to name it:

  • In typing_extensions the type variables use underscore prefixes to indicate that they are not public, but typing doesn't appear to use that convention so I named the type variable F.
  • I couldn't put the type variable next to all of the other ones (used in generic container definitions) because it relies on Callable which is defined below. If this is a problem I could correct it using a forward reference, or we could try to rearrange the module to put special forms above type variables.

Testing the code:

First set up the repo, following instructions
at https://devguide.python.org/ by running:

./configure --with-pydebug && make -j

Then run the typing tests:

./python -m test test_typing -v

I ran the full test suite with

./python -m test -j3

and it came back clean except for a test_grp failure which I seem to get on trunk as well - likely something in my build is misconfigured but I'm pretty sure it is unrelated to the changes here.

Make sure the docs build:

cd Doc
make venv
make html

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@stroxler stroxler changed the title PEP 698: Add typing.override decorator gh-101561: Add typing.override decorator Feb 4, 2023
@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks! Haven't looked in depth yet at all, but we'll also need:

  • A news entry, as @bedevere-bot says
  • An entry in Doc/whatsnew/3.12.rst
  • (optionally) you can add your name to Misc/ACKS

@arhadthedev arhadthedev added stdlib Python modules in the Lib dir topic-typing labels Feb 4, 2023
@AlexWaygood AlexWaygood added type-feature A feature request or enhancement 3.12 bugs and security fixes labels Feb 4, 2023
Lib/typing.py Outdated Show resolved Hide resolved
@Fidget-Spinner
Copy link
Member

The implementation for override LGTM.

facebook-github-bot referenced this pull request in facebook/pyre-check Feb 9, 2023
Summary:
THe `typing_extensions.override` was added a few months ago,
we should support it.

The `typing.override` decorator isn't landed yet, but that's okay - we
should support it in override checks; it will still raise a "no such name"
method when Pyre is being run against a typeshed that doens't have it yet.

The PR to add `typing.override` to CPython is out for review now:
https://github.com/python/cpython/pull/101564`

Reviewed By: grievejia

Differential Revision: D43055550

fbshipit-source-id: 3f51f643063afea75bf54ef38ae3a48681b99bd9
stroxler and others added 9 commits February 27, 2023 11:55
Testing:

First set up the repo, following instructions
at https://devguide.python.org/ by running:
```
./configure --with-pydebug && make -j
```

Then run the typing tests:
```
./python -m test test_typing -v
```

I ran the full test suite with
```
./python -m test -j3
```
and it came back clean except for a `test_grp` failure which I seem to
get on trunk as well - likely something in my build is misconfigured
but I'm pretty sure it is unrelated to the changes here.
Correct `s/final/override` (from reusing a similar block of documentation on the `@final` decorator)

Co-authored-by: Jelle Zijlstra <[email protected]>
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@stroxler
Copy link
Contributor Author

(Rebase on main)

@gvanrossum I think this is waiting on review from you

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Sorry for dropping this; I have a few nits but I'm happy to merge the PR. I'll leave it open for a few days in case Guido does want to take a look himself.

Doc/whatsnew/3.12.rst Outdated Show resolved Hide resolved
@gvanrossum
Copy link
Member

I defer to @JelleZijlstra if that's okay.

Thanks, looks good

Co-authored-by: Jelle Zijlstra <[email protected]>
@sobolevn
Copy link
Member

sobolevn commented Feb 28, 2023

Sorry for being late with a review, I've opened a follow-up for this: #102324

Thanks a lot for adding this decorator! Great work, it would be very useful! 👍

carljm added a commit to carljm/cpython that referenced this pull request Feb 28, 2023
* main: (67 commits)
  pythongh-99108: Add missing md5/sha1 defines to Modules/Setup (python#102308)
  pythongh-100227: Move _str_replace_inf to PyInterpreterState (pythongh-102333)
  pythongh-100227: Move the dtoa State to PyInterpreterState (pythongh-102331)
  pythonGH-102305: Expand some macros in generated_cases.c.h (python#102309)
  Migrate to new PSF mailgun account (python#102284)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives (in Python/) (python#102193)
  pythonGH-90744: Fix erroneous doc links in the sys module (python#101319)
  pythongh-87092: Make jump target label equal to the offset of the target in the instructions sequence (python#102093)
  pythongh-101101: Unstable C API tier (PEP 689) (pythonGH-101102)
  IDLE: Simplify DynOptionsMenu __init__code (python#101371)
  pythongh-101561: Add typing.override decorator (python#101564)
  pythongh-101825: Clarify that as_integer_ratio() output is always normalized (python#101843)
  pythongh-101773: Optimize creation of Fractions in private methods (python#101780)
  pythongh-102251: Updates to test_imp Toward Fixing Some Refleaks (pythongh-102254)
  pythongh-102296 Document that inspect.Parameter kinds support ordering (pythonGH-102297)
  pythongh-102250: Fix double-decref in COMPARE_AND_BRANCH error case (pythonGH-102287)
  pythongh-101100: Fix sphinx warnings in `types` module (python#102274)
  pythongh-91038: Change default argument value to `False` instead of `0` (python#31621)
  pythongh-101765: unicodeobject: use Py_XDECREF correctly (python#102283)
  [doc] Improve grammar/fix missing word (pythonGH-102060)
  ...
JelleZijlstra pushed a commit that referenced this pull request Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes stdlib Python modules in the Lib dir topic-typing type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants