-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
find_sources: find build sources recursively #9614
Conversation
Fixes python#8548 This is important to fix because it's a really common source of surprising false negatives. I also lay some groundwork for supporting passing in namespace packages on the command line. The approach towards namespace packages that this anticipates is that if you pass in files to mypy and you want mypy to understand your namespace packages, you'll need to specify explicit package roots. We also make many fewer calls to crawl functions, since we just pass around what we need.
Fixes part of python#5759 The other part is passing files arguments, which we make steps towards in python#9614
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.
Thanks for fixing this bug! This was pretty bad. Looks good -- my only comments are about additional test cases.
Additional thanks for cleaning up some of the code, even if it wasn't needed for this PR.
@@ -68,6 +81,7 @@ undef | |||
undef | |||
[out] | |||
pkg/a.py:1: error: Name 'undef' is not defined | |||
pkg/subdir/a.py:1: error: Name 'undef' is not defined |
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.
Test importing from modules found via recursive directory traversal. Test cases where the directory contains __init__.py
and where it doesn't exist.
Maybe test this also with --namespace-packages
?
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.
testCmdlinePackageSlash tests when the directory contains init.py and testCmdlinePackageContainingSubdir tests when it doesn't.
I added importing of the modules.
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.
There currently isn't an interaction with --namespace-packages
and cmdline tests are slow, so I won't do that unless told.
That said, I have another diff coming soon where I flesh out the package root code and make --namespace-packages
the default. All these tests pass with that.
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'll also update the docs once all these changes (essentially important bug fixes + first three points of #8584) are shipped :-)
This should cover the current state on master, as implemented across python#9742, python#9683, python#9632, python#9616, python#9614, etc. This will need to be changed if we can make `--namespace-packages` the default (python#9636).
This should cover the current state on master, as previously discussed / implemented across python#9742, python#9683, python#9632, python#9616, python#9614, etc. This will need to be changed if we can make `--namespace-packages` the default (python#9636). I haven't documented some of the finer points of the changes, since it felt like an inappropriate level of detail (e.g. using absolute paths when crawling, how directories with invalid package names affect crawling, etc)
This should cover the current state on master, as previously discussed / implemented across python#9742, python#9683, python#9632, python#9616, python#9614, etc. This will need to be changed if we can make `--namespace-packages` the default (python#9636). I haven't documented some of the finer points of the changes, since it felt like an inappropriate level of detail (e.g. using absolute paths when crawling, how directories with invalid package names affect crawling, etc)
This should cover the current state on master, as previously discussed / implemented across #9742, #9683, #9632, #9616, #9614, etc. This will need to be changed if we can make `--namespace-packages` the default (#9636). I haven't documented some of the finer points of the changes, since it felt like an inappropriate level of detail (e.g. using absolute paths when crawling, how directories with invalid package names affect crawling, etc) Co-authored-by: hauntsaninja <>
This should cover the current state on master, as previously discussed / implemented across #9742, #9683, #9632, #9616, #9614, etc. This will need to be changed if we can make `--namespace-packages` the default (#9636). I haven't documented some of the finer points of the changes, since it felt like an inappropriate level of detail (e.g. using absolute paths when crawling, how directories with invalid package names affect crawling, etc) Co-authored-by: hauntsaninja <>
With python/mypy#9614, mypy now tries to collect all files in tests/, but it fails due to: tests/end2end/conftest.py: error: Duplicate module named 'conftest' (also at 'tests/end2end/features/conftest.py') tests/end2end/conftest.py: error: Are you missing an __init__.py? [misc] We should probably add __init__.py files to tests/ at some point... See #6059 and #5249
Ugh, turns out many users were relying on the old behavior and this causes issues because mypy now finds unexpected Python files from For example, creating an empty virtualenv with Python 3.9 breaks mypy:
I think this is a big enough regression that this change should be reverted in a hotfix release, until a longer-term solution can be figured out. |
Fixes #8548
This is important to fix because it's a really common source of
surprising false negatives.
I also lay some groundwork for supporting passing in namespace packages
on the command line. The approach towards namespace packages that this
anticipates is that if you pass in files to mypy and you want mypy to
understand your namespace packages, you'll need to specify explicit
package roots.
We also make many fewer calls to crawl functions, since we just pass
around what we need.