Skip to content
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

Add rudimentary mypy type checking #5575

Merged
merged 1 commit into from
Jul 9, 2019

Conversation

bluetech
Copy link
Member

@bluetech bluetech commented Jul 8, 2019

(This PR is currently an RFC, I will discuss it in issue #3342).

Add a very lax mypy configuration, add it to tox -e linting, and
fix/ignore the few errors that come up. The idea is to get it running
before diving in too much.

This enables:

  • Progressively adding type annotations and enabling more strict
    options, which will improve the codebase (IMO).

  • Annotating the public API in-line, and eventually exposing it to
    library users who use type checkers (with a py.typed file).

Though, none of this is done yet.

Refs #3342.

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

please also add additional_dependencies: [flake8-typing-imports] to the flake8 hook configuration in .pre-commit-config.yaml

tox.ini Outdated Show resolved Hide resolved
src/_pytest/tmpdir.py Show resolved Hide resolved
src/_pytest/python.py Outdated Show resolved Hide resolved
src/_pytest/mark/structures.py Outdated Show resolved Hide resolved
src/_pytest/fixtures.py Show resolved Hide resolved
src/_pytest/assertion/rewrite.py Outdated Show resolved Hide resolved
src/_pytest/_code/code.py Show resolved Hide resolved
setup.cfg Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 8, 2019

Codecov Report

Merging #5575 into features will increase coverage by <.01%.
The diff coverage is 98.14%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #5575      +/-   ##
============================================
+ Coverage     96.08%   96.09%   +<.01%     
============================================
  Files           117      117              
  Lines         25716    25728      +12     
  Branches       2494     2494              
============================================
+ Hits          24709    24723      +14     
+ Misses          702      701       -1     
+ Partials        305      304       -1
Impacted Files Coverage Δ
src/_pytest/tmpdir.py 100% <ø> (ø) ⬆️
src/_pytest/assertion/rewrite.py 96.11% <100%> (+0.03%) ⬆️
src/_pytest/capture.py 93.44% <100%> (-1.29%) ⬇️
src/_pytest/python_api.py 97.34% <100%> (+0.01%) ⬆️
src/_pytest/_code/source.py 91.42% <100%> (ø) ⬆️
testing/test_pdb.py 99% <100%> (ø) ⬆️
testing/python/raises.py 94.48% <100%> (ø) ⬆️
src/_pytest/mark/structures.py 92.61% <100%> (+0.04%) ⬆️
src/_pytest/reports.py 95.07% <100%> (+0.02%) ⬆️
src/_pytest/fixtures.py 97.52% <100%> (ø) ⬆️
... and 12 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 e33736c...c1167ac. Read the comment docs.

@nicoddemus
Copy link
Member

Thanks @bluetech for working on this!

I've changed the title to "WIP" to explicitly state this is not yet prime for merging. 👍

@nicoddemus nicoddemus changed the title Add rudimentary mypy type checking WIP Add rudimentary mypy type checking Jul 8, 2019
@bluetech
Copy link
Member Author

bluetech commented Jul 8, 2019

I updated the PR to address some of @asottile's comments.

setup.cfg Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
src/_pytest/_code/code.py Show resolved Hide resolved
src/_pytest/capture.py Outdated Show resolved Hide resolved
src/_pytest/fixtures.py Show resolved Hide resolved
src/_pytest/python.py Outdated Show resolved Hide resolved
@bluetech
Copy link
Member Author

bluetech commented Jul 8, 2019

Updated again. I've switched to use pre-commit and addresses some of the other comments. Currently some of the tests are failing, but I can't check what causes them exactly ATM - will check later, but I pushed anyway.

Add a very lax mypy configuration, add it to tox -e linting, and
fix/ignore the few errors that come up. The idea is to get it running
before diving in too much.

This enables:

- Progressively adding type annotations and enabling more strict
  options, which will improve the codebase (IMO).

- Annotating the public API in-line, and eventually exposing it to
  library users who use type checkers (with a py.typed file).

Though, none of this is done yet.

Refs pytest-dev#3342.
@bluetech
Copy link
Member Author

bluetech commented Jul 9, 2019

Updated - CI looks happy now. I added __init__.pyi files (note: pyi not py) to workaround the mypy issue without disturbing the tests.

I also changed the mypy pre-commit to use args: [] so all options are managed exclusively in the setup.cfg.

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

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

👏 🗜️ 👏 thanks for your work on this! we should probably make a followup to make *Capture classes generic on the string type -- but for now we can just # type: ignore it

@asottile asottile changed the title WIP Add rudimentary mypy type checking Add rudimentary mypy type checking Jul 9, 2019
@asottile asottile merged commit 39a43db into pytest-dev:features Jul 9, 2019
@asottile
Copy link
Member

asottile commented Jul 9, 2019

@nicoddemus ok with cherry-picking this to master? I'd like to be able to iterate on this there (and not have it regress)

@nicoddemus
Copy link
Member

@asottile it doesn't add any extra dependencies to users of the library or changes any public API, so feel free to go ahead! 👍

@@ -188,11 +189,11 @@ def path(self):
""" path to the source code """
return self.frame.code.path

def getlocals(self):
@property
Copy link
Member

Choose a reason for hiding this comment

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

that one is a breaking change of the api

Copy link
Member

Choose a reason for hiding this comment

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

I don't think Code objects are exposed -- or at least I can't find any cases that they are (?)

@@ -31,6 +33,9 @@
from _pytest.outcomes import fail
from _pytest.outcomes import TEST_OUTCOME

if False: # TYPE_CHECKING
from typing import Type
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this only imported conditionally, but not the others?

Copy link
Member

@asottile asottile Jul 10, 2019

Choose a reason for hiding this comment

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

it is not available in python 3.5.0 / 3.5.1

@bluetech bluetech deleted the mypy-initial 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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants