bpo-38605: Make postponed evaluation of annotations default by isidentical · Pull Request #20434 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation61 Commits18 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
[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:
- add what's new entry
`
https://bugs.python.org/issue38605
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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!
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! 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 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.
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);
import typing
from dataclasses import dataclass
@dataclass
class T:
a: "typing.ClassVar[int]"
T()
We can either handle it like how we do in the ForwardRef, or solve these all problems completely in the compiler level by doing something like this;
+++ 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., list["int"]). And it doesn't produce the same result with the future import in 3.9. It also seems quite inconsistent to say "annotations get stringified except if they are already strings" -- this makes it impossible to reconstruct the original annotation by un-stringifying.
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 :-).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 from __future__ import annotations... :-(
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...
As a side effect, this now folds into Optional[int]
@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)
As a side effect, this now folds into
Optional[int]@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)
But note that type(None) is invalid according to PEP 484 -- it should just be None.
But note that type(None) is invalid according to PEP 484 -- it should just be None.
I just copied over the test case, my point was we are changing the shape of a declaration;
>>> from typing import *
>>> def x(a: Union[int, None]): pass
...
>>> inspect.signature(x)
<Signature (a: Optional[int])>
But it is not much of a big deal, I guess, just wanted to mention about it
But note that type(None) is invalid according to PEP 484 -- it should just be None.
I just copied over the test case, my point was we are changing the shape of a declaration;
>>> from typing import * >>> def x(a: Union[int, None]): pass ... >>> inspect.signature(x) <Signature (a: Optional[int])>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]
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight wording changes. I'll apply these and then hopefully the test flake will go away, and then I'll land it. Fingers crossed! And thanks for all the work put into this -- I betcha you didn't expect it to be this subtle. (I know I didn't.)
I betcha you didn't expect it to be this subtle. (I know I didn't.)
Indeed!
shihai1991 added a commit to shihai1991/cpython that referenced this pull request
- origin/master: (147 commits)
Fix the attribute names in the docstring of GenericAlias (pythonGH-22594)
bpo-39337: Add a test case for normalizing of codec names (pythonGH-19069)
bpo-41557: Update Windows installer to use SQLite 3.33.0 (pythonGH-21960)
bpo-41976: Fix the fallback to gcc of ctypes.util.find_library when using gcc>9 (pythonGH-22598)
bpo-41306: Allow scale value to not be rounded (pythonGH-21715)
bpo-41970: Avoid test failure in test_lib2to3 if the module is already imported (pythonGH-22595)
bpo-41376: Fix the documentation of
site.getusersitepackages()(pythonGH-21602) Revert "bpo-26680: Incorporate is_integer in all built-in and standard library numeric types (pythonGH-6121)" (pythonGH-22584) bpo-41923: PEP 613: Add TypeAlias to typing module (python#22532) Fix comment about PyObject_IsTrue. (pythonGH-22343) bpo-38605: Make 'from future import annotations' the default (pythonGH-20434) bpo-41905: Add abc.update_abstractmethods() (pythonGH-22485) bpo-41944: No longer call eval() on content received via HTTP in the UnicodeNames tests (pythonGH-22575) bpo-41944: No longer call eval() on content received via HTTP in the CJK codec tests (pythonGH-22566) Post 3.10.0a1 Python 3.10.0a1 bpo-41584: clarify when the reflected method of a binary arithemtic operator is called (python#22505) bpo-41939: Fix test_site.test_license_exists_at_url() (python#22559) bpo-41774: Tweak new programming FAQ entry (pythonGH-22562) bpo-41936. Remove macros Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION (pythonGH-22552) ...
xzy3 pushed a commit to xzy3/cpython that referenced this pull request
The hard part was making all the tests pass; there are some subtle issues here, because apparently the future import wasn't tested very thoroughly in previous Python versions.
For example, inspect.signature() returned type objects normally (except for forward references), but strings with the future import. We changed it to try and return type objects by calling typing.get_type_hints(), but fall back on returning strings if that function fails (which it may do if there are future references in the annotations that require passing in a specific namespace to resolve).
Reviewers
