bpo-46195: Do not add `Optional` in `get_type_hints` by sobolevn · Pull Request #30304 · python/cpython

sobolevn · 2021-12-30T18:40:24Z

Useful related links:

PEP 484: Do not require type checkers to treat a None default specially peps#689
Should we change PEP 484 to disable implicit Optional when default = None? typing#275

https://bugs.python.org/issue46195

merwok · 2022-01-02T18:39:20Z

A note about commit messages / PR titles: something does something is ambiguous because it could be describing wrong behaviour that the code did before the change, or new correct behaviour. Similar to the docstring convention (method columnize: Display a list of blah by blah), a good commit message or PR title should show what the changes do:

Fix extra space added by columnize (better than: columnize adds extra space (a bug))
Add extra space in columnize (better than: columnize adds extra space (a feature))
columnize now adds extra space when needed (same — present tense + now avoids ambiguity)

So for this PR (if I understand current title correctly) it could be:

Do not add Optional in get_type_hints
Make get_type_hints not add Optional

sobolevn · 2022-01-02T18:47:54Z

@merwok thanks, it is way more readable now! Wording (and writing in general) in English is something I really try to improve.

sobolevn · 2022-01-09T08:16:15Z

@Zac-HD do we rely on this behavior in hypothesis? 🤔

Zac-HD · 2022-01-09T09:32:00Z

@Zac-HD do we rely on this behavior in hypothesis? 🤔

It's certainly observable by our users, and would probably make some of our tests fail.

That said, I support the more consistent design proposed in the BPO issue, and we'll be fine carrying another clause in our type hints wrapper in Hypothesis. Probably important to reference the relevant PEP (section of 484, I think?) in the changelog though.

pbryan · 2022-01-09T16:21:46Z

Will this be applied in Python 3.11, or is it intended to be back-ported to previous versions?

sobolevn · 2022-01-09T16:39:47Z

In my opinion this should be 3.11+ only 🤔
But, there might be other opinions of course.

pbryan · 2022-01-09T17:01:56Z

I have production code that currently assumes parameters that default to None are implicitly Optional; the required changes to accommodate this are trivial on my part, and I can certainly plan within the 3.11 time horizon.

bedevere-bot · 2022-01-09T22:26:20Z

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.

Fidget-Spinner

In my opinion this should be 3.11+ only 🤔

Yeah. This is definitely a breaking change so it should be 3.11+ only. I wonder how many runtime checkers are affected or even use get_type_hints for that matter? @samuelcolvin can I trouble you for an opinion on this please? @Tinche too for cattrs too please. Thank you.

In short, typing.get_type_hints automatically wraps a type annotation with Optional if it sees a default None. This PR proposes to cease that behavior.

Tinche · 2022-01-11T14:50:39Z

In my opinion this should be 3.11+ only 🤔
Yeah. This is definitely a breaking change so it should be 3.11+ only. I wonder how many runtime checkers are affected or even use get_type_hints for that matter? @samuelcolvin can I trouble you for an opinion on this please? @Tinche too for cattrs too please. Thank you.
In short, typing.get_type_hints automatically wraps a type annotation with Optional if it sees a default None. This PR proposes to cease that behavior.

@Fidget-Spinner I'm totally fine with this, correct change in my opinion.

Just out of curiosity, does this change only apply to function parameters or class members too?

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>

sobolevn · 2022-01-11T15:01:48Z

@Tinche for classes Optional was never added implictly (which is another point why we should fix this inconsistency).

Here are two examples, main and 3.9:

» ./python.exePython3.11.0a3+ (heads/main:e13cdca0f5, Jan112022, 12:43:49) [Clang11.0.0 (clang-1100.0.33.16)] ondarwinType"help", "copyright", "credits"or"license"formoreinformation.
>>>importtyping>>>classA:
... x: int=None
... >>>typing.get_type_hints(A)
{'x':<class'int'>}

» pythonPython3.9.9 (main, Dec212021, 11:35:28) [Clang11.0.0 (clang-1100.0.33.16)] ondarwinType"help", "copyright", "credits"or"license"formoreinformation.
>>>importtyping>>>classA:
... x: int=None
... >>>typing.get_type_hints(A)
{'x':<class'int'>}

However, I can't find a test case for this 🤔

Tinche · 2022-01-11T15:17:02Z

@Tinche for classes Optional was never added implictly (which is another point why we should fix this inconsistency).

Here are two examples, main and 3.9:

» ./python.exePython3.11.0a3+ (heads/main:e13cdca0f5, Jan112022, 12:43:49) [Clang11.0.0 (clang-1100.0.33.16)] ondarwinType"help", "copyright", "credits"or"license"formoreinformation.
>>>importtyping>>>classA:
... x: int=None
... >>>typing.get_type_hints(A)
{'x':<class'int'>}

» pythonPython3.9.9 (main, Dec212021, 11:35:28) [Clang11.0.0 (clang-1100.0.33.16)] ondarwinType"help", "copyright", "credits"or"license"formoreinformation.
>>>importtyping>>>classA:
... x: int=None
... >>>typing.get_type_hints(A)
{'x':<class'int'>}

However, I can't find a test case for this 🤔

Thanks for the info!

sobolevn · 2022-01-11T15:18:47Z

Thanks for the great question! 👍

I've added this corner case to our tests in a separate PR: #30535

JelleZijlstra

I like this change, just a comment on the docs.

JelleZijlstra · 2022-01-22T03:37:20Z

+ ``Optional`` annotation is not added implicitly anymore
+ when ``None`` default is used for a function argument.


I like the wording in the removed docs better. Suggestion:
"Previously, Optional[t] was added for function and method annotations if a default value equal to None was set. Now the annotation is returned unchanged."

Great addition!

samuelcolvin · 2022-01-26T12:55:21Z

In my opinion this should be 3.11+ only thinking
Yeah. This is definitely a breaking change so it should be 3.11+ only. I wonder how many runtime checkers are affected or even use get_type_hints for that matter? @samuelcolvin can I trouble you for an opinion on this please? @Tinche too for cattrs too please. Thank you.
In short, typing.get_type_hints automatically wraps a type annotation with Optional if it sees a default None. This PR proposes to cease that behavior.

Thanks @Fidget-Spinner for asking my opinion, really useful to get a heads up about things like this. I don't think it will affect pydantic, if it does we can take care of it when adding support for 3.11.

I'm happy with this change provided it's no back-ported to other versions of python.

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

JelleZijlstra

@gvanrossum this is another one I'd like to merge.

@gpshead previously requested changes, but his request was addressed.

gvanrossum

IIUC, @gpshead mist still approve it.

Since python-3.11, `typing.get_type_hints()` will not add Optional[t] to type annotations even if a default value for function argument is None. refs: python/cpython#30304 (bpo-46195)

Zac-HD · 2022-03-14T01:52:17Z

Just re-found this issue because Hypothesis does indeed have a test for this 😂

Let's mention this change in https://docs.python.org/3.11/whatsnew/3.11.html, not just the docs for typing.get_type_hints().

bpo-46195: get_type_hints does not add Optional anymore
dd3a750

sobolevn requested review from Fidget-Spinner and gvanrossum as code owners December 30, 2021 18:40

the-knights-who-say-ni added the CLA signed label Dec 30, 2021

bedevere-bot added the awaiting review label Dec 30, 2021

sobolevn changed the title ~~bpo-46195: get_type_hints does not add Optional anymore~~bpo-46195: Do not add Optional in get_type_hintsJan 2, 2022

sobolevn added the type-feature A feature request or enhancement label Jan 9, 2022

gpshead requested changes Jan 9, 2022
View reviewed changes

Comment threadDoc/library/typing.rst

