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 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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.

Excuse me I have argued wrongly. You are right that None would be from __annotations__. In such a case value should be NoneType and _could_be_inserted_optional(NoneType) would have already skipped this iteration (get_origin(NoneType) is not Union).

That's why original_value would still never be None here; 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