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 5 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
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
31 changes: 31 additions & 0 deletions src/test_typing_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -4993,6 +4993,37 @@ def test_nested_annotated_with_unhashable_metadata(self):
self.assertEqual(X.__origin__, List[Annotated[str, {"unhashable_metadata"}]])
self.assertEqual(X.__metadata__, ("metadata",))

def test_get_type_hints(self):
annotation = Annotated[Union[int, None], "data"]
optional_annotation = Optional[annotation]

def wanted_optional(bar: optional_annotation): ...
def wanted_optional_default(bar: optional_annotation = None): ...
def wanted_optional_ref(bar: 'Optional[Annotated[Union[int, None], "data"]]'): ...

def no_optional(bar: annotation): ...
def no_optional_default(bar: annotation = None): ...
def no_optional_defaultT(bar: Union[annotation, T] = None): ...
def no_optional_defaultT_ref(bar: "Union[annotation, T]" = None): ...

for func in(wanted_optional, wanted_optional_default, wanted_optional_ref):
self.assertEqual(
get_type_hints(func, include_extras=True),
{"bar": optional_annotation}
)

for func in (no_optional, no_optional_default):
self.assertEqual(
get_type_hints(func, include_extras=True),
{"bar": annotation}
)

for func in (no_optional_defaultT, no_optional_defaultT_ref):
self.assertEqual(
get_type_hints(func, globals(), locals(), include_extras=True),
{"bar": Union[annotation, T]}
)


class GetTypeHintsTests(BaseTestCase):
def test_get_type_hints(self):
Expand Down
65 changes: 65 additions & 0 deletions src/typing_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1236,10 +1236,75 @@ 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) and hint:
hint = _clean_optional(obj, hint, globalns, localns)
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"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

_UnionGenericAlias does not exist in the whole version range. Is this sufficient to assure that it's a Union? I am not 100% sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to get_origin.

# 3.8+ compatible checking before _UnionGenericAlias
if not hasattr(t, "__origin__") or t.__origin__ 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
original_hints = getattr(obj, '__annotations__', None)
defaults = typing._get_defaults(obj)
Copy link
Member

Choose a reason for hiding this comment

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

Should exit early if there are no defaults. The current code risks accessing .__annotations__ directly on classes, which could have unwanted effects.

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:
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):
ref = ForwardRef(original_value)
else:
ref = ForwardRef(
original_value,
is_argument=not isinstance(obj, _types.ModuleType)
)
original_value = typing._eval_type(ref, globalns, localns)
# Values was not modified or original is already Optional
if original_value == value or _could_be_inserted_optional(original_value):
continue
# NoneType was added to value
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively hints[name] = original_value which should be equivalent. I wonder which would be the safer alternative.

Copy link
Member

Choose a reason for hiding this comment

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

Using original_value is incorrect as we may have modified the internals of the hint. For example, get_type_hints() turns List["int"] into List[int].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the feedback. Yes, it should have been piped trough _eval_type as well. Can you take a look again?

if len(value.__args__) == 2:
hints[name] = value.__args__[0] # not a Union
Daraan marked this conversation as resolved.
Show resolved Hide resolved
else:
hints[name] = Union[value.__args__[:-1]] # still a Union

return hints


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