bpo-33569 Preserve type information with InitVar#8927
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for your contribution, we look forward to reviewing it! |
|
The failing tests need to be addressed. Also, a news blurb is required. |
0406ce7 to
25ad285
Compare
25ad285 to
6136311
Compare
|
@ericvsmith I can't reproduce the build failure, the logs from the build: The output from my machine: I assumed it could be something with local branch, which doesn't have an upstream set, so I refetched master and rebased, but the error persist. Any pointers? Thanks |
|
@hackaugusto: I've checked but everything looks okay to me. I can't see what the problem is, unfortunately. |
There was a problem hiding this comment.
Found it: this line has 5 space indentation.
There was a problem hiding this comment.
ohhh, little bastard. thanks, should be fixed now :)
6136311 to
6426b5e
Compare
There was a problem hiding this comment.
We should probably have a nicer __repr__ here.
Currently you get:
>>> from dataclasses import InitVar
>>> InitVar[int]
<dataclasses.InitVar object at 0x00A041C0>
I'd suggest something like:
def __repr__(self):
return f'dataclasses.InitVar[{self.type}]'
for starters, although typing.ClassVar does something more sophisticated in place of {self.type}. Maybe someday when we import typing from dataclasses we can switch to using their helper function. It's probably worth adding a comment to that effect, too.
There was a problem hiding this comment.
Using f'dataclasses.InitVar[{self.type}]' would look like:
>>> from dataclasses import InitVar
>>> InitVar[int]
dataclasses.InitVar[<class 'int'>]So I added f'dataclasses.InitVar[{self.type.__name__}]' instead. Should I write a test for this?
There was a problem hiding this comment.
If that's what gives you the equivalent of:
>>> import typing
>>> typing.ClassVar[int]
typing.ClassVar[int]
Then I'm all for it.
Yes, we'll need tests for this. Nothing complex, InitVar[int] should be good enough.
There was a problem hiding this comment.
Sorry for the delay, I added a really simple test:
self.assertEqual(InitVar[int].type, int)
Would you like tests using __repr__ and @dataclass?
There was a problem hiding this comment.
I apologize for taking forever to get back to you on this.
Could you test the repr of an InitVar? Something like (untested) repr(InitVar[int]) == 'dataclasses.InitVar[int]'?
I promise I'll commit this as soon as you add this change.
7828a4a to
93dd162
Compare
dataclassses accepts forward declaration of types. This is achieved by having a string which represents the type, e.g. `InitVar[int]`. To properly construct the dataclass attributes these annotations must be interpreted, which is done by pattern matching for the class name and doing a dynamic lookup for the the name in the corresponding module. This dynamic lookup returns the **class** being used in the string, and not the instance which the string represents. Because of that the predicate `_is_initvar` must check for both the type and instances of the type.
93dd162 to
e9c983f
Compare
https://bugs.python.org/issue33569