Skip to content
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

Make ModuleFinder try identifying non-PEP 561 packages (#8238) #28

Merged
merged 1 commit into from
Feb 24, 2020

Conversation

sthagen
Copy link
Owner

@sthagen sthagen commented Feb 24, 2020

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:

  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 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.
@sthagen sthagen merged commit 2db6233 into sthagen:master Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants