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

[WIP/RFC] Improve selection of environments #836

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blueyed
Copy link
Collaborator

@blueyed blueyed commented Jun 23, 2018

@blueyed
Copy link
Collaborator Author

blueyed commented Jun 23, 2018

@davidhalter
This is WIP, but quite usable already.
You can use let g:jedi#environment_paths = [$HOME . '/.pyenv/versions]' to get all pyenv version in the completion for :JediUseEnvironment.. :) (a bit slow though!)

Using :JediUseEnvironment! … sets it for the current buffer.

@davidhalter
Copy link
Owner

BTW: I actually wanted to list the virtualenvs and system pythons in a buffer anyway so a user could choose. I think improved environment selection will be a great feature.

@@ -233,6 +242,14 @@ call jedi#init_python() " Might throw an error.
" ------------------------------------------------------------------------
" functions that call python code
" ------------------------------------------------------------------------
function! jedi#use_environment(...) abort
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to set_environment?

PythonJedi jedi_vim.set_environment(*vim.eval('a:000'))
endfunction

function! jedi#use_environment_for_buffer(env) abort
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.
There should also be setters (and getters) for tab- and window-local settings.

" files?!).
function! jedi#complete_environments(argl, cmdl, pos) abort
PythonJedi jedi_vim.complete_environments()
EOF
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove EOF.

@@ -48,6 +48,21 @@ endif
" Pyimport command
command! -nargs=1 -complete=custom,jedi#py_import_completions Pyimport :call jedi#py_import(<q-args>)

" Transfer bang into current buffer number.
" TODO: use window instead!?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above - should support global, tab, window and buffer.

@@ -158,42 +158,84 @@ def wrapper(*args, **kwargs):
return func_receiver


# XXX: might use lru cache, given that you can configure it per buffer?!
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably all the caching (and invalidation) should be done by Jedi.

current_environment = (None, None)


def set_environment(executable=None, bufnr=None):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs support for tab, window, and bufnr.
Could be scope.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While you can do that, I don't think this is what most users need/want. The scope most users want to use is probably global, but I'm not against it - I'm just saying that you're probably doing more work than necessary if you use a bufnr scope. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it is not really necessary, but a good example for getting the scoping right from the beginning.
More important would be a tab-local setting then I guess.
This is kind of useful in an IDE-like environment where you would work on different projects/venvs in parallel - but yes, it is a bit constructed.

vim_command("let g:jedi#use_environment = %r" % executable)
current_environment = (executable, environment)


def get_environment(use_cache=True):
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add scope here then, too. But it would default to preferring bufnr over window over tab over globals.

vim.command("return '%s'" % '\n'.join(
('%s (*)' if env.executable == current_executable else '%s') % (
env.executable
) for env in envs))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: display more information (after the name, similar to the currently selected one), e.g. the version.

@blueyed
Copy link
Collaborator Author

blueyed commented Jul 15, 2018

Updated to use davidhalter/jedi#1176.

@blueyed
Copy link
Collaborator Author

blueyed commented Jul 15, 2018

@davidhalter
Would appreciate some feedback on this early stage for the next iteration.

@blueyed blueyed changed the title [WIP] Improve selection of environments [WIP/RFC] Improve selection of environments Jul 15, 2018


def complete_environments():
envs = get_known_environments()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Especially since the latest changes we need to cache those, otherwise this is going to be very very slow.

@davidhalter
Copy link
Owner

I like the general direction of this PR. For me personally I have always envisioned a "list environments" feature where you could then choose which environment you would want to use. However this is a feature that might also be interesting for some people!

@codecov-io
Copy link

codecov-io commented Aug 6, 2018

Codecov Report

Merging #836 into master will decrease coverage by 1.56%.
The diff coverage is 17.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #836      +/-   ##
==========================================
- Coverage   47.68%   46.11%   -1.57%     
==========================================
  Files           3        3              
  Lines         799      798       -1     
  Branches      164      165       +1     
==========================================
- Hits          381      368      -13     
- Misses        384      396      +12     
  Partials       34       34
