-
-
Notifications
You must be signed in to change notification settings - Fork 33.3k
gh-89687: fix get_type_hints with dataclasses __init__ generation
#29158
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good fix -- the test script I linked to in the BPO issue passes fine (as do the tests you've added, obviously). Nice job!
Lib/test/test_typing.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bikeshedding point, but: would it be better to put these tests in test_dataclasses? For the similar issue with TypedDict, the new tests were put next to the other TypedDict tests. Moreover, given that this is a very specific issue with the way that the dataclasses module generates __init__ functions, it seems like more of a dataclasses bug than a get_type_hints bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with any decision from maintainers 👍
Both cases make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed!
|
I found a corner case: when I define a proxy module, say from __future__ import annotations
import dataclasses
from test import dataclass_textanno # We need to be sure that `Foo` is not in scope
class Custom:
pass
@dataclasses.dataclass
class Child(dataclass_textanno.Bar):
custom: Custom
@dataclasses.dataclass(init=False)
class WithFutureInit(Child):
def __init__(self, foo: dataclass_textanno.Foo, custom: Custom) -> None:
passThen, this test does not pass: def test_dataclass_from_proxy_module(self):
from test import dataclass_textanno, dataclass_textanno2
from dataclasses import dataclass
@dataclass
class Default(dataclass_textanno2.Child):
pass
self.assertEqual(
get_type_hints(Defualt.__init__),
{'foo': dataclass_textanno.Foo, 'custom': dataclass_textanno2.Custom, 'return': type(None)},
)Output: |
|
Now it is even more wild. I collect |
|
One more case: same names obviously get overriden by the current logic. I will try something else. |
Not sure I understand. What's a failing test case? |
|
@AlexWaygood done! Here's the failing test case: https://github.com/python/cpython/pull/29158/files#diff-f0436842533448f8f02a19081d7c9cc8372b6685756caf32c83dfecebbee68e7R3189-R3219 And here's my new solution: https://github.com/python/cpython/pull/29158/files#diff-44ce2dc1c4922b2f5cf7631d8f86cc569a4c25eb003aaecdc2bc22eb9163d5f5R455-R468 I now manually resolve |
Ahh, I understand -- thanks! |
|
Thanks! Done 👍 |
|
This PR is stale because it has been open for 30 days with no activity. |
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>
7205490 to
df5fda9
Compare
|
Rebased 🙂 Friendly ping to @ericvsmith and @Fidget-Spinner |
|
I'm putting this off until the SC makes a decision on PEP 649. |
get_type_hints with dataclasses __init__ generationget_type_hints with dataclasses __init__ generation
|
PEP 649 is accepted and implemented in 3.14, so what should be done with this PR? |
|
I lost context of this, please recreate :) |
This is a rather brave idea to manipulate
globals, but it looks like it works.It can backfire is some corner cases, though. But, I am not able to think about any. Ideas? 🙂
https://bugs.python.org/issue45524
CC @AlexWaygood @aidan.b.clark