-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
In Python 3.8, use the stdlib ast instead of typed_ast #6539
Conversation
…ctual test output (Python 3.8's ast changed a whole bunch of error messages from *can't* to *cannot*.)
…conditional on version
… in Python 3.8 We shouldn't need to test parser errors anyways
I changed visit_NameConstant() to set the line number, and this caused the HTML report to flag the line 'assert False' as precise (correct) rather than as empty (wrong).
(My tox-fu is too weak to make this work -- it still tries to run Python 3.7.) This reverts commit 8c24cd8.
At some point tomorrow I hope the Python 3.8-dev build will include python/cpython#12295 and then the tests should all pass. |
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.
Thanks! Mostly looks good, just few minor suggestions.
Oh, my review crossed with some updates and this is no more WIP. |
After a job restart, the tests passed (meaning python/cpython#12295 made it into Travis's "3.8-dev"). Maybe @jukka can greenlight this? |
@ilevkivskyi Can you review this one more time? @JukkaL told me offline he's fine with it. |
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.
Thanks for updates! LGTM.
@@ -1094,7 +1177,7 @@ def invalid_type(self, node: AST, note: Optional[str] = None) -> RawExpressionTy | |||
def visit(self, node: ast3.expr) -> Type: ... | |||
|
|||
@overload # noqa | |||
def visit(self, node: Optional[AST]) -> Optional[Type]: ... | |||
def visit(self, node: Optional[AST]) -> Optional[Type]: ... # noqa |
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.
It's sad this #noqa
is still needed. The issue PyCQA/pyflakes#434 was fixed two weeks ago, so we will be able to update flake8
and remove these soon.
Does it makes sense to add a |
Remove the conditional method definition added in #6539, which mypyc doesn't support.
No, it’s still needed for 2.7. |
2.7 is included in The idea is to avoid needlessly installing a package when it's not needed, similar to how the |
@bluetech, I believe Guido means "It's needed to parse 2.7 on Python 3". Mypy only runs under Python 3, which means to type check Python 2 code we still need typed_ast. |
Ha, I misunderstood. That makes sense. |
Fixes #3871. This change fixes a few things concerning class/function definition line numbers. For users running mypy with Python 3.8+: - Function definitions are now reported on the correct line, rather than the line of the first decorator plus the number of decorators. - Class definitions are now reported on the correct line, rather than the line of the first decorator. - **These changes are fully backward compatible with respect to `# type: ignore` lines**. In other words, existing comments will _not_ need to be moved. This is accomplished by defining an ignore "scope" just like we added to expressions with #6648. See the recent discussion at #3871 for reasoning. For other users (running mypy with Python 3.7 or earlier): - Class definition lines are reported by using the same decorator-offset estimate as functions. This is both more accurate, and necessary to get 15 or so tests to pass for both pre- and post-3.8 versions. This seems fine, since there [doesn't appear](#6539 (comment)) to be a compelling reason why it was different in the first place. - **This change is also backward-compatible with existing ignores, as above.** About 15 tests had to have their error messages / nodes moved down one line, and 4 new regression tests have been added to `check-38.test`.
This required mostly changes to fastparse.py, plus a smattering of other things. The tests still don't all pass, but I believe all remaining errors are due to differences in error messages for
SyntaxError
between typed_ast and Python 3.8's ast (where a bunch of improvements and spelling changes were made, including "can't" -> "cannot").To test, you need python/cpython#12295 (else it crashes due to
ast.Constant
not having akind
field). UPDATE: That landed today, March 13.UPDATE: This also requires a typeshed sync to incorporate python/typeshed#2859; I've send a PR to typeshed as #6540. UPDATE^2: Those have landed.