Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upbpo-38605: Make postponed evaluation of annotations default #20434
Conversation
Misc/NEWS.d/next/Core and Builtins/2020-05-26-19-02-54.bpo-38605.9Vxlbm.rst
Outdated
Show resolved
Hide resolved
99bf3de
to
b9d9f0a
|
Overall, the whole change looks like the right approach. I will wait until the PR is not marked as draft to review it in depth. Thanks for working on this issue! |
Thanks! As far as I'm aware there are only 2 things that currently hold me from marking this PR as ready to review, one is some silly typing failures (will look them) and the other one is issue 40794. |
1efa11e
to
51c7556
|
@isidentical: hey, there is now a conflict on your PR. Would you mind to try to solve it? |
|
I think this is in part on me -- I started a review but never finished it. IIRC there were also some unresolved problems, but I don't know if they are still unresolved. Anyway, here are my (very old) review comments that I never sent out. |
I'm giving a rebase right now, and try to merge the latest typing changes on get_type_hints etc with current version. Please do not review until I push the latest changes (at worst tomorrow) |
|
Found a different example for double-stringified sources in dataclasses (this used to work, but now it can't get parsed by the static regex in the dataclasses module since now there are extra quotes);
We can either handle it like how we do in the +++ b/Python/compile.c
@@ -2016,7 +2016,15 @@ error:
static int
compiler_visit_annexpr(struct compiler *c, expr_ty annotation)
{
- ADDOP_LOAD_CONST_NEW(c, _PyAST_ExprAsUnicode(annotation));
+ PyObject *value;
+ if (annotation->kind == Constant_kind && PyUnicode_CheckExact(annotation->v.Constant.value)) {
+ value = annotation->v.Constant.value;
+ Py_INCREF(value);
+ }
+ else {
+ value = _PyAST_ExprAsUnicode(annotation);
+ }
+ ADDOP_LOAD_CONST_NEW(c, value);
return 1;
}Any suggestions @gvanrossum? |
|
I don't think that solves it completely, does it? ISTM we still get double quotes when the annotation contains a string literal as a generic parameter (e.g., I'd rather fix this in dataclasses.py and backport the fix to 3.9.1 (since it's clearly too late for 3.9.0 :-). |
|
I worry that people do introspection on dataclasses and will be confused by finding strings instead of type objects. But I don't know what else to do. I somehow think not too many people outside Facebook have been using I also worry about the change in behavior in the inspect module. But I think you're over the hump and we can soon land this... |
Misc/NEWS.d/next/Core and Builtins/2020-05-27-16-08-16.bpo-38605.rcs2uK.rst
Outdated
Show resolved
Hide resolved
|
As a side effect, this now folds into @dataclass
class C:
x: Union[int, type(None)] = None- C.__doc__ == C(x:'Union[int, type(None)]'=None)
+ C.__doc__ == C(x:Optional[int]=None) |
| except Exception: | ||
| # First, try to use the get_type_hints to resolve | ||
| # annotations. But for keeping the behavior intact | ||
| # if there was a problem with that (like the namespace | ||
| # can't resolve some annotation) continue to use | ||
| # string annotations | ||
| return func.__annotations__ |
gvanrossum
Oct 4, 2020
Member
I worry that that exception is going to mask errors (and e.g. cause us to update tests from a: int to a: 'int').
I wonder if it would be better to just return {} in this case (given that we don't want to bubble up the error)?
Also, catching Exception feels wrong -- but I do agree that there are lots of different errors you could get (I managed to trigger TypeError, AttributeError, NameError, SyntaxError, IndentationError, IndentationError -- so who knows what else it could raise. (Just mumbling to myself here. :-)
isidentical
Oct 4, 2020
Author
Member
Actually, there is not much we can do for the exceptions since there is a code path that is going to eval.
(.venv) (Python 3.10.0a0) [ 12:31ÖÖ ] [ isidentical@desktop:~/cpython/cpython(bpo-38605✗) ]
$ python t.py
(a: 'b()')
(.venv) (Python 3.10.0a0) [ 12:31ÖÖ ] [ isidentical@desktop:~/cpython/cpython(bpo-38605✗) ]
$ ./python t.py
Traceback (most recent call last):
File "/home/isidentical/cpython/cpython/t.py", line 4, in <module>
print(inspect.signature(x))
File "/home/isidentical/cpython/cpython/Lib/inspect.py", line 3133, in signature
return Signature.from_callable(obj, follow_wrapped=follow_wrapped)
File "/home/isidentical/cpython/cpython/Lib/inspect.py", line 2882, in from_callable
return _signature_from_callable(obj, sigcls=cls,
File "/home/isidentical/cpython/cpython/Lib/inspect.py", line 2333, in _signature_from_callable
return _signature_from_function(sigcls, obj,
File "/home/isidentical/cpython/cpython/Lib/inspect.py", line 2178, in _signature_from_function
annotations = _get_type_hints(func)
File "/home/isidentical/cpython/cpython/Lib/inspect.py", line 2127, in _get_type_hints
return typing.get_type_hints(func)
File "/home/isidentical/cpython/cpython/Lib/typing.py", line 1420, in get_type_hints
value = _eval_type(value, globalns, localns)
File "/home/isidentical/cpython/cpython/Lib/typing.py", line 255, in _eval_type
return t._evaluate(globalns, localns, recursive_guard)
File "/home/isidentical/cpython/cpython/Lib/typing.py", line 501, in _evaluate
eval(self.__forward_code__, globalns, localns),
File "<string>", line 1, in <module>
File "/home/isidentical/cpython/cpython/t.py", line 1, in b
def b(): 1/0
ZeroDivisionError: division by zero
(.venv) (Python 3.10.0a0) [ 12:31ÖÖ ] [ isidentical@desktop:~/cpython/cpython(bpo-38605✗) ]
$ cat t.py
def b(): 1/0
def x(a: 'b()'): pass
import inspect
print(inspect.signature(x))
isidentical
Oct 5, 2020
Author
Member
This is a bit problematic when we are in a different context than the module, like this;
import inspect, typing
def foo():
class F: ...
def foo(bar: F): ...
print(inspect.signature(foo))
foo()This would be normally resolved when we call get_type_hints with locals() but we can't do it (at least not without changing the API / or doing some frame-level dark magic). (that is from a test case of ours).
isidentical
Oct 5, 2020
Author
Member
Does inspect.signature() have a way to pass a namespace?
Apparently not
But note that |
I just copied over the test case, my point was we are changing the shape of a declaration;
But it is not much of a big deal, I guess, just wanted to mention about it |
Hm, your comment was not linked to any particular line of code. FWIW that behavior is built into typing.Union: >>> import typing
>>> typing.Union[int, None]
typing.Optional[int]
>>> |
[This is a draft for my public work in progress PoC, feel free to create another PR if you can successfully pass the test suite with the least breakage]Todo:
`
https://bugs.python.org/issue38605