Suggestion: keep sys.path per Evaluator instance

[tkf]

Currently Jedi uses sys.path directly and so editors have to modify sys.path in order to tell where modules locate. This is not ideal. For example, users may want to add Python 3 modules to sys.path while running Jedi in Python 2 (e.g., to write Python 2/3 compatible module). To do that, I suggest to keep additional paths in a place other than sys.path. Not only that, we can do more. If we keep sys.path per Evaluator instance, then we can ask Jedi "what kind of modules I'd have if I have this set of sys.path?" without running Jedi in another Python process. This may be useful, for example if you want to have different set of completions when you switch project.

(BTW, I am using partially 8 months-old knowledge of Jedi so, sorry if I miss some new features.)

[ColinDuquesnoy]

we can ask Jedi "what kind of modules I'd have if I have this set of sys.path ?"

This could also be useful to implement some kind of library view or a package manager in an IDE, e.g/ to retrieve the list of modules currently installed and their docstrings. It would also be great if it could support virtual envs.

[tkf]

It would also be great if it could support virtual envs.

Once my suggestion is in, it is straight forward to support virtualenv. It can be done by just adding VIRTUAL_ENV/lib/pythonX.Y/site-packages/ so sys.path, like Jedi does it directly to sys.path now. You can even add multiple virtual environments.

[ColinDuquesnoy]

You can even add multiple virtual environments.

Awesome!

[davidhalter]

Also covers #371.

Once my suggestion is in, it is straight forward to support virtualenv. It can be done by just adding VIRTUAL_ENV/lib/pythonX.Y/site-packages/ so sys.path,

It's actually more, e.g.:
'py33/lib/python3.3', 'py33/lib/python3.3/plat-linux', 'py33/lib/python3.3/lib-dynload', 'py33/lib/python3.3/site-packages'

I would propose to add something like settings.sys_path_insertions = [] and settings.virtualenv_path = None. This would enable the inclusion of "only" one virtualenv, but I think that's enough ;-) Everybody else could just add the additional paths to the insertions.

@tkf I agree with the whole proposal. However, I don't think it goes far enough. We should get rid of all global state. We could easily do that by just cloning the jedi.settings.__dict__ and putting that into evaluator.settings. This is a pretty straightforward process and probably wouldn't take long. After that we would have to do clever locking in for the caches and Jedi would (probably?) support multithreading.
Forking the settings module is IMHO the best approach, since it allows both easy configuration and multithreading.

[tkf]

This would enable the inclusion of "only" one virtualenv, but I think that's enough ;-)

Why not just take a list of virtual environments? For example, you could have different set of modules in .tox/py27/... and .tox/py33/... it could be useful to look into both places.

something like settings.sys_path_insertions = [] and settings.virtualenv_path = None

I think it is OK, but you may want to think about it more, if you want to consider deletion of sys.path. Someone might want to know completion given modules in a subset of sys.path. In that case, we need to tell Jedi to ignore the actual sys.path and only consider the given paths.

cloning the jedi.settings.__dict__ and putting that into evaluator.settings.

Sounds good.

After that we would have to do clever locking in for the caches and Jedi would (probably?) support multithreading.

I prefer if you go to multi-processing direction, since emacs-jedi already launches multiple Jedi processes from an Emacs process. I am bit scary about cache right now, but well, if you mess up with cache then you just have to delete it. But if Jedi handles multiprocessing nicely that would be cool. Anyway, this is not scope of this issue.

[davidhalter]

something like settings.sys_path_insertions = [] and settings.virtualenv_path = None

I think it is OK, but you may want to think about it more, if you want to consider deletion of sys.path. Someone might want to know completion given modules in a subset of sys.path. In that case, we need to tell Jedi to ignore the actual sys.path and only consider the given paths.

So just settings.sys_path = sys.path + some magic maybe? Sounds good to me. Let's do that.

This would enable the inclusion of "only" one virtualenv, but I think that's enough ;-)

Why not just take a list of virtual environments? For example, you could have different set of modules in .tox/py27/... and .tox/py33/... it could be useful to look into both places.

You're probably right, it doesn't matter for the plugin developer and it's good to have that option (not that I will ever use it). I wanted to keep it simple, but I guess a list instead doesn't hurt.

I prefer if you go to multi-processing direction

I agree and I totally share your concerns about the cache. But that doesn't change anything, for multithreading. People have used Jedi without noticing that it doesn't support multithreading yet, so I'd like to support it.

[tkf]

Then settings has something like

include_sys_path = True
sys_path_insertions = []
virtualenv_paths = []

And sys_path is composed by something like:

sys_path = settings.sys_path_insertions
sys_path += map(get_env_path, settings.virtualenv_paths)
if settings.include_sys_path:
    sys_path += sys.path

Sounds nice. I'd use something like additional_sys_path instead of sys_path_insertions but that's just bike-shedding.

[davidhalter]

Hmm, I don't know, that's three settings instead of two. Just thinking out loud - Why not like this:

sys_path = sys.path
virtualenv_paths = []

->

sys_path = list(map(get_env_path, settings.virtualenv_paths))
sys_path += sys.path

If you don't want certain parts of the sys_path, you can just play with it yourself.

An option would be to make utils.get_virtual_env_path part of the API and to set settings.virtualenv_paths = None, because people can just play with settings.sys_path.

[tkf]

Why not just have settings.sys_path, which defaults to sys.path, then? Maybe Jedi does not need to care about virtualenv at all, since actually "the right" implementation of get_virtualenv_path is a bit long. You can see virtualenv.path_locations in https://github.com/pypa/virtualenv/blob/1.11.4/virtualenv.py#L999
(@t2psyto found this in tkf/emacs-jedi#124 (comment))

But I don't know if virtualenv.path_locations is a public API from virtualenv module so I don't think it is a good idea to use it in Jedi.

[davidhalter]

Why not just have settings.sys_path, which defaults to sys.path, then?

That's what I mean, sorry I copied the wrong bits.

But I don't know if virtualenv.path_locations is a public API from virtualenv module so I don't think it is a good idea to use it in Jedi.

We cannot use virtualenv directly, because it's not in the standard lib. I don't think a dependency just for this is a good idea. But thank you for the hint.

[tkf]

Then maybe we should drop virtualenv support completely (i.e., not having settings.virtualenv_paths), since current implementation of utils.get_virtual_env_path does not work in Windows anyway.

[davidhalter]

There is no utils.get_virtual_env_path, I was just trying to show how an API could look like. Sorry for the confusion. The question is if we can write a function ourselves in like 40 lines that covers all virtualenv paths.

[tkf]

Right, I mean current VIRTUAL_ENV treatment in Jedi does not work with Windows etc. Just look at how complex virtualenv.path_locations is.

The question is if we can write a function ourselves in like 40 lines that covers all virtualenv paths.

virtualenv.path_locations is like "the definition" of virtualenv path. I don't think we can minimize it very much. You can copy it from virtualenv but the function is more than 50 lines. Plus, you will need import etc. for it. So maybe about 60 lines?

[davidhalter]

Fixed by #636.