bpo-45166: fixes `get_type_hints` failure on `Final` #28279

sobolevn · 2021-09-10T18:36:42Z

https://bugs.python.org/issue45166

sobolevn · 2021-09-23T08:26:21Z

@ambv @Fidget-Spinner any chance to get a review please? 🙂

Fidget-Spinner

Sorry about the delay and thanks for your fix! This looks mostly good to me. I just have a few concerns.

Lib/typing.py

Misc/NEWS.d/next/Library/2021-09-10-21-35-53.bpo-45166.UHipXF.rst

bedevere-bot · 2021-09-23T13:16:07Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

sobolevn · 2021-09-23T20:59:43Z

I have made the requested changes; please review again

bedevere-bot · 2021-09-23T20:59:45Z

Thanks for making the requested changes!

@Fidget-Spinner: please review the changes made to this pull request.

ambv

Very good insight about the need for is_class=. I have one change here that we need to make (the __init__ signature) and two suggestions which I think make things clearer.

Lib/typing.py

Co-authored-by: Łukasz Langa <[email protected]>

Review changes implemented.

Fidget-Spinner

Thanks for your changes. I only have one more nit about adding a docstring :).

Lib/test/ann_module6.py

Fidget-Spinner · 2021-09-24T13:06:28Z

Lib/test/ann_module6.py

@@ -0,0 +1,7 @@
+# Tests that top-level ClassVar is not allowed
+
+from __future__ import annotations


Interestingly, this test fails when you remove this import. So it still allows top-level ClassVar if they aren't strings.

If you'd like to forbid this, you probably need to create another issue/and or PR. This behavior is what typing currently does (since 3.7-3.10). Even if the behavior is wrong, we don't usually backport bugfixes that add extra restrictions (unless it's a security fix) because that usually breaks backwards compatibility.

Yes, good catch! I've opened a new issue: https://bugs.python.org/issue45283
I will work on it after this one will be finished.

Thanks! 👍

Fidget-Spinner · 2021-09-24T13:08:21Z

Lib/typing.py

+    if not is_class:
+        invalid_generic_forms += (ClassVar,)
+        if is_argument:
+            invalid_generic_forms += (Final,)


Note to self: there's a subtle change in the logic compared to the old code, but it's ok because is_argument defaulted to True in both ForwardRef and _type_check. Which means by default we should always add ClassVar too.

Lib/test/ann_module5.py

Fidget-Spinner

LGTM. Thanks for your patience!

sobolevn · 2021-09-24T13:50:21Z

This test_asyncio is failing sometimes on Windows, reruning the CI.

miss-islington · 2021-09-25T08:56:25Z

Thanks @sobolevn for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

miss-islington · 2021-09-25T08:56:33Z

Sorry, @sobolevn and @ambv, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 784905dbeff68cf788bbeefe0a675af1af04affc 3.9

bedevere-bot · 2021-09-25T08:56:34Z

GH-28560 is a backport of this pull request to the 3.10 branch.

Co-authored-by: Łukasz Langa <[email protected]> Co-authored-by: Ken Jin <[email protected]> (cherry picked from commit 784905d) Co-authored-by: Nikita Sobolev <[email protected]>

…28279) Co-authored-by: Łukasz Langa <[email protected]> Co-authored-by: Ken Jin <[email protected]>. (cherry picked from commit 784905d) Co-authored-by: Nikita Sobolev <[email protected]>

bedevere-bot · 2021-09-25T08:58:31Z

GH-28561 is a backport of this pull request to the 3.9 branch.

sobolevn · 2021-09-25T09:01:57Z

Thanks everyone! 🎉

ambv · 2021-09-25T09:06:20Z

Don't mention it, thank you for the fix!

…8560) Co-authored-by: Łukasz Langa <[email protected]> Co-authored-by: Ken Jin <[email protected]> (cherry picked from commit 784905d) Co-authored-by: Nikita Sobolev <[email protected]>

…GH-28561) Co-authored-by: Łukasz Langa <[email protected]> Co-authored-by: Ken Jin <[email protected]>. (cherry picked from commit 784905d) Co-authored-by: Nikita Sobolev <[email protected]>

pawamoy · 2021-09-26T09:22:56Z

Just a question: will this not be backported to Python 3.8 as well?

bpo-45166: fixes get_type_hints failure on Final

1d46e26

sobolevn requested review from Fidget-Spinner and gvanrossum as code owners September 10, 2021 18:36

the-knights-who-say-ni added the CLA signed label Sep 10, 2021

bedevere-bot added the awaiting review label Sep 10, 2021

sobolevn mentioned this pull request Sep 10, 2021

