Skip to content

bpo-46333: Honor module parameter in ForwardRef #30536

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Feb 17, 2022
Merged

bpo-46333: Honor module parameter in ForwardRef #30536

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
980ea28
bpo-46333: Honor `module` parameter in ForwardRef
andreas-h-sie Jan 11, 2022
82fa061
📜🤖 Added by blurb_it.
blurb-it[bot] Jan 11, 2022
ac51d98
Update Lib/test/test_typing.py
aha79 Jan 11, 2022
d643861
Update Lib/typing.py
aha79 Jan 11, 2022
ac16c5c
Update Misc/NEWS.d/next/Library/2022-01-11-15-54-15.bpo-46333.B1faiF.rst
aha79 Jan 11, 2022
a2e1377
Update ACKS
andreas-h-sie Jan 11, 2022
c084fdb
Update Misc/NEWS.d/next/Library/2022-01-11-15-54-15.bpo-46333.B1faiF.rst
aha79 Jan 12, 2022
7577633
Remove module output from __repr__
andreas-h-sie Feb 10, 2022
d608058
Merge branch 'bpo_46333_eq_and_repr_for_forwardref' of github.com:aha…
andreas-h-sie Feb 10, 2022
3df5009
take out repr change
andreas-h-sie Feb 10, 2022
42fc26b
Update Misc/NEWS.d/next/Library/2022-01-11-15-54-15.bpo-46333.B1faiF.rst
aha79 Feb 11, 2022
9dd2f8f
add tests for ForwardRef.hash
andreas-h-sie Feb 11, 2022
94c9a69
move ForwardRef.__hash__ test to correct function
andreas-h-sie Feb 11, 2022
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
6 changes: 6 additions & 0 deletions Lib/test/test_typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2611,6 +2611,10 @@ def test_forward_equality(self):
fr = typing.ForwardRef('int')
self.assertEqual(fr, typing.ForwardRef('int'))
self.assertNotEqual(List['int'], List[int])
self.assertNotEqual(fr, typing.ForwardRef('int', module=__name__))
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I wonder if this test is run directly, will __name__ be `main``? Wouldn't that make the test pass incorrectly? (Throwing this question out there because I'm not too sure myself). Maybe we should be safe and give it a distinct name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test is to ensure that ForwardRef with module set is different from ForwardRef with module not set (That was previsously not the case). The actual value of module (or __name__) is not relevant. Under the hood the comparison is None vs str

Copy link
Member

@Fidget-Spinner Fidget-Spinner Jan 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

My bad, thanks for pointing that out. I mistakenly assumed it would grab a default value from the current scope to populate module in things like ClassVar['int'] due to the _type_check/_type_convert function (I'm now shocked that it doesn't and only does that for TypedDict).

Copy link
Contributor Author

@aha79 aha79 Jan 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, unfortunately the current module is not captured automatically.
The current scope/module is typicallly obtained by get_type_hints because a class knows its module (and so all ForwardRefs in member annotations can be correctly resolved. But each forward Ref does originally not know its module).
However for type aliases this does not work, and there one needs the explicit module.

For example

Json = Union[ List['Json'], Dict[str,'Json'], int, float, bool, None ]

cannot be exported (and expect to work) unless one sets the module for each forward ref.

frm = typing.ForwardRef('int', module=__name__)
self.assertEqual(frm, typing.ForwardRef('int', module=__name__))
self.assertNotEqual(frm, typing.ForwardRef('int', module='__other_name__'))

def test_forward_equality_gth(self):
c1 = typing.ForwardRef('C')
Expand Down Expand Up @@ -2673,6 +2677,8 @@ def fun(x: a):

def test_forward_repr(self):
self.assertEqual(repr(List['int']), "typing.List[ForwardRef('int')]")
self.assertEqual(repr(List[ForwardRef('int', module='mod')]),
"typing.List[ForwardRef('int', module='mod')]")

def test_union_forward(self):

Expand Down
11 changes: 8 additions & 3 deletions Lib/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,10 +713,11 @@ def __eq__(self, other):
if self.__forward_evaluated__ and other.__forward_evaluated__:
return (self.__forward_arg__ == other.__forward_arg__ and
self.__forward_value__ == other.__forward_value__)
return self.__forward_arg__ == other.__forward_arg__
return (self.__forward_arg__ == other.__forward_arg__ and
self.__forward_module__ == other.__forward_module__)

def __hash__(self):
return hash(self.__forward_arg__)
return hash((self.__forward_arg__, self.__forward_module__))

def __or__(self, other):
return Union[self, other]
Expand All @@ -725,7 +726,11 @@ def __ror__(self, other):
return Union[other, self]

def __repr__(self):
return f'ForwardRef({self.__forward_arg__!r})'
if self.__forward_module__ is None:
module_repr = ''
else:
module_repr = f', module={self.__forward_module__!r}'
return f'ForwardRef({self.__forward_arg__!r}{module_repr})'

class _TypeVarLike:
"""Mixin for TypeVar-like types (TypeVar and ParamSpec)."""
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2022-01-11-15-54-15.bpo-46333.B1faiF.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Honor module parameter in :class:`ForwardRef` as it carries semantic information about the forward ref. This affects :func:`__eq__`, :func:`__repr__` and :func:`__hash__` .