GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign up|
I wonder if the fix shouldn't apply to all callers of |
I think there's no harm in doing that. I've updated the PR. It's a bit more invasive now, but the benefit is that we always use the same mechanism (a closure) to inject objects into generated code. |
|
BTW this looks like a relatively safe change for 3.7.1, but it's Eric's call. |
| if globals is not None and '__builtins__' not in globals: | ||
| globals['__builtins__'] = builtins | ||
| if '__builtins__' not in locals: | ||
| locals['__builtins__'] = builtins |
This doesn't actually work -- exec() gets __builtins__ only from globals.
Hm, we only need to bound the "__builtins__" name inside the closure as there's generated code that explicitly uses it as "__builtins__.attribute". I can change the name to "_builtins" and everything should be fine too or... am I missing something here?
Probably it's better to rename it to BUILTINS, to make it more obvious.
|
I don't follow the logic in dataclasses.py well enough to approve this, though I see nothing wrong other than a missing trailing comma. @ericvsmith will have to have the last word. |
|
I'll try and look at this today. Thanks, @1st1 and @gvanrossum for the code and reviews. |
|
According to https://discuss.python.org/t/3-7-1-and-3-6-7-release-update/184 there's still couple of days to get this into 3.7.1. @ericvsmith do you have time for a review? |
|
FWIW we can just merge the 39006fb commit to 3.7 which does the trick in the most minimal way. |
|
Unfortunately I'm out of town until 2018-10-10 with limited internet access until then. I won't be able to do a decent review until then. |
|
Hi Eric, Python 3.7.2rc1 has been released but this PR is still not merged. Is it possible to somehow unblock it? |
|
I apologize for being tied up with a client on end-of-year work. I don't see how this could get in to 3.7.2, but I'm planning on looking at it in January. |
| @@ -0,0 +1 @@ | |||
| Fix dataclasses to support __future__ "annotations" mode | |||
Strictly speaking, this problem is not directly related to __future__ annotations.
It's about postponed evaluation of type annotations in general, including forward references in pre-PEP-563 annotations.
Consider rewording along the lines of "Fix dataclasses to support forward references in type annotations".
Here is an example that does not use PEP 563:
from typing import get_type_hints
from dataclasses import dataclass
class T:
pass
@dataclass()
class C2:
x: 'T'
print(get_type_hints(C2.__init__))Before your change:
Traceback (most recent call last):
File ".\zzz.py", line 11, in <module>
print(get_type_hints(C2.__init__))
File "C:\Python37\lib\typing.py", line 1001, in get_type_hints
value = _eval_type(value, globalns, localns)
File "C:\Python37\lib\typing.py", line 260, in _eval_type
return t._evaluate(globalns, localns)
File "C:\Python37\lib\typing.py", line 464, in _evaluate
eval(self.__forward_code__, globalns, localns),
File "<string>", line 1, in <module>
NameError: name 'T' is not defined
After your change it works as expected:
{'x': <class '__main__.T'>, 'return': <class 'NoneType'>}
|
|
||
| exec(txt, globals, locals) | ||
| return locals[name] | ||
| local_vars = ', '.join(locals.keys()) |
|
It looks like https://bugs.python.org/issue37948 will be also fixed by this PR. |
|
FWIW the fix looks good to me. It would be great to add few more comments and a test for plain string annotations and/or https://bugs.python.org/issue37948 |
It won't, I just tested with https://github.com/1st1/cpython/tree/fix_dc_introspect |
@a-recknagel : Could you post your test code and the results? Thanks. |
|
@ericvsmith Sure. python was built from e38de43, and I ran >>> import dataclasses
>>> import typing
>>> A = dataclasses.make_dataclass('A', ['var'])
>>> typing.get_type_hints(A)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/arne/pythons/python_dc_fix/lib/python3.8/typing.py", line 972, in get_type_hints
value = _eval_type(value, base_globals, localns)
File "/home/arne/pythons/python_dc_fix/lib/python3.8/typing.py", line 259, in _eval_type
return t._evaluate(globalns, localns)
File "/home/arne/pythons/python_dc_fix/lib/python3.8/typing.py", line 463, in _evaluate
eval(self.__forward_code__, globalns, localns),
File "<string>", line 1, in <module>
NameError: name 'typing' is not definedWhich is the same behavior as on the current master. |
|
@ambv: Please replace |
…ythonGH-9518) (cherry picked from commit d219cc4) Co-authored-by: Yury Selivanov <yury@magic.io>
|
GH-17531 is a backport of this pull request to the 3.8 branch. |
|
GH-17532 is a backport of this pull request to the 3.7 branch. |
…ythonGH-9518) (cherry picked from commit d219cc4) Co-authored-by: Yury Selivanov <yury@magic.io>
1st1 commentedSep 23, 2018
•
edited by bedevere-bot
https://bugs.python.org/issue34776