Skip to content

1/X Fix check_untyped_defs = True mypy errors #5673

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

Merged
merged 2 commits into from
Aug 20, 2019

Conversation

bluetech
Copy link
Member

check_untyped_defs = True instructs mypy to also check bodies of functions which are not annotated (doc).

I would like to enable this option because it provides some internal dogfooding of the type annotations as they are added. For example, we added type annotations for pytest.raises, but since all of the tests are untyped, the tests weren't checked against it. With check_untyped_defs = True, there is at least some basic coverage (number of parameters, parameter names, etc.).

There are a few hundred such errors, mostly mypy complaining that it can't infer the type of empty containers, but some other minor stuff as well. This PR fixes the error for a first batch of modules. Each commit handles a separate module.

@bluetech bluetech force-pushed the type-annotations-3 branch from ddd265a to cd300cc Compare July 30, 2019 07:03
@bluetech
Copy link
Member Author

Updated to fix a couple CI failures.

@blueyed blueyed added the type: enhancement new feature or API change, should be merged into features branch label Jul 30, 2019
@blueyed
Copy link
Contributor

blueyed commented Aug 12, 2019

@bluetech
Is that ready from your side?
Needs to be rebased.
(I would like to have it squashed also, but can see that it might be useful to have it per file/module)

@blueyed
Copy link
Contributor

blueyed commented Aug 12, 2019

Oh, it does not enabled check_untyped_defs = True yet..

@bluetech bluetech force-pushed the type-annotations-3 branch from cd300cc to 64e551f Compare August 12, 2019 10:27
@bluetech
Copy link
Member Author

Is that ready from your side?
Needs to be rebased.
(I would like to have it squashed also, but can see that it might be useful to have it per file/module)

I rebased, it's ready from my side. I don't mind if it's squashed - I mostly split it for reviewing. Let me know if you want me to do it.

Oh, it does not enabled check_untyped_defs = True yet..

Yes, I didn't want to send too much in one PR. I will follow up with more.

@pytest-dev pytest-dev deleted a comment from codecov bot Aug 15, 2019
@blueyed
Copy link
Contributor

blueyed commented Aug 15, 2019

@bluetech
Ok, please squash and force-push then.

This is nice for self-documentation, and is the type required by mypy
for adding to sys.meta_path.
@bluetech bluetech force-pushed the type-annotations-3 branch from 64e551f to 7259c45 Compare August 16, 2019 07:42
@codecov
Copy link

codecov bot commented Aug 16, 2019

Codecov Report

Merging #5673 into features will decrease coverage by 0.04%.
The diff coverage is 99.32%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #5673      +/-   ##
============================================
- Coverage     96.25%   96.21%   -0.05%     
============================================
  Files           117      117              
  Lines         25948    25993      +45     
  Branches       2497     2497              
============================================
+ Hits          24977    25008      +31     
- Misses          667      677      +10     
- Partials        304      308       +4
Impacted Files Coverage Δ
src/_pytest/assertion/rewrite.py 96.35% <100%> (-0.51%) ⬇️
src/_pytest/config/__init__.py 94.5% <100%> (+0.06%) ⬆️
src/_pytest/reports.py 95.07% <100%> (ø) ⬆️
src/_pytest/runner.py 98.06% <100%> (+0.06%) ⬆️
src/_pytest/mark/structures.py 92.44% <100%> (ø) ⬆️
src/_pytest/config/argparsing.py 88.46% <100%> (+0.25%) ⬆️
src/_pytest/nodes.py 95.73% <100%> (+0.19%) ⬆️
src/_pytest/_code/source.py 91.42% <100%> (ø) ⬆️
src/_pytest/mark/evaluate.py 96.7% <100%> (+0.03%) ⬆️
src/_pytest/_code/code.py 94.61% <100%> (+0.04%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f11a7a...7259c45. Read the comment docs.

@bluetech
Copy link
Member Author

@blueyed I squashed all of the commits except for the first one.

Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've quickly compared the old and new number of issues with --check-untyped-defs: 219 before, 134 after.

@bluetech bluetech merged commit c2f7624 into pytest-dev:features Aug 20, 2019
@bluetech
Copy link
Member Author

I merged this, thanks everyone. Next parts will come soon.

@bluetech bluetech deleted the type-annotations-3 branch February 28, 2020 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement new feature or API change, should be merged into features branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants