-
-
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
Import handling is confusing and misleading #4542
Comments
Sorry you are having such a bad experience. Maybe PEP 561 will (eventually) make things better. And maybe we can improve the error messages. In the meantime, in order to understand your predicament better, can you link to the contents of census.py, and explain where the things mypy couldn't find are installed? |
census.py is here: https://github.com/edx/openedx-census/blob/4b242cc72896db002d9818a06bcdebdcdd6f8480/census.py All of the imports mentioned in the error messages above are installed in the same virtualenv that mypy is installed into. Simply running "import aiohttp" succeeds, for example. |
So for If you have a habit of splitting your 1st party project into multiple packages that are installed separately (IIUC that's what you do at Edx) that's also a bit of a problem for mypy compared to the workflow where most code is in a unirepo (like we do at Dropbox, and IIUC that's also how Instagram and other large current mypy users do it), since with the unirepo approach you only get this problem for 3rd party packages, which are usually in typeshed. This is also what |
You mention missing stubs. Perhaps this is the heart of the matter? The message says "missing imports", and what it means is, "missing stubs"? Or, I read it as trying to import Python, but it's trying to import stubs? Adding a few more words could clarify things. FWIW: Although this code is in the edx org and about Open edX, it is a small-ish standalone program. |
Not quite standalone (opaque_keys seems to be an edx package). Maybe "missing imports" is indeed not the right term. When mypy sees PS. People tend to counter "why doesn't it use $PYTHONPATH or sys.path" but that really doesn't work because the majority of code on sys.path is stdlib code which has no annotations and triggers complaints about all sorts of edge cases. |
I've just started playing around with mypy and run into this same confusion. I think that something like "missing type definitions or type-annotated code for module I initially tried to get my environment to find the code, but what I actually need to do is provide type definitions for these modules in some way. |
The flow project from facebook has a similar scope to mypy - i'm not sure whether or not I had a similar experience when beginning to use that project, but their docs section on how to work with library definitions seems relevant here: https://flow.org/en/docs/libdefs/ |
I think we could wait a bit until PEP 561 is implemented, then we can design a better error message that will better detect the situation and proposes possible solutions (with a link to the docs, like we do for a common error with invariant collections). @ethanhs what do you think? |
Yes, I think once #4693 is merged it would be good to take a look at updating and improving the missing module messages. Especially considering some of them (such as those that reference typeshed) may be misleading after PEP 561 packaging becomes the norm. |
I was quite confused by this too (at first I thought it was a virtualenv thing). Would folks be open to a page in the docs that has a separate section (for easier searchability) that looks like:
Also, perhaps changing the error message from:
to
would make things clearer. "Cannot find module" is very similar to the |
How do you propose mypy should tell the difference between a module without a stub and a typo? It does not use the same module search path as the Python interpreter. For known standard library modules and for well-known third-party modules it does produce a different error message -- see the logic at Lines 2059 to 2072 in 6784d85
It's probably true that we need to improve the docs explaining how mypy finds modules though -- clearly this is a frequent source of confusion. (Many people assume mistakenly that mypy loads everything from sys.path.) |
Would you be open to outputting a doc link that goes directly to a page
explaining how all of this works? (Happy to write up something if so)
Pandas does this for setting vs copy and I think it’s an elegant way to
handle a confusing issue :)
My gut instinct when I saw that error was to repeatedly try to get mypy to
run within the virtualenv or try to set it up to point at the right
PYTHONPATH. (Now I totally get it but not really until I read this github
issue)
Would it be possible to check if the module is importable and generate a
different error? (Maybe this violates static typing) :)
…On Sun, Apr 8, 2018 at 9:09 PM Guido van Rossum ***@***.***> wrote:
How do you propose mypy should tell the difference between a module
without a stub and a typo? It does not use the same module search path as
the Python interpreter. For known standard library modules and for
well-known third-party modules it does produce a different error message --
see the logic at
https://github.com/python/mypy/blob/6784d85a084ff1d782fe5b3d6ab1331d27d49791/mypy/build.py#L2059-L2072.
Perhaps you can provide an update to the list of well-known third-party
modules? (It is in
https://github.com/python/mypy/blob/master/mypy/moduleinfo.py.)
It's probably true that we need to improve the docs explaining how mypy
finds modules though -- clearly this is a frequent source of confusion.
(Many people assume mistakenly that mypy loads everything from sys.path.)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#4542 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABhjqycjzKO8IS4pHtvWr8i_NLSpu6Ygks5tmt7-gaJpZM4R5VzC>
.
|
I wonder if producing a different error message for known third-party packages is actually more confusing… Such a list of packages will inevitably be incomplete, and it's not clear as an end user why some of your dependencies are being treated differently to others. |
I'm fine with doing all that but please submit a PR that both adds some
docs and adds a note to the error message that links to it. And please test
it as well as you can.
(Your gut instinct was definitely wrong, so we clearly need to do some user
education here. :-)
|
How do I contribute a .stub file?
|
@Lewiscowles1986 You will get better help from Gitter (where I see you already checked in). |
This issue seems to be a constant source of confusion and frustration for many starting out with mypy (judging by number of issues about this here and seeing this also in our team). To me that clearly means the error message is less than it could be.
Shouldn't checking if the module is importable (in the Python sense) be enough to make this distinction? If it is the type annotations are missing, if not it's either a typo or a missing package. And even if the above is for some technical reason not easily implementable at least changing the error message to mention the possibility that it's the type definitions that can't be found would be a nice usability improvement. |
I invite you to come up with a PR that implements this idea (there is indeed a technical reason why "checking if it is importable" is not a reasonable thing to try). |
And this is kind of normal. Although some people call mypy a linter, it is much closer to a compiler.
IMO this is a bad idea. Importing modules can have side effects (and sometimes bad ones).
What exactly do you propose to change? Currently mypy gives dedicated errors for known packages:
and a link to docs for unknown ones:
|
Didn't notice @gvanrossum already replied. Essentially I agree, improving error messages is the only thing we (including you) can do. |
I'm interested in improving the error messages and documentation here. Does mypy currently distinguish between:
If not, is it feasible to make such a distinction? |
Mypy currently doesn't distinguish between modules that exist (in site-packages) without a py.typed marker and code that doesn't exist at all. The relevant search is in mypy/modulefinder.py (but I am aware that the logic there is pretty hard to follow). Mypy does have a large list of "known popular modules" in mypy/moduleinfo.py -- maybe that list also needs updating? |
I guess part of the problem is that |
Showing this note for every error may be too noisy, potentially we can show it e.g. once in a file. |
What about making this a blocking error -- it would be prominent that way. |
But I thought blocking errors can't be |
Yeah. mypy has terrible UX about importing modules.
+100500 If I can use modules then they are not missing. They may be not have stubs or something but at the same time I see both messages: |
Marked this a high priority since this seems to affect many users, and there is some low-hanging fruit that could improve the situation. |
It can be difficult to understand the missing module error message at times. Changes > Cannot find module named '{}' to > Cannot find implementation or library stub for module named '{}' This improves #4542.
If this list is here to stay (I haven't checked the code yet and I am not sure what impact #7698 have on this list). Would it be possible to have a way to specify "locally popular modules" in the configuration file? eg: I work with Tryton, which is not a very popular. But the vast majority of code I import will have some code in the |
@nicoe that seems unlikely to be terribly helpful; all that the list is used for, as far as I can tell, is deciding whether to say
or
|
To better understand your use case, can you give an example of a scenario where this would be helpful? If it's locally popular, you could perhaps use |
I came here through issue #7788 and indeed after testing stubgen it works. The minor annoyance is that I'll have to do it for every virtualenv but I can live with it. |
@nicoe maybe you want something like [mypy-sql]
ignore_missing_imports = True in your |
I had trouble understanding how mypy works with third party packages as well. I just spent something like 40 minutes testing things and reading issues and the documentation to understand. My thoughts and actions when trying mypy the first time:
Maybe this last docs section should be added in a section of the README titled So, if I got it right, I should just open a PR on Well, I just looked and this file is already there! So I'm lost again... I finally solved that by setting MYPYPATH environment variable to the EDIT: I was tired. Mypy was not installed in the virtualenv. It's working as expected, without having to add site-packages to MYPYPATH. Clearly a "did you try to turn it off an on again" case. But anyway, I still learned some things 🙂 Thank you @The-Compiler and @ethanhs for your help. |
@pawamoy As far as I understand, having a |
@The-Compiler of course, but in my case, The TL;DR is:
EDIT: don't add your site-packages to MYPYPATH. This is not desirable as mypy will pick every package, even the non-typed ones, generating errors. |
Ah, indeed, I missed that bit in your comment. Yeah, as soon as fastapi is reasonably sure its type annotations are correct/stable, it should probably ship a |
@pawamoy you should never add site-packages to your mypy path. If there is a package that is PEP 561 compatible in the venv you should be able to use it. Is mypy globally installed? If so you may need to pass the venv Python executable to mypy. |
@pawamoy you can chat on gitter and get help there in real time (https://gitter.im/python/typing) |
Edited my comments accordingly, thanks @ethanhs |
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.
I was looking at python#4542, an old issue that I think we've mostly addressed, but nonetheless has an alarming number of likes. It occurs to me that front and centre we mention an issue that most people never run into in practice. I also pulled the details of this. mypy's hardcoded list of stdlib modules is incomplete, but here are the ones on the list that we do not have stubs for (I only checked Python 3): ``` {'__dummy_stdlib1', '__dummy_stdlib2', 'ossaudiodev', 'sqlite3.dump', 'turtledemo', 'xml.dom.expatbuilder', 'xml.dom.minicompat', 'xml.dom.xmlbuilder', 'xml.sax._exceptions', 'xml.sax.expatreader', 'xxlimited'} ``` We should maybe consider getting rid of the hardcoded list altogether.
I was looking at #4542, an old issue that I think we've mostly addressed, but nonetheless has an alarming number of likes. It occurs to me that front and centre we mention an issue that most people never run into in practice. I also pulled the details of this. mypy's hardcoded list of stdlib modules is incomplete, but here are the ones on the list that we do not have stubs for (I only checked Python 3): ``` {'__dummy_stdlib1', '__dummy_stdlib2', 'ossaudiodev', 'sqlite3.dump', 'turtledemo', 'xml.dom.expatbuilder', 'xml.dom.minicompat', 'xml.dom.xmlbuilder', 'xml.sax._exceptions', 'xml.sax.expatreader', 'xxlimited'} ``` We should maybe consider getting rid of the hardcoded list altogether. Co-authored-by: hauntsaninja <>
I think #8238 does a good job of improving the situation here, so I'm tentatively going to close this issue. As always, if you see ways for the error messages to be improved, let us know. |
I tried mypy and was alarmed to see it unable to find imports that are clearly available. Reading the explanations in #1293 and #1339, I now understand why it doesn't merrily parse third-party code that will only add to a growing pile of unannotated code.
But this is a really bad first experience. I install something that claims to understand Python code, and it's confused by imports? Then it gives me two suggestions, neither of which is seems like the right advice:
I shouldn't try to set MYPYPATH, that's silly busy-work that if successful will only lead me into the unannotated third-party module trap. The imports aren't missing, so why would
--ignore-missing-imports
help? NOTE: using--ignore-missing-imports
silences the warnings, but the option is misnamed: the imports are not missing.Reading the help, I see
--follow-imports
, so I try those options. I try--follow-imports=silent
, but that still complains about the imports. I try--follow-imports=skip
, and that also still complains about the imports. Both of these sound like exactly what I need, and neither changes the behavior much:The text was updated successfully, but these errors were encountered: