Skip to content

Allow to use custom python interpreter [fixes #339] #401

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

Conversation

st4lk
Copy link
Contributor

@st4lk st4lk commented Aug 8, 2018

Python interpreter or virtualenv can be specified in settings. Server can receive them from workspace/didChangeConfiguration request.

Tested with SublimeText 3 and LSP client plugin.
Example of Sublime LSP plugin settings:

{
  "clients": {
    "pyls":
    {
      "command": ["pyls"],
      "settings":
      {
          "pyls":
          {
              "python_virtualenv": "/path/to/virtualenv/",
              "python_interpreter": "/path/to/virtualenv/bin/python",
          }
      },
      ...
    }
  }
}

Example of Sublime Text 3 project settings (to specify virtualenv specific to current project):

// Project settings
{
    "folders":
    [

        {
            "path": "/path/to/my/project"
        }
    ],
    "settings":
    {
        "LSP":
        {
            "pyls":
            {
                "settings":
                {
                    "pyls":
                    {
                        "python_virtualenv": "/path/to/virtualenv/",
                        "python_interpreter": "/path/to/virtualenv/bin/python"
                    }
                }
            }
        }
    }
}

@palantirtech
Copy link
Member

Thanks for your interest in palantir/python-language-server, @st4lk! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@gatesn
Copy link
Contributor

gatesn commented Aug 26, 2018

Why can't your client just take the command ['/path/to/virtualenv/bin/python', '-m', 'pyls'] ?

@st4lk
Copy link
Contributor Author

st4lk commented Aug 26, 2018

@gatesn
Am I right, that this will require to install pyls in every virtualenv that I use?

@st4lk
Copy link
Contributor Author

st4lk commented Aug 26, 2018

Even if I'll install pyls in every virtualenv, looks like jedi will still use the system python if nor python_virtualenv, nor python_interpreter are used.

@st4lk
Copy link
Contributor Author

st4lk commented Aug 26, 2018

From jedi doc:

Tries to return an active Virtualenv. If there is no VIRTUAL_ENV variable set it will return the latest Python version installed on the system. This makes it possible to use as many new Python features as possible when using autocompletion and other functionality.

So by defualt, it will return system python or python from VIRTUAL_ENV env. I tried it, but jedi will accept virtualenv from VIRTUAL_ENV variable only if that env is a system env (owned by root).
Here is the code:
https://github.com/davidhalter/jedi/blob/master/jedi/api/environment.py#L345

So need to specify safe=False explicitly. It can be done in jedi.api.environment.create_environment call, not in jedi.api.environment.get_default_environment().

Another option is to take virtualenv path from VIRTUAL_ENV env variable and try to create it manually.
It will simplify the code. Let me update the PR.

@st4lk
Copy link
Contributor Author

st4lk commented Aug 26, 2018

Added a new commit - get virtualenv from VIRTUAL_ENV env variable.
The implementation will be simpler.

@st4lk
Copy link
Contributor Author

st4lk commented Aug 26, 2018

Why can't your client just take the command ['/path/to/virtualenv/bin/python', '-m', 'pyls'] ?

Just to be sure we are on the same page: my goal is to tell jedi, where virtualenv for my current project is. I don't want to install pyls in every project's virtualenv.

This is not about starting pyls from custom virtualenv (not from default's python). This can be achived by specifying path to pyls executable as you suggested.

@st4lk
Copy link
Contributor Author

st4lk commented Aug 26, 2018

In fact I'm not completely sure about VIRTUAL_ENV env var, this can be dangerous since it can be used by jedi in other places.
So I prefer approach from my first commit: 119c831

Your call

@sprocket-9
Copy link

@gatesn: @st4lk is correct we do not want to have to install pls in every venv, but have it installed separately from the Workspace's venv and have a way to alter the sys.path passed to Jedi to search the Workspace venv libs.

@st4lk I also prefer the approach in your first commit. The only thing is the non-public method you've used jedi.api.environment._get_python_prefix() isn't in Jedi master branch anymore so needs changed.

There's also PR #382 which uses an alternative approach to pass in an extraSysPath config option from the client list - I'm currently using pls with this PR merged and it works well.

However I do think the approach taken in this PR of passing in the venv path to pls and having Jedi work out the sys.path using its Environment API is a bit more elegant. It will also give clients an option to detect if they're running inside a venv and dynamically pass in the $VIRTUAL_ENV path for the current Workspace without needing to hardcode paths in config files.

But maybe there's a need for both approaches? @Kronuz do you think both PRs config options are needed?

@Kronuz
Copy link

Kronuz commented Aug 28, 2018

@spr0cketeer, each PR is for different things; I would thing both are needed.

@gatesn
Copy link
Contributor

gatesn commented Sep 9, 2018

How about we do something like this:

environment = next(jedi.api.environment.find_virtualenvs(safe=False), None)
if not environment:
    environment = jedi.api.environment.get_default_environment()

@st4lk st4lk force-pushed the allow-to-use-custom-python-interpreter-per-project branch from 90d9fe9 to 9551f5c Compare September 11, 2018 08:03
@st4lk
Copy link
Contributor Author

st4lk commented Sep 11, 2018

@gatesn
Looks like it will require to install pyls in every project's virtualenv. But this is what I'm trying to avoid.
I have a separate single virtualenv exclusively for pyls. And in my client (Sublime) I just paste a path to pyls executable in this isolated virtualenv.

With this patch, I don't need to install pyls into every project's virtualenv. I just need to define python_virtualenv in project's settings that will be sent to pyls. In that way, pyls can find correct virtualenv.

P.S. updated commit to use only python_virtualenv. Have removed python_interpreter for now, since _get_python_prefix is not in jedi master branch anymore, but that change is not released yet.

@st4lk st4lk force-pushed the allow-to-use-custom-python-interpreter-per-project branch from a6829dc to c8dbb18 Compare October 1, 2018 08:05
@st4lk st4lk force-pushed the allow-to-use-custom-python-interpreter-per-project branch from c8dbb18 to 26ca5a0 Compare December 16, 2018 23:33
@st4lk
Copy link
Contributor Author

st4lk commented Dec 16, 2018

@gatesn
Have updated PR: just need to update jedi dependency.

version >= 1.13.0 contains this fix: davidhalter/jedi@56bd795#diff-c204f420c3d90e6c005f6354c65dc638R161

Now, passing path to virtualenv of my project in VIRTUAL_ENV environment variable is enough.

@st4lk st4lk force-pushed the allow-to-use-custom-python-interpreter-per-project branch from 26ca5a0 to 008c432 Compare January 26, 2019 23:18
@st4lk st4lk force-pushed the allow-to-use-custom-python-interpreter-per-project branch from 008c432 to 8fe2e0b Compare January 26, 2019 23:22
@st4lk
Copy link
Contributor Author

st4lk commented Feb 10, 2019

Jedi version was updated by another commit: 50d03d5
So feature should work, closing this PR.

@st4lk st4lk closed this Feb 10, 2019
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.

5 participants