Make ModuleFinder try identifying non-PEP 561 packages (#8238)#28
Merged
sthagen merged 1 commit intosthagen:masterfrom Feb 24, 2020
Merged
Make ModuleFinder try identifying non-PEP 561 packages (#8238)#28sthagen merged 1 commit intosthagen:masterfrom
sthagen merged 1 commit intosthagen:masterfrom
Conversation
This pull request is an attempt at mitigating #4542 by making mypy report a custom error when it detects installed packages that are not PEP 561 compliant. (I don't think it resolves it though -- I've come to the conclusion that import handling is just inherently complex/spooky. So if you were in a cynical mode, you could perhaps argue the issue is just fundamentally unresolvable...) But anyways, this PR: 1. Removes the hard-coded list of "popular third party libraries" from `moduleinfo.py` and replaces it with a heuristic that tries to find when an import "plausibly matches" some directory or Python file while we search for packages containing ``py.typed``. If we do find a plausible match, we generate an error that looks something like this: ``` test.py:1: error: Skipping analyzing 'scipy': found module but no type hints or library stubs test.py:1: note: See https://mypy.readthedocs.io/en/latest/running_mypy.html#missing-imports ``` The heuristic I'm using obviously isn't foolproof since we don't have any obvious signifiers like the ``py.typed`` file we can look for, but it seemed to work well enough when I tried testing in on some of the libraries in the old list. Hopefully this will result in less confusion when users use a mix of "popular" and "unpopular" libraries. 2. Gives us a way to add more fine-grained "module not found" error messages and heuristics in the future: we can add more entries to the ModuleNotFoundReason enum. 3. Updates the docs about missing imports to use these new errors. I added a new subsection per each error type to try and make things a little less unwieldy. 4. Adds what I think are common points of confusion to the doc -- e.g. that missing imports are inferred to be of type Any, what exactly it means to add a `# type: ignore`, and the whole virtualenv confusion thing. 5. Modifies the docs to more strongly discourage the use of MYPYPATH. Unless I'm wrong, it's not a feature most people will find useful. One limitation of this PR is that I added tests for just ModuleFinder. I didn't want to dive into modifying our testcases framework to support adding custom site-packages/some moral equivalent -- and my PR only changes the behavior of ModuleFinder when it would have originally reported something was not found, anyways.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request is an attempt at mitigating
python#4542 by making
mypy report a custom error when it detects installed
packages that are not PEP 561 compliant.
(I don't think it resolves it though -- I've come to
the conclusion that import handling is just inherently
complex/spooky. So if you were in a cynical mode, you
could perhaps argue the issue is just fundamentally
unresolvable...)
But anyways, this PR:
Removes the hard-coded list of "popular third party
libraries" from
moduleinfo.pyand replaces it witha heuristic that tries to find when an import "plausibly
matches" some directory or Python file while we search
for packages containing
py.typed.If we do find a plausible match, we generate an error
that looks something like this:
The heuristic I'm using obviously isn't foolproof since
we don't have any obvious signifiers like the
py.typedfile we can look for, but it seemed to work well enough
when I tried testing in on some of the libraries in the old list.
Hopefully this will result in less confusion when users
use a mix of "popular" and "unpopular" libraries.
Gives us a way to add more fine-grained "module not found"
error messages and heuristics in the future: we can add
more entries to the ModuleNotFoundReason enum.
Updates the docs about missing imports to use these new
errors. I added a new subsection per each error type to
try and make things a little less unwieldy.
Adds what I think are common points of confusion to the
doc -- e.g. that missing imports are inferred to be of
type Any, what exactly it means to add a
# type: ignore,and the whole virtualenv confusion thing.
Modifies the docs to more strongly discourage the use
of MYPYPATH. Unless I'm wrong, it's not a feature most
people will find useful.
One limitation of this PR is that I added tests for just ModuleFinder.
I didn't want to dive into modifying our testcases framework to support
adding custom site-packages/some moral equivalent -- and my PR
only changes the behavior of ModuleFinder when it would have originally
reported something was not found, anyways.