mypy-pycharm
mypy-pycharm copied to clipboard
Respect custom mypy path when checking for mypy availability
First time contributor checklist
- [x] I have read how to contribute to this project
- [x] I have read the code of conduct to this project
Contributor checklist
- [x] I am targeting the
master
branch (and not therelease
branch) - [x] I am using the provided codeStyleConfig.xml
- [x] I have tested my contribution on these versions of PyCharm/IDEA:
- PyCharm Community 2021.3.1
- IntelliJ IDEA 2021.3.2
- [x] My contribution is fully baked and ready to be merged as is
- [x] I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit
using the
Fixes #1234
syntax
Description
- Function checkMypyAvailable is called from various places in the code. If user specifies a custom mypy executable, which is not part of project's venv, then this function will return false when called with showNotifications == true. This happens because the function checks for mypy presence in the project's venv and it doesn't respect the custom path. This change makes the function check for mypy in the project's venv only if no custom path is present.
- Additional note. It may be better to check for mypy presence in the current venv after isMypyPathValid call. Thus, a user would get two notifications: mypy is not available, click here to install it.
Type of Changes
Type | |
---|---|
✓ | :bug: Bug fix |
:sparkles: New feature | |
:hammer: Refactoring | |
:scroll: Docs |
Related Issue
Closes #98
Hi, thanks for the PR.
I had a bit of time and tried this out. I couldn't make it work though, for some reason the plugin gives me a notice "executable ... not found" for any path I try, even though the path obviously exists. Possibly PEBKAC, I will try again when I have more time.
Thanks for looking at it. I will double check that it works and will try to also do it under Linux (I mainly use Windows).
@intgr , I got some time to look into it. The problem is in Linux system. Kind of. ;) I was able to reproduce the problem from a terminal:
- If I simply run
/usr/bin/mypy -V
, then it works. - If I do
/something/something/venv/bin/python /usr/bin/mypy -V
, then it doesn't work.python
here is inside a virtual environment without mypy in it.
There is this code in the plugin: https://github.com/leinardi/mypy-pycharm/blob/26299d03651b45f8721ea36b8d538be7a020b501/src/main/java/com/leinardi/pycharm/mypy/mpapi/MypyRunner.java#L369 in function called getMypyCommandLine
. It only checks if an executable is a windows executable. On Linux it calls mypy as in line 2 from above instead of line 1. It could be fixed with simply checking if file is executable regardless of OS. Although, there was probably a reason why it is checked only under Windows. And checking if something is executable under Linux may be ambiguous.
I think that the whole process of executing mypy should be simplified:
- If user didn't provide a custom path, do what is done today: check current python environment, auto-detect system-wide mypy.
- If user provided a custom path, try to use that path directly. If it doesn't work, try to use it as
current_python custom_mypy_path
.
Either way, add --python-executable
argument to mypy, which would point to project's interpreter. This will ensure correct packages could be imported by mypy.
I agree, the isWindowsExecutable
check is suspicious. It was apparently added in #16 to fix bug #15. Apparently in Windows, mypy's entry point mypy.exe
is a binary file rather than a Python script.
The issue I had with 'executable "/usr/local/bin/mypy" not found' on macOS https://github.com/leinardi/mypy-pycharm/pull/99#issuecomment-1067303500 seems to stem from the same root cause as #15:
2022-03-26 14:49:53,294 [ 245192] INFO - .pycharm.mypy.mpapi.MypyRunner - Detected Mypy path: /usr/local/bin/mypy
2022-03-26 14:49:53,387 [ 245285] INFO - .pycharm.mypy.mpapi.MypyRunner - Command Line string: /usr/local/bin/python3.9 /usr/local/bin/mypy -V
2022-03-26 14:49:53,387 [ 245285] WARN - .pycharm.mypy.mpapi.MypyRunner - Error while checking Mypy path: Traceback (most recent call last):
File "/usr/local/bin/mypy", line 5, in <module>
from mypy.__main__ import console_entry
ModuleNotFoundError: No module named 'mypy'
Peering into /usr/local/bin/mypy
gives a clue: a globally installed mypy
script itself is supposed to know the correct Python interpreter:
#!/usr/local/Cellar/mypy/0.940/libexec/bin/python3.10
But this plugin tries to invoke it with the project's interpreter, which does not have mypy installed locally.
I think that the whole process of executing mypy should be simplified
You're on the right track. The plugin should distinguish between two different ways how mypy can be installed:
- If mypy is installed in a virtualenv, invoke mypy via the virtualenv's interpreter:
$VIRTUAL_ENV/bin/python $VIRTUALENV/bin/mypy ...
- If mypy is installed globally, invoke the mypy binary directly without specifying a Python interpreter. Similar to the
isWindowsExecutable
condition, but should behave the same on all OSes.
So the question is, how to distinguish between (1) and (2). Perhaps we can get away with assuming that when a custom path is specified, it's referring to a global mypy installation?
Or make some attempt to detect whether the specified path is a virtualenv path? Seems fragile to me. But it sounds like a legitimate use case that someone may want to have a special virtualenv that contains mypy, and use that in multiple projects.
Perhaps the settings UI should be redesigned to offer three options:
- Specify globally installed mypy by path, executed as (2) (e.g. my use case with
/usr/local/bin/mypy
) - Dropdown to select between virtual envs configured in PyCharm, these would be executed as (1)
- (Default) Auto-detect: if the project's virtualenv contains
mypy
run it as (1), otherwise try to detect a globalmypy
installation inPATH
environment variable and run it as (2).
Either way, add --python-executable argument to mypy, which would point to project's interpreter
Agreed, this is a good idea. But it solves a different problem and should probably be a separate pull request.
Closing this since it seems to be addressed by #111