cjrh/vim-conda

Restrict the changes to `sys.path` to jedi-vim only

cjrh opened this issue · 2 comments

cjrh commented

See this.

The current behaviour is bad because sys.path gets modified globally for the Python interpreter. We must not modify sys.path itself, rather just provide the extras to jedi. This might require a jedi patch.

cjrh commented

In the latest jedi code, see /jedi/evaluate/sys_path.py at the function get_sys_path(). We could either "pretend" to be a VIRTUAL_ENV (in order to be picked up here) or monkey-patch the get_sys_path() function. Or submit a jedi patch.

cjrh commented

Ok, so it turns out that I misread the jedi-vim code. In fact, jedi-vim does really modify the sys.path of the embedded Python...which is what I am doing currently in vim-conda.

There are other issues. In /jedi/evaluate/sys_path.py, the get_sys_path() function looks like this:

def get_sys_path():
    def check_virtual_env(sys_path):
        """ Add virtualenv's site-packages to the `sys.path`."""
        venv = os.getenv('VIRTUAL_ENV')
        if not venv:
            return
        venv = os.path.abspath(venv)
        p = os.path.join(
            venv, 'lib', 'python%d.%d' % sys.version_info[:2], 'site-packages')
        sys_path.insert(0, p)

    check_virtual_env(sys.path)
    return [p for p in sys.path if p != ""]

The problem I've observed is that this function gets called a bunch of times while editing code. After one lookup for a completion, the embedded Python's sys.path looks like this:

image

Lots and lots of the same path getting .insert(0, p)ed. vim-conda does not do this: I am carefully managing the modification of sys.path at the moment. I would think that jedi-vim should operate lookups on a copy of sys.path rather than modify the actual one in the embedded vim-python (I wrongly assumed this was how it worked) but I don't yet know enough about how Jedi works to make a definitive statement about this yet.

I was working on a patch for jedi to incorporate a check for the Conda-specific environment variables, completely analogous to the VIRTUALENV version, but I think I'll hold off for now. The patch looked something like this:

image

Now that I see that jedi-vim modifies sys.path anyway, I'm going to go ahead and stick with my current design. Also, jedi-vim does not have to be modified in this case.