frozenleaves/pytorch
1
0
Fork 0
mirror of https://github.com/pytorch/pytorch.git synced 2025-10-20 21:14:14 +08:00
Code Issues Packages Projects Releases Wiki Activity

Refactor mypy configs list into editor-friendly wrapper (#50826)

Browse Source 
Summary:
Closes https://github.com/pytorch/pytorch/issues/50513 by resolving the first three checkboxes. If this PR is merged, I will also modify one or both of the following wiki pages to add instructions on how to use this `mypy` wrapper for VS Code editor integration:

- [Guide for adding type annotations to PyTorch](https://github.com/pytorch/pytorch/wiki/Guide-for-adding-type-annotations-to-PyTorch)
- [Lint as you type](https://github.com/pytorch/pytorch/wiki/Lint-as-you-type)

The test plan below is fairly manual, so let me know if I should add more automated tests to this PR.

Pull Request resolved: https://github.com/pytorch/pytorch/pull/50826

Test Plan:
Unit tests for globbing function:
```
python test/test_testing.py TestMypyWrapper -v
```

Manual checks:

- Uninstall `mypy` and run `python test/test_type_hints.py` to verify that it still works when `mypy` is absent.
- Reinstall `mypy` and run `python test/test_type_hints.py` to verify that this didn't break the `TestTypeHints` suite.
- Run `python test/test_type_hints.py` again (should finish quickly) to verify that this didn't break `mypy` caching.
- Run `torch/testing/_internal/mypy_wrapper.py` on a few Python files in this repo to verify that it doesn't give any additional warnings when the `TestTypeHints` suite passes. Some examples (compare with the behavior of just running `mypy` on these files):
  ```sh
  torch/testing/_internal/mypy_wrapper.py README.md
  torch/testing/_internal/mypy_wrapper.py tools/fast_nvcc/fast_nvcc.py
  torch/testing/_internal/mypy_wrapper.py test/test_type_hints.py
  torch/testing/_internal/mypy_wrapper.py torch/random.py
  torch/testing/_internal/mypy_wrapper.py torch/testing/_internal/mypy_wrapper.py
  ```
- Remove type hints from `torch.testing._internal.mypy_wrapper` and verify that running `mypy_wrapper.py` on that file gives type errors.
- Remove the path to `mypy_wrapper.py` from the `files` setting in `mypy-strict.ini` and verify that running it again on itself no longer gives type errors.
- Add `test/test_type_hints.py` to the `files` setting in `mypy-strict.ini` and verify that running the `mypy` wrapper on it again now gives type errors.
- Remove type hints from `torch/random.py` and verify that running the `mypy` wrapper on it again now gives type errors.
- Add the suggested JSON from the docstring of `torch.testing._internal.mypy_wrapper.main` to your `.vscode/settings.json` and verify that VS Code gives the same results (inline, while editing any Python file in the repo) as running the `mypy` wrapper on the command line, in all the above cases.

Reviewed By: glaringlee, walterddr

Differential Revision: D25977352

Pulled By: samestep

fbshipit-source-id: 4b3a5e8a9071fcad65a19f193bf3dc7dc3ba1b96
This commit is contained in:
Sam Estep
2021-01-22 13:26:58 -08:00
committed by Facebook GitHub Bot
parent 7e10fbfb71
commit 73dffc8452
5 changed files with 247 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
mypy-strict.ini
View File

@ -1,7 +1,10 @@
# This is a MyPy config file but unlike mypy.ini, it enforces very strict typing
# rules. The intention is for this config file to be used to ENFORCE that
# people are using mypy on codegen files.
#
# This is the PyTorch mypy-strict.ini file (note: don't change this line! -
# test_run_mypy in test/test_type_hints.py uses this string)
# Unlike mypy.ini, it enforces very strict typing rules. The intention is for
# this config file to be used to ENFORCE that people are using mypy on codegen
# files.
# For now, only code_template.py and benchmark utils Timer are covered this way
[mypy]
@ -33,6 +36,7 @@ strict_equality = True
files = tools/codegen/gen.py,
tools/autograd/*.py,
tools/pyi/*.py,
torch/testing/_internal/mypy_wrapper.py,
torch/utils/benchmark/utils/common.py,
torch/utils/benchmark/utils/timer.py,
torch/utils/benchmark/utils/valgrind_wrapper/*.py,
@ -50,6 +54,10 @@ follow_imports = skip
[mypy-torch.*]
follow_imports = skip
# Missing stub.
# Missing stubs.
[mypy-numpy]
ignore_missing_imports = True
[mypy-mypy.*]
ignore_missing_imports = True

6
mypy.ini
View File

@ -1,5 +1,6 @@
# This is the PyTorch MyPy config file (note: don't change this line! -
# This is the PyTorch mypy.ini file (note: don't change this line! -
# test_run_mypy in test/test_type_hints.py uses this string)
[mypy]
cache_dir = .mypy_cache/normal
warn_unused_configs = True
@ -263,3 +264,6 @@ ignore_missing_imports = True
[mypy-librosa.*]
ignore_missing_imports = True
[mypy-mypy.*]
ignore_missing_imports = True

57
test/test_testing.py
View File

@ -6,6 +6,7 @@ from torch.testing._internal.common_utils import \
(TestCase, make_tensor, run_tests, slowTest)
from torch.testing._internal.common_device_type import \
(instantiate_device_type_tests, onlyCUDA, onlyOnCPUAndCUDA, dtypes)
from torch.testing._internal import mypy_wrapper
# For testing TestCase methods and torch.testing functions
class TestTesting(TestCase):
@ -487,5 +488,61 @@ if __name__ == '__main__':
instantiate_device_type_tests(TestTesting, globals())
class TestMypyWrapper(TestCase):
def test_glob(self):
# can match individual files
self.assertTrue(mypy_wrapper.glob(
pattern='test/test_torch.py',
filename='test/test_torch.py',
))
self.assertFalse(mypy_wrapper.glob(
pattern='test/test_torch.py',
filename='test/test_testing.py',
))
# dir matters
self.assertFalse(mypy_wrapper.glob(
pattern='tools/codegen/utils.py',
filename='torch/nn/modules.py',
))
self.assertTrue(mypy_wrapper.glob(
pattern='setup.py',
filename='setup.py'
))
self.assertFalse(mypy_wrapper.glob(
pattern='setup.py',
filename='foo/setup.py'
))
self.assertTrue(mypy_wrapper.glob(
pattern='foo/setup.py',
filename='foo/setup.py'
))
# can match dirs
self.assertTrue(mypy_wrapper.glob(
pattern='torch',
filename='torch/random.py',
))
self.assertTrue(mypy_wrapper.glob(
pattern='torch',
filename='torch/nn/cpp.py',
))
self.assertFalse(mypy_wrapper.glob(
pattern='torch',
filename='tools/fast_nvcc/fast_nvcc.py',
))
# can match wildcards
self.assertTrue(mypy_wrapper.glob(
pattern='tools/autograd/*.py',
filename='tools/autograd/gen_autograd.py',
))
self.assertFalse(mypy_wrapper.glob(
pattern='tools/autograd/*.py',
filename='tools/autograd/deprecated.yaml',
))
if __name__ == '__main__':
run_tests()

67
test/test_type_hints.py
View File

@ -1,5 +1,6 @@
import unittest
from torch.testing._internal.common_utils import TestCase, run_tests, set_cwd
from torch.testing._internal.mypy_wrapper import config_files
import tempfile
import torch
import doctest
@ -7,7 +8,7 @@ import os
import inspect
try:
import mypy.api # type: ignore
import mypy.api
HAVE_MYPY = True
except ImportError:
HAVE_MYPY = False
@ -129,55 +130,45 @@ class TestTypeHints(TestCase):
@unittest.skipIf(not HAVE_MYPY, "need mypy")
def test_run_mypy(self):
"""
Runs mypy over all files specified in mypy.ini
Note that mypy.ini is not shipped in an installed version of PyTorch,
so this test will only run mypy in a development setup or in CI.
Runs mypy over all files specified in our mypy configs
Note that our mypy configs are not shipped in an installed
version of PyTorch, so this test will only run mypy in a
development setup or in CI.
"""
def is_torch_mypyini(path_to_file):
with open(path_to_file, 'r') as f:
first_line = f.readline()
if first_line.startswith('# This is the PyTorch MyPy config file'):
name = os.path.basename(path_to_file)
if first_line.startswith(f'# This is the PyTorch {name} file'):
return True
return False
test_dir = os.path.dirname(os.path.realpath(__file__))
repo_rootdir = os.path.join(test_dir, '..')
mypy_inifile = os.path.join(repo_rootdir, 'mypy.ini')
if not (os.path.exists(mypy_inifile) and is_torch_mypyini(mypy_inifile)):
self.skipTest("Can't find PyTorch MyPy config file")
# to add more configs, edit the implementation of the
# config_files function rather than editing this test or adding
# more tests to this suite
for ini in config_files():
with self.subTest(msg=ini):
test_dir = os.path.dirname(os.path.realpath(__file__))
repo_rootdir = os.path.join(test_dir, '..')
mypy_inifile = os.path.join(repo_rootdir, ini)
if not (os.path.exists(mypy_inifile) and is_torch_mypyini(mypy_inifile)):
self.skipTest("Can't find PyTorch MyPy config file")
import numpy
if numpy.__version__ == '1.20.0.dev0+7af1024':
self.skipTest("Typeannotations in numpy-1.20.0-dev are broken")
import numpy
if numpy.__version__.startswith('1.20.0.dev0'):
self.skipTest("Typeannotations in numpy-1.20.0-dev are broken")
# TODO: Would be better not to chdir here, this affects the entire
# process!
with set_cwd(repo_rootdir):
(stdout, stderr, result) = mypy.api.run([])
# TODO: Would be better not to chdir here, this affects
# the entire process!
with set_cwd(repo_rootdir):
(stdout, stderr, result) = mypy.api.run([
'--config', mypy_inifile,
])
if result != 0:
self.fail(f"mypy failed: {stdout} {stderr}")
@unittest.skipIf(not HAVE_MYPY, "need mypy")
def test_run_mypy_strict(self):
"""
Runs mypy over all files specified in mypy-strict.ini
"""
test_dir = os.path.dirname(os.path.realpath(__file__))
repo_rootdir = os.path.join(test_dir, '..')
mypy_inifile = os.path.join(repo_rootdir, 'mypy-strict.ini')
if not os.path.exists(mypy_inifile):
self.skipTest("Can't find PyTorch MyPy strict config file")
with set_cwd(repo_rootdir):
(stdout, stderr, result) = mypy.api.run([
'--config', mypy_inifile,
])
if result != 0:
self.fail(f"mypy failed: {stdout} {stderr}")
if result != 0:
self.fail(f"mypy failed: {stdout} {stderr}")
if __name__ == '__main__':
run_tests()

143
torch/testing/_internal/mypy_wrapper.py Executable file
View File

@ -0,0 +1,143 @@
#!/usr/bin/env python3
"""
This module serves two purposes:
- it holds the config_files function, which defines the set of subtests
for the test_run_mypy test in test/test_type_hints.py
- it can be run as a script (see the docstring of main below) and passed
the filename of any Python file in this repo, to typecheck that file
using only the subset of our mypy configs that apply to it
Since editors (e.g. VS Code) can be configured to use this wrapper
script in lieu of mypy itself, the idea is that this can be used to get
inline mypy results while developing, and have at least some degree of
assurance that those inline results match up with what you would get
from running the TestTypeHints test suite in CI.
See also these wiki pages:
- https://github.com/pytorch/pytorch/wiki/Guide-for-adding-type-annotations-to-PyTorch
- https://github.com/pytorch/pytorch/wiki/Lint-as-you-type
"""
import re
import sys
from configparser import ConfigParser
from itertools import chain
from pathlib import Path
from typing import List, Set
# don't import any files that live in the PyTorch repo, since this is
# meant to work as a standalone script
try:
import mypy.api
except ImportError:
# let test/test_type_hints.py import this even if mypy is absent
pass
def config_files() -> Set[str]:
"""
Return a set of the names of all the PyTorch mypy config files.
"""
return {
'mypy.ini',
'mypy-strict.ini',
}
def repo_root() -> Path:
"""
This script assumes that the cwd is the PyTorch repository root.
"""
return Path.cwd()
def glob(*, pattern: str, filename: str) -> bool:
"""
Return True iff the filename matches the (mypy ini) glob pattern.
"""
full_pattern = str(repo_root() / pattern)
path = Path(filename)
return any(
prefix.resolve().match(full_pattern)
for prefix in chain([path], path.parents)
)
def in_files(*, ini: str, py: str) -> bool:
"""
Return True iff the py file is included in the ini file's "files".
"""
config = ConfigParser()
config.read(repo_root() / ini)
return any(
glob(pattern=pattern, filename=py)
for pattern in re.split(r',\s*', config['mypy']['files'].strip())
)
def main(args: List[str]) -> None:
"""
Run mypy on one Python file using the correct config file(s).
This function assumes the following about its input:
- args is a valid list of CLI arguments that could be passed to mypy
- the last element of args is the path of a file to typecheck
- all the other args are config flags for mypy, rather than files
These assumptions hold, for instance, when mypy is run automatically
by VS Code's Python extension, so in your clone of this repository,
you could modify your .vscode/settings.json to look like this:
{
"python.linting.enabled": true,
"python.linting.mypyEnabled": true,
"python.linting.mypyPath":
"/path/to/pytorch/torch/testing/_internal/mypy_wrapper.py",
"python.linting.mypyArgs": [
"--show-column-numbers"
]
}
More generally, this should work for any editor that runs mypy on
one file at a time (setting the cwd to the repo root) and allows you
to set the path to the mypy executable.
"""
if not args:
sys.exit('The PyTorch mypy wrapper must be passed exactly one file.')
configs = [f for f in config_files() if in_files(ini=f, py=args[-1])]
mypy_results = [
mypy.api.run(
# insert right before args[-1] to avoid being overridden
# by existing flags in args[:-1]
args[:-1] + [
'--no-error-summary',
f'--config-file={config}',
args[-1],
]
)
for config in configs
]
mypy_issues = [
item
# assume stderr is empty
# https://github.com/python/mypy/issues/1051
for stdout, _, _ in mypy_results
for item in stdout.splitlines()
]
for issue in mypy_issues:
print(issue)
# assume all mypy exit codes are nonnegative
# https://github.com/python/mypy/issues/6003
sys.exit(max(
[exit_code for _, _, exit_code in mypy_results],
default=0,
))
if __name__ == '__main__':
main(sys.argv[1:])