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

Fix Union[..., NoneType] injection by get_type_hints if a None default value is used. #482

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
376ae56
Fix get_type_hints with None default interaction
Daraan Oct 9, 2024
24c5602
moved section
Daraan Oct 9, 2024
7f000b1
removed empty line
Daraan Oct 9, 2024
8cc4464
Restructured tests and applied suggested changes
Daraan Oct 9, 2024
881ffee
+1 test and formatting
Daraan Oct 9, 2024
58082b9
Increase test coverage
Daraan Oct 10, 2024
5c94bd6
use key access and not values
Daraan Oct 10, 2024
b42e203
str test passes as well
Daraan Oct 10, 2024
ed552e4
removed invalid 3.8 code
Daraan Oct 10, 2024
313ddd8
Extended tests
Daraan Oct 10, 2024
3ba4ee7
refinement of tests, more tests with aliases
Daraan Oct 11, 2024
6ffcd23
Merge 'main' into get_type_hints
Daraan Oct 11, 2024
403e7ce
Merge branch 'main' into get_type_hints
Daraan Oct 15, 2024
52c93e9
Add comment for special case
Daraan Oct 21, 2024
a1777f3
Assure UnionType stays UnionType; check repr only
Daraan Oct 21, 2024
9e59796
Update src/test_typing_extensions.py
Daraan Oct 21, 2024
1761a43
Merge remote-tracking branch 'upstream/main' into get_type_hints
Daraan Oct 21, 2024
54b8eb0
Corrected and clarified case
Daraan Oct 21, 2024
5612f6f
Merge remote-tracking branch 'upstream/main' into get_type_hints
Daraan Oct 22, 2024
91075ff
Merge branch 'main' into get_type_hints
Daraan Oct 29, 2024
1eac721
Merge branch 'main' into get_type_hints
Daraan Nov 25, 2024
1f84c68
Merge branch 'main' into get_type_hints
Daraan Nov 27, 2024
214dfef
Merge branch 'main' into get_type_hints
Daraan Dec 12, 2024
00dedc2
Merge branch 'main' into get_type_hints
Daraan Jan 6, 2025
e929085
Merge branch 'main' into get_type_hints
Daraan Jan 8, 2025
47e47ad
Merge branch 'main' into get_type_hints
Daraan Jan 17, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/test_typing_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,7 @@ def test_annotation_and_optional_default(self):
Union["annotation", T_default] : Union[annotation, T_default],
Annotated["annotation", "nested"] : Annotated[Union[int, None], "data", "nested"],
}
if TYPING_3_10_0: # cannot construct UnionTypes
if TYPING_3_10_0: # cannot construct UnionTypes before 3.10
do_not_stringify_cases["str | NoneAlias | StrAlias"] = str | None
cases[str | None] = Optional[str]
cases.update(do_not_stringify_cases)
Expand All @@ -1690,7 +1690,7 @@ def test_annotation_and_optional_default(self):
skip_reason = None
annot_unchanged = annot
if sys.version_info[:2] == (3, 10) and annot == "str | NoneAlias | StrAlias" and none_default:
# different repr here as Optional[str | None] -> Optional[str] not a UnionType
# In 3.10 converts Optional[str | None] to Optional[str] which has a different repr
skip_reason = "UnionType not preserved in 3.10"
if wrap_optional:
if annot_unchanged == ():
Expand Down Expand Up @@ -1726,11 +1726,13 @@ def func(x: annot): pass
# Hash
for k in type_hints.keys():
self.assertEqual(hash(type_hints[k]), hash(expected[k]))
# Test if UnionTypes are preserved
self.assertEqual(isinstance(type_hints[k], type(expected[k])), True)
Daraan marked this conversation as resolved.
Show resolved Hide resolved
# Repr
with self.subTest("Check str and repr"):
if skip_reason == "UnionType not preserved in 3.10":
self.skipTest(skip_reason)
self.assertEqual(str(type_hints) + repr(type_hints), str(expected) + repr(expected))
self.assertEqual(repr(type_hints), repr(expected))


class GetUtilitiesTestCase(TestCase):
Expand Down
12 changes: 9 additions & 3 deletions src/typing_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1282,7 +1282,7 @@ def _clean_optional(obj, hints, globalns=None, localns=None):
):
continue
original_value = original_hints[name]
if original_value is None: # should not happen
if original_value is None: # should be NoneType already; check just in case
Copy link
Member

Choose a reason for hiding this comment

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

It shouldn't be NoneType already, since we get this directly from __annotations__.

Copy link
Contributor Author

@Daraan Daraan Oct 21, 2024

Choose a reason for hiding this comment

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

(Edited for more clarity)
Excuse me I have argued wrongly. You are right that None would be from __annotations__. In such a case value should be NoneType. However, in that case _could_be_inserted_optional(NoneType) will skip this iteration (as get_origin(NoneType) is not Union).

That's why code with a original_value that is None should never propagate until this line, but keeping these two lines to keep it safe; updated the comment.

original_value = _NoneType
# Forward reference
if isinstance(original_value, str):
Expand Down Expand Up @@ -1310,8 +1310,14 @@ def _clean_optional(obj, hints, globalns=None, localns=None):
if sys.version_info < (3, 9) and get_origin(original_evaluated) is Union:
# Union[str, None, "str"] is not reduced to Union[str, None]
original_evaluated = Union[original_evaluated.__args__]
# Compare if values differ
if original_evaluated != value:
# Compare if values differ. Note that even if equal
# value might be cached by typing._tp_cache contrary to original_evaluated
if original_evaluated != value or (
# 3.10: ForwardRefs of UnionType might be turned into _UnionGenericAlias
hasattr(_types, "UnionType")
and isinstance(original_evaluated, _types.UnionType)
and not isinstance(value, _types.UnionType)
):
hints[name] = original_evaluated

# Python 3.9+ has PEP 593 (Annotated)
Expand Down
Loading