-
Notifications
You must be signed in to change notification settings - Fork 30
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
Conversation
* 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
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:
There is this code in the plugin:
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:
Either way, add |
I agree, the 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:
Peering into
But this plugin tries to invoke it with the project's interpreter, which does not have mypy installed locally. |
You're on the right track. The plugin should distinguish between two different ways how mypy can be installed:
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:
|
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 |
First time contributor checklist
Contributor checklist
master
branch (and not therelease
branch)using the
Fixes #1234
syntaxDescription
Type of Changes
Related Issue
Closes #98