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

Respect custom mypy path when checking for mypy availability #99

Closed

Conversation

fralik
Copy link

@fralik fralik commented Mar 14, 2022

First time contributor checklist

Contributor checklist

  • I am targeting the master branch (and not the release branch)
  • I am using the provided codeStyleConfig.xml
  • I have tested my contribution on these versions of PyCharm/IDEA:
  • PyCharm Community 2021.3.1
  • IntelliJ IDEA 2021.3.2
  • My contribution is fully baked and ready to be merged as is
  • 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 fix
✨ New feature
🔨 Refactoring
📜 Docs

Related Issue

Closes #98

* 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.

Fixes leinardi#98
@intgr
Copy link
Collaborator

intgr commented Mar 14, 2022

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.

image

@fralik
Copy link
Author

fralik commented Mar 15, 2022

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).

@fralik
Copy link
Author

fralik commented Mar 23, 2022

@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:

  1. If I simply run /usr/bin/mypy -V, then it works.
  2. 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:

if (interpreterFile == null || FileTypes.isWindowsExecutable(mypyPath)) {
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:

  1. If user didn't provide a custom path, do what is done today: check current python environment, auto-detect system-wide mypy.
  2. 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.

@intgr
Copy link
Collaborator

intgr commented Mar 26, 2022

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 #99 (comment) 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.

@intgr
Copy link
Collaborator

intgr commented Mar 26, 2022

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:

  1. If mypy is installed in a virtualenv, invoke mypy via the virtualenv's interpreter: $VIRTUAL_ENV/bin/python $VIRTUALENV/bin/mypy ...
  2. 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 global mypy installation in PATH environment variable and run it as (2).

@intgr
Copy link
Collaborator

intgr commented Mar 26, 2022

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.

@leinardi
Copy link
Owner

leinardi commented Sep 2, 2023

Closing this since it seems to be addressed by #111

@leinardi leinardi closed this Sep 2, 2023
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.

Mypy executable is reported missing despite successful test
3 participants