bedevere-bot removed the awaiting review label Jan 9, 2022

bedevere-bot added the awaiting changes label Jan 9, 2022

sobolevn added 2 commits January 10, 2022 12:16

Add .. versionchanged information
1cbe416

Update typing.rst
b87e785

Fidget-Spinner reviewed Jan 11, 2022
View reviewed changes

Comment threadMisc/NEWS.d/next/Library/2021-12-30-21-38-51.bpo-46195.jFKGq_.rst Outdated

Update Misc/NEWS.d/next/Library/2021-12-30-21-38-51.bpo-46195.jFKGq_.rst
e4f5fee
Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>

sobolevn mentioned this pull request Jan 11, 2022
bpo-46345: add a test case for implicit Optional class attribute #30535
Merged

sobolevn closed this Jan 12, 2022

sobolevn reopened this Jan 12, 2022

JelleZijlstra reviewed Jan 22, 2022
View reviewed changes

Change docs
396ddcf

sobolevn closed this Jan 22, 2022

sobolevn reopened this Jan 22, 2022

sobolevn closed this Jan 26, 2022

sobolevn reopened this Jan 26, 2022

sobolevn requested a review from Fidget-Spinner January 26, 2022 09:50

sobolevn closed this Feb 3, 2022

sobolevn reopened this Feb 3, 2022

JelleZijlstra reviewed Feb 17, 2022
View reviewed changes

Comment threadMisc/NEWS.d/next/Library/2021-12-30-21-38-51.bpo-46195.jFKGq_.rst Outdated

JelleZijlstra self-assigned this Feb 17, 2022

Update Misc/NEWS.d/next/Library/2021-12-30-21-38-51.bpo-46195.jFKGq_.rst
153dd09
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>

JelleZijlstra approved these changes Feb 19, 2022
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting changes labels Feb 19, 2022

gvanrossum approved these changes Feb 19, 2022
View reviewed changes

gpshead approved these changes Mar 2, 2022
View reviewed changes

JelleZijlstra merged commit 20a1c8e into python:mainMar 2, 2022

bedevere-bot removed the awaiting merge label Mar 2, 2022

tk0miya mentioned this pull request Mar 2, 2022
test: Follow the new behavior of typing.get_type_hints() since py311 sphinx-doc/sphinx#10232
Merged

JelleZijlstra mentioned this pull request Apr 13, 2022
Add typing to 3.11 What's New #91491
Closed

AlexWaygood mentioned this pull request Apr 14, 2022
Don't return implicit optional types by get_type_hints #77314
Closed

		``Optional`` annotation is not added implicitly anymore
		when ``None`` default is used for a function argument.

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Uh oh!

merwok commented Jan 2, 2022

Uh oh!

sobolevn commented Jan 2, 2022

Uh oh!

sobolevn commented Jan 9, 2022

Uh oh!

Zac-HD commented Jan 9, 2022

Uh oh!

pbryan commented Jan 9, 2022

Uh oh!

sobolevn commented Jan 9, 2022

Uh oh!

pbryan commented Jan 9, 2022

Uh oh!

Uh oh!

bedevere-bot commented Jan 9, 2022

Uh oh!

Fidget-Spinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Tinche commented Jan 11, 2022

Uh oh!

sobolevn commented Jan 11, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tinche commented Jan 11, 2022

Uh oh!

sobolevn commented Jan 11, 2022

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

JelleZijlstraJan 22, 2022

Choose a reason for hiding this comment

Uh oh!

sobolevnJan 22, 2022

Choose a reason for hiding this comment

Uh oh!

samuelcolvin commented Jan 26, 2022

Uh oh!

Uh oh!

JelleZijlstra left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

Zac-HD commented Mar 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants

sobolevn commented Dec 30, 2021•
edited by bedevere-bot
Loading

sobolevn commented Jan 11, 2022•
edited
Loading