Move test_utils.py back to MYPY #113745
Move test_utils.py back to MYPY #113745
Conversation
Since MYPYNOFOLLOW is about to turn on import following, there's no reason to keep test_utils.py in the MYPYNOFOLLOW config. Moreover, I'm not sure it still takes 10 minutes to typecheck this file; adding it to the MYPY config takes `lintrunner --take MYPY --all-files` from 53s to 57s on my machine, which is substantial but not horrible. I guess we'll see how it fares on CI. (Note that we cannot simply merge MYPY and MYPYNOFOLLOW because the latter config turns on `disallow_any_generics` and so is in that sense stricter than the MYPY config.) [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/113745
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit 7d3a33b with merge base 6f44090 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
Since MYPYNOFOLLOW is about to turn on import following, there's no reason to keep test_utils.py in the MYPYNOFOLLOW config. Moreover, I'm not sure it still takes 10 minutes to typecheck this file; adding it to the MYPY config takes `lintrunner --take MYPY --all-files` from 53s to 57s on my machine, which is substantial but not horrible. I guess we'll see how it fares on CI. (Note that we cannot simply merge MYPY and MYPYNOFOLLOW because the latter config turns on `disallow_any_generics` and so is in that sense stricter than the MYPY config.) ghstack-source-id: 0a9b3a8 Pull Request resolved: #113745
|
Doesn't look like the lintrunner job on CI is taking longer than usual |
| files = | ||
| torch/_dynamo, | ||
| test/test_utils.py | ||
| torch/_inductor |
There was a problem hiding this comment.
Sorry, why did inductor get added here?
There was a problem hiding this comment.
ah I just snuck this one-line change along the way. MYPYNOFOLLOW is already typechecking all the inductor files in the .lintrunner.toml config, so I figured I should add it here as well. tbh I'm not sure when this files config setting is used directly since lintrunner passes in an explicit list of files to typecheck. (The docs for files says it's "A comma-separated list of paths which should be checked by mypy if none are given on the command line.")
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
A missing comma in the file list currently leads to errors when running mypy, introduced in #113745 Fixes #133096 Pull Request resolved: #133097 Approved by: https://github.com/Skylion007, https://github.com/malfet
3 participants
Stack from ghstack (oldest at bottom):
Since MYPYNOFOLLOW is about to turn on import following, there's no
reason to keep test_utils.py in the MYPYNOFOLLOW config. Moreover, I'm
not sure it still takes 10 minutes to typecheck this file; adding it to
the MYPY config takes
lintrunner --take MYPY --all-filesfrom 53s to57s on my machine, which is substantial but not horrible. I guess we'll
see how it fares on CI.
(Note that we cannot simply merge MYPY and MYPYNOFOLLOW because the
latter config turns on
disallow_any_genericsand so is in that sensestricter than the MYPY config.)