-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Fix search paths for -p
#9387
Fix search paths for -p
#9387
Conversation
e97f83e
to
492f011
Compare
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.
This looks like it should work, but I'd like a small refactoring to reduce the code duplication.
Also, please add a test. Instructions in test-data/unit/README.md
. You could add a test to cmdline.test
.
(In the future, please don't amend or squash commits. Code reviewers prefer to see changes resulting from code review separate from the original changes. We'll squash when we land.) |
Sorry, I'm used to code review tools that gracefully handle force pushes and can show diffs between successive updates... I will avoid force pushes here in the future. |
`-p` is similar to `-m` except that it will recursively typecheck submodules. Unfortunately the early module walk was being done with search paths that did not benefit from site_packages for the selected python executable, which resulted in some packages being typechecked fine with `-m`, but rejected as `not found` by `-p`. Fixes python#9386
The framework for cmdline.test explicitly disables site-packages. I'm modifying the pep561 test suite instead, since it already provisions a typed pkg in a venv. |
Thanks for this contribution! |
-p
is similar to-m
except that it will recursively typecheck submodules.Unfortunately the early module walk was being done with search paths that
did not benefit from site_packages for the selected python executable,
which resulted in some packages being typechecked fine with
-m
, butrejected as
not found
by-p
.Fixes #9386