-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Some type annotation & check_untyped_defs fixes #6311
Conversation
8a3c06e
to
838f7bf
Compare
838f7bf
to
6175738
Compare
Rebased on latest features (no conflicts). |
I'd rather see this enabled via mypy's config (see #6452), mostly to avoid diff churn. |
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.
Let's get this merged finally.
(needs a rebase though)
Would also be interesting to see how this behaves with #6486, but should not hold it back any longer.
recorder.hook.pytest_runtest_logreport(report=rep) | ||
rep2.passed = False | ||
rep2.skipped = True | ||
recorder.hook.pytest_runtest_logreport(report=rep2) |
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.
Diff churn like this makes me think that --allow-redefinition
would be better (docs).
If not for "src" then maybe for "testing" at least, and maybe only per module there then?
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.
I personally think this makes the code less confusing. I generally prefer to fix than to use allow-redefinition
, for clarity and for the occasional errors it catches. Though it might be slightly annoying sometimes.
@bluetech |
Also replace one direct call to `compile` with this flag with the equivalent wrapper `ast.parse`. This function can have a more precise type.
These are more "dirty" than the previous batch (that's why they were left out). The trouble is that `compile` can return either a code object or an AST depending on a flag, so we need to add an overload to make the common case Union free. But it's still worthwhile.
The expected_warning is optional.
6175738
to
9b41b05
Compare
9b41b05
to
3392be3
Compare
The first two commits mostly finish annotating
_pytest/code/
.The other commits fix
check_untyped_defs
errors in a few test files. This discovered a couple of problems in the annotations, which are fixed.Most of the diff is adding
-> None
to test functions. Also, because of a deficiency in mypy, exceptions defined on functions e.g.pytest.fail.Exception
don't work, so I replace them with the direct classes instead.