Flag Coverage Δ
#py27 45.23% <17.85%> (-0.56%) ⬇️
#py37 44.36% <17.85%> (-0.54%) ⬇️
#py38 ?
Impacted Files Coverage Δ
pythonx/jedi_vim.py 47.33% <17.85%> (-1.66%) ⬇️
test/test_integration.py 86.66% <0%> (-0.57%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2abec2...54f30f6. Read the comment docs.

# XXX: create_environment is not deterministic
# (https://github.com/davidhalter/jedi/issues/1155)
# jedi.create_environment(executable, safe=False)
environment = jedi.api.environment.Environment(executable)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for information, I had to comment this line and use environment = jedi.create_environment(executable, safe=False) instead to get it working.

I'm using python3.7 with pyenv.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fenkiou
Thanks for trying this.
Did you get an error before? ("'executable=%s is not supported: %s.'")
If so, what was the message?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope the message was this one:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/alex/.vim/repos/github.com/davidhalter/jedi-vim/pythonx/jedi_vim.py", line 177, in set_environment
    environment = jedi.api.environment.Environment(executable=executable)
TypeError: __init__() missing 1 required positional argument: 'path'

But in fact, I didn't run git submodule update when checking out your branch, everything is working correctly without any change! Sorry for the noise.

@petobens
Copy link
Contributor

petobens commented Dec 2, 2018

Hi @blueyed. I've recently started using pipenv to manage my python applications and was wondering if there is an easy way (either with this PR or with the current jedi/vim-jedi version) to have jedi automatically use a pipenv environment if one is available in the current directory. I was thinking of something along the lines:

let pipenv_venv_path = system('pipenv --venv')
if v:shell_error == 0
    let jedi_venv = trim(pipenv_venv_path)
else
   let jedi_venv = use_system_python
endif

that would triggered every time python buffer is read.

Do you think this is possible? If so, can you give a few pointers on how to implement it? Thanks!!

@blueyed
Copy link
Collaborator Author

blueyed commented Dec 2, 2018

system('pipenv --venv')

pipenv is slow in general, and system() should be avoided in general.
I suggest having your $PATH setup properly (as with pipenv --shell (IIRC) or through some other method).

@blueyed
Copy link
Collaborator Author

blueyed commented Dec 8, 2018

Rebased, without the updates to jedi/parso which I assume have landed already.

@petobens
Copy link
Contributor

petobens commented Dec 12, 2018

Anyway, have you tried this PR after all, which is supposed to make this simpler? (but also needs updating / more work)

I gave it a shot and it seems to work. My main problem (with your changes and with vim and venvs in general) is how to disable a virtual env:
i) Start vim/nvim
ii) Use JediUseEnvironment or something else to activate a virtual env.
iii) Run JediDebugInfo correctly shows paths to the activate venv
iv) Use unlet $VIRTUAL_ENV or vim-virtualev vim plugin to deactivate the virtualenv
v) Run JediDebugInfo again. -> Still reports path to the activated venv.

Can you reproduce that? Is there a workaround? Thanks! And sorry for the late reply.

@aliceh75
Copy link

Just tested this a little bit, seems to work fine (the patch didn't apply right away, but simple enough to fix). Thanks 😄 🎉 🥇

The autocompletion for JediUseEnvironment only looks in the path (which is fair enough :) ). One suggestion would be to allow this to be configured - ideally with some sort of wildcard system. I'm current using mkvirtualenv so my virtual environments are under ~/.virtualenvs/<project name>. If I used Pipenv they would be in ~/.local/share/virtualenvs/<project name> (I think). It's also not uncommon to have virtualenv within project folder (I've encountered both venv and .ve folder).

For anyone reading this - it's only the auto-completion I'm talking about. You can provide the correct path to JediUseEnvironment and it works as expected.

It's great as it is though, I could easily enough provide my own wrapper around JediUseEnvironment so I wouldn't let this suggestion delay a possible merge of this feature 😄

@blueyed
Copy link
Collaborator Author

blueyed commented Sep 23, 2019

@aliceh75
Thanks for review. I've rebased it now.

As for completing from certain paths there is g:jedi#environment_paths already, which (IIRC) should do what you want.
E.g. :let g:jedi#environment_paths = [expand('~/.pyenv/versions')] works for me to complete installed pyenv versions.

@blueyed
Copy link
Collaborator Author

blueyed commented Sep 23, 2019

@petobens

My main problem (with your changes and with vim and venvs in general) is how to disable a virtual env:
i) Start vim/nvim
ii) Use JediUseEnvironment or something else to activate a virtual env.
iii) Run JediDebugInfo correctly shows paths to the activate venv
iv) Use unlet $VIRTUAL_ENV or vim-virtualev vim plugin to deactivate the virtualenv
v) Run JediDebugInfo again. -> Still reports path to the activated venv.

