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 20 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
subscripted objects) had wrong parameters if they were directly
subscripted with an `Unpack` object.
Patch by [Daraan](https://github.com/Daraan).
- Fix backport of `get_type_hints` to reflect Python 3.11+ behavior which does not add
`Union[..., NoneType]` to annotations that have a `None` default value anymore.
This fixes wrapping of `Annotated` in an unwanted `Optional` in such cases.
Patch by [Daraan](https://github.com/Daraan).

# Release 4.12.2 (June 7, 2024)

Expand Down
87 changes: 87 additions & 0 deletions src/test_typing_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1645,6 +1645,93 @@ def test_final_forward_ref(self):
self.assertNotEqual(gth(Loop, globals())['attr'], Final[int])
self.assertNotEqual(gth(Loop, globals())['attr'], Final)

def test_annotation_and_optional_default(self):
Daraan marked this conversation as resolved.
Show resolved Hide resolved
annotation = Annotated[Union[int, None], "data"]
NoneAlias = None
StrAlias = str
T_default = TypeVar("T_default", default=None)
Ts = TypeVarTuple("Ts")

cases = {
# annotation: expected_type_hints
Annotated[None, "none"] : Annotated[None, "none"],
annotation : annotation,
Optional[int] : Optional[int],
Optional[List[str]] : Optional[List[str]],
Optional[annotation] : Optional[annotation],
Union[str, None, str] : Optional[str],
Unpack[Tuple[int, None]]: Unpack[Tuple[int, None]],
# Note: A starred *Ts will use typing.Unpack in 3.11+ see Issue #485
Unpack[Ts] : Unpack[Ts],
}
# contains a ForwardRef, TypeVar(~prefix) or no expression
do_not_stringify_cases = {
() : {},
Daraan marked this conversation as resolved.
Show resolved Hide resolved
int : int,
"int" : int,
None : type(None),
"NoneAlias" : type(None),
List["str"] : List[str],
Union[str, "str"] : str,
Union[str, None, "str"] : Optional[str],
Union[str, "NoneAlias", "StrAlias"]: Optional[str],
Union[str, "Union[None, StrAlias]"]: Optional[str],
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
do_not_stringify_cases["str | NoneAlias | StrAlias"] = str | None
cases[str | None] = Optional[str]
cases.update(do_not_stringify_cases)
for (annot, expected), none_default, as_str, wrap_optional in itertools.product(
cases.items(), (False, True), (False, True), (False, True)
):
# Special case:
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
skip_reason = "UnionType not preserved in 3.10"
if wrap_optional:
if annot_unchanged == ():
continue
annot = Optional[annot]
expected = {"x": Optional[expected]}
else:
expected = {"x": expected} if annot_unchanged != () else {}
if as_str:
if annot_unchanged in do_not_stringify_cases or annot_unchanged == ():
continue
annot = str(annot)
with self.subTest(
annotation=annot,
as_str=as_str,
wrap_optional=wrap_optional,
none_default=none_default,
expected_type_hints=expected,
):
# Create function to check
if annot_unchanged == ():
if none_default:
def func(x=None): pass
else:
def func(x): pass
elif none_default:
def func(x: annot = None): pass
else:
def func(x: annot): pass
type_hints = get_type_hints(func, globals(), locals(), include_extras=True)
# Equality
self.assertEqual(type_hints, expected)
# Hash
for k in type_hints.keys():
self.assertEqual(hash(type_hints[k]), hash(expected[k]))
# 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))
Copy link
Member

Choose a reason for hiding this comment

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

Concatenating these seems a bit odd. In any case, type_hints is a dictionary, so the str() and repr() are the same.

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.

Thanks for pointing this out I was not 100% sure here and kept a lazy version. Changed to only repr.



class GetUtilitiesTestCase(TestCase):
def test_get_origin(self):
Expand Down
73 changes: 73 additions & 0 deletions src/typing_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1236,10 +1236,83 @@ def get_type_hints(obj, globalns=None, localns=None, include_extras=False):
)
else: # 3.8
hint = typing.get_type_hints(obj, globalns=globalns, localns=localns)
if sys.version_info < (3, 11):
_clean_optional(obj, hint, globalns, localns)
if sys.version_info < (3, 9):
# In 3.8 eval_type does not flatten Optional[ForwardRef] correctly
# This will recreate and and cache Unions.
hint = {
k: (t
if get_origin(t) != Union
else Union[t.__args__])
for k, t in hint.items()
}
if include_extras:
return hint
return {k: _strip_extras(t) for k, t in hint.items()}

_NoneType = type(None)

def _could_be_inserted_optional(t):
"""detects Union[..., None] pattern"""
JelleZijlstra marked this conversation as resolved.
Show resolved Hide resolved
# 3.8+ compatible checking before _UnionGenericAlias
if get_origin(t) is not Union:
return False
# Assume if last argument is not None they are user defined
if t.__args__[-1] is not _NoneType:
return False
return True

# < 3.11
def _clean_optional(obj, hints, globalns=None, localns=None):
# reverts injected Union[..., None] cases from typing.get_type_hints
# when a None default value is used.
# see https://github.com/python/typing_extensions/issues/310
if not hints or isinstance(obj, type):
return
defaults = typing._get_defaults(obj) # avoid accessing __annotations___
if not defaults:
return
original_hints = obj.__annotations__
for name, value in hints.items():
# Not a Union[..., None] or replacement conditions not fullfilled
if (not _could_be_inserted_optional(value)
or name not in defaults
or defaults[name] is not None
):
continue
original_value = original_hints[name]
if original_value is None: # should not happen
Copy link
Member

Choose a reason for hiding this comment

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

Why shouldn't this happen? You can put None in annotations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this point a None value should already be converted to NoneType in original_hints. I think these two lines are redundant but to be safe I added the double check.

Added a comment to clarify this.

original_value = _NoneType
# Forward reference
if isinstance(original_value, str):
if globalns is None:
if isinstance(obj, _types.ModuleType):
globalns = obj.__dict__
else:
nsobj = obj
# Find globalns for the unwrapped object.
while hasattr(nsobj, '__wrapped__'):
nsobj = nsobj.__wrapped__
globalns = getattr(nsobj, '__globals__', {})
if localns is None:
localns = globalns
elif localns is None:
localns = globalns
if sys.version_info < (3, 9):
original_value = ForwardRef(original_value)
else:
original_value = ForwardRef(
original_value,
is_argument=not isinstance(obj, _types.ModuleType)
)
original_evaluated = typing._eval_type(original_value, globalns, localns)
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:
hints[name] = original_evaluated
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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:
hints[name] = original_evaluated
hints[name] = original_evaluated

Would this also work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it would. However there are minor differences involving identities and caching.

typing._eval_type uses _GenericAlias.copy_with or in some cases creates a new GenericAlias which do not make use of the caches.

If a ForwardRef is involved original_evaluated can be a new object as _tp_cache is not queried. Not changing hints["x"] will return an object that matches expected["x"] by identity in more cases.

That's a minor feature and I am fine with dropping it if you want; for now added a comment to clarify this.


# Python 3.9+ has PEP 593 (Annotated)
if hasattr(typing, 'Annotated'):
Expand Down
Loading