get_type_hints + Final + future annotations = TypeError python/typing#890

Closed

gvanrossum removed their request for review September 10, 2021 19:14

ambv added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Sep 13, 2021

pawamoy mentioned this pull request Sep 21, 2021

fix: Support future annotations mkdocstrings/pytkdocs#113

Closed

Fidget-Spinner previously requested changes Sep 23, 2021

View reviewed changes

Lib/typing.py Outdated Show resolved Hide resolved

Misc/NEWS.d/next/Library/2021-09-10-21-35-53.bpo-45166.UHipXF.rst Outdated Show resolved Hide resolved

bedevere-bot added awaiting changes and removed awaiting review labels Sep 23, 2021

Adds is_class prototype to ForwardRef in typing

e121641

bedevere-bot removed the awaiting changes label Sep 23, 2021

bedevere-bot added the awaiting change review label Sep 23, 2021

bedevere-bot requested a review from Fidget-Spinner September 23, 2021 20:59

ambv approved these changes Sep 24, 2021

View reviewed changes

Lib/typing.py Outdated Show resolved Hide resolved

Lib/typing.py Show resolved Hide resolved

Lib/typing.py Outdated Show resolved Hide resolved

bedevere-bot added awaiting merge and removed awaiting change review labels Sep 24, 2021

ambv reviewed Sep 24, 2021

View reviewed changes

Lib/typing.py Outdated Show resolved Hide resolved

sobolevn and others added 4 commits September 24, 2021 14:28

Update Lib/typing.py

9752a76

Co-authored-by: Łukasz Langa <[email protected]>

Update Lib/typing.py

ff0deb5

Co-authored-by: Łukasz Langa <[email protected]>

Update Lib/typing.py

7a34336

Co-authored-by: Łukasz Langa <[email protected]>

Update Lib/typing.py

3e033a8

Co-authored-by: Łukasz Langa <[email protected]>

Fidget-Spinner reviewed Sep 24, 2021

View reviewed changes

Update news entry

b66c8f6

Fidget-Spinner reviewed Sep 24, 2021

View reviewed changes

Lib/test/ann_module5.py Show resolved Hide resolved

Add comment/docstring to Lib/test/ann_module5.py

4f9e4c6

Fidget-Spinner approved these changes Sep 24, 2021

View reviewed changes

sobolevn closed this Sep 24, 2021

sobolevn reopened this Sep 24, 2021

ambv merged commit 784905d into python:main Sep 25, 2021

bedevere-bot removed the awaiting merge label Sep 25, 2021

bedevere-bot removed the needs backport to 3.10 only security fixes label Sep 25, 2021

miss-islington assigned ambv Sep 25, 2021

bedevere-bot removed the needs backport to 3.9 only security fixes label Sep 25, 2021

sobolevn mentioned this pull request Sep 25, 2021

bpo-45283: Run _type_check on get_type_hints() #28563

Closed

uriyyo mentioned this pull request Nov 18, 2021

Fix issue with ClassVar parsing pydantic/pydantic#3403

Merged

5 tasks

		@@ -0,0 +1,7 @@
		# Tests that top-level ClassVar is not allowed

		from __future__ import annotations

Uh oh!

bpo-45166: fixes get_type_hints failure on Final #28279

bpo-45166: fixes get_type_hints failure on Final #28279

Uh oh!

Conversation

sobolevn commented Sep 10, 2021 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sobolevn commented Sep 23, 2021

Uh oh!

Fidget-Spinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Sep 23, 2021

Uh oh!

sobolevn commented Sep 23, 2021

Uh oh!

bedevere-bot commented Sep 23, 2021

Uh oh!

ambv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Fidget-Spinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Fidget-Spinner Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

sobolevn Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

Fidget-Spinner Sep 24, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Fidget-Spinner left a comment

Choose a reason for hiding this comment

Uh oh!

sobolevn commented Sep 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

miss-islington commented Sep 25, 2021

Uh oh!

miss-islington commented Sep 25, 2021

Uh oh!

bedevere-bot commented Sep 25, 2021

Uh oh!

bedevere-bot commented Sep 25, 2021

Uh oh!

sobolevn commented Sep 25, 2021

Uh oh!

ambv commented Sep 25, 2021

Uh oh!

pawamoy commented Sep 26, 2021

Uh oh!

Uh oh!

bpo-45166: fixes `get_type_hints` failure on `Final` #28279

bpo-45166: fixes `get_type_hints` failure on `Final` #28279

sobolevn commented Sep 10, 2021 •

edited by bedevere-bot

Loading

sobolevn commented Sep 24, 2021 •

edited

Loading