Unletting $VIRTUAL_ENV will not have an impact on JediDebugInfo / the
env being used.

Mixing it with vim-virtualenv requires you to know what you're doing, but
in general should not have an impact on jedi-vim in this regard, apart from
when it would define $VIRTUAL_ENV that Jedi might pick up by default.

Can you reproduce that? Is there a workaround?

It should work OK when only sticking to :JediUseEnvironment, and using it
also after / instead of unlet $VIRTUAL_ENV/vim-virtualenv.

Maybe jedi-vim could be made smarter about handling $VIRTUAL_ENV (defined in Vim).
Currently only Jedi handles $VIRTUAL_ENV, and you might run into cache issues
there If I remember correctly - check the code at https://github.com/davidhalter/jedi/blob/master/jedi/api/environment.py.

Copy link
Owner

@davidhalter davidhalter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like the API here is definitely going into the right direction.

I've always had a feeling that we should add something like a quickfix window where you could select the "new" environment. get_known_environments will make that possible.

I feel like completing the environments might work, but it's a bit risky since it takes a lot of time. Not that I'm saying it's a bad idea to use completions, it's just slow (as I noted in a comment).

Otherwise good work. I definitely like where this is going!

return environment


def get_known_environments():
"""Get known Jedi environments."""
envs = list(jedi.api.environment.find_virtualenvs())
paths = vim_eval("g:jedi#environment_paths")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If paths is empty, it should probably be the current directory. Or does that not make sense?

@davidhalter
Copy link
Owner

@petobens Just FYI we won't support a combination of Jedi/vim-virtualenv. That will break in a lot of ways for sure. There's so many potential issues there. Also it will really not be needed once this stuff is finished.

@syphar
Copy link

syphar commented Dec 3, 2019

@blueyed having problems with jedi and virtualenvs too, just discovered this PR here.
Is this testable? I don't know jedi-code good enough to review that, but I could test it under my conditions :)

(in my case, completion with packages virtualenv works perfectly, but goto-definition not, $VIRTUAL_ENV is set correctly)

@blueyed
Copy link
Collaborator Author

blueyed commented Dec 3, 2019

@syphar sure, give it a try. (I've just rebased it on master)

@petobens
Copy link
Contributor

I'm been using this with my poetry/pipenv plugin petobens/poet-v#2 and it seems to work great so far. Thank you!!

@blueyed
Copy link
Collaborator Author

blueyed commented Jan 17, 2020

@petobens
Thanks for the feedback. I also think this should be ready, but IIRC needs a last round of polishing I guess.

@petobens
Copy link
Contributor

One minor question: is there a way to disable enviroment usage and point jedi to use the global python executable? The current way I'm doing it is :execute 'JediUseEnvironment ' . py3eval('sys.executable') but I don't know if a new more direct way could be added (something like JediDisableEnvironment)? Thanks

@davidhalter
Copy link
Owner

@petobens This is already the default, you don't have to do this.

Default environment selection changed quite a bit over the last year. At the moment it's pretty much what you are already doing.

@petobens
Copy link
Contributor

Ok. Thanks

@petobens
Copy link
Contributor

petobens commented Mar 8, 2020

@blueyed if this is not moving forward yet can you rebase on latest master? Thanks!!

@petobens
Copy link
Contributor

petobens commented Mar 8, 2020

Thank you!

@petobens
Copy link
Contributor

@blueyed Sorry for asking this again but can this branch be rebased on latest master? Also any idea/plans for this to be actually merged (i've been using it for a while now without any issues AFAICT)?

@petobens
Copy link
Contributor

Thanks!

@davidhalter davidhalter mentioned this pull request Aug 1, 2020
@davidhalter
Copy link
Owner

@blueyed I feel like this change is obsolete with my latest changes. I was heavily inspired by your changes here, but ultimately decided no to reuse the code for various reasons. I would therefore probably close it or do you think that there is still stuff in here that needs to be kept?

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.

7 participants