jorgenschaefer / elpy

Emacs Python Development Environment
GNU General Public License v3.0
1.89k stars 259 forks source link

Automatic selection and installation of versioned venv for elpy rpc/jedi/... #516

Open BinaryKhaos opened 9 years ago

BinaryKhaos commented 9 years ago

Hello Jorgen,

I have been doing this partly on my own emacs configuration for a while (w/o elpy) but I think this would be better suited in a dedicated mode like elpy to make it available to a wider audience as I think it is a nice feature to have:

The basic idea is, elpy checks the currently active environment's (either system or venv) Python version and selects a proper venv within a base path (like ~/.emacs.d/.python-venvs) that contains all general tools like elpy's rpc, Jedi and so forth and uses that for the current buffer as its internal venv (where it starts its RPC from and so forth). Something like:

base-path/interpreter-name/major.minor

Example:

~/.emacs.d/.python-venvs/python/3.4/

In the above case, that directory would contain a venv for the normal Python interpreter version 3.4 that includes all necessary tools and libs for elpy.

In a first iteration, the user would have to make sure to install that venv manually. Later, that could be automated as well by providing a variable the user can customize that includes both the requirements of Elpy and what the user needs.

This way, the tools required for working within Emacs are nicely separated from any project's venv, can easily be upgraded and managed and the general workflow is more comfortable imho.

What do you think of this idea? Would you accept such a feature into Elpy? If yes, I would be willing to work on this and get a first PR fleshed out as soon as time somehow permits.

Comments, suggestions and whatnot are naturally also more than welcome. :-)

Thanks for your time, Matthias

jorgenschaefer commented 9 years ago

Thank you for the idea! I am afraid I am not sure I fully understand what you are proposing there.

Let's say I am working on project foo (python 3.4). So I activate my project's virtualenv prj_foo. You seem to say there is another virtualenv, let's call it elpy_py34, with Elpy's support packages. But if I now run Elpy in elpy_py34, Jedi/Rope/importmagic etc. won't have access to the packages in prj_foo.

How do you use both virtualenvs at the same time?

BinaryKhaos commented 9 years ago

I should have mentioned that, sorry. :-)

Naturally Elpy will need to add the current project's python path to the tools python path while executing anything there to give it access to the project.

For me, this works reasonably well. The alternative, having the tools installed in each project's venv, was never very appealing to me. :(

jorgenschaefer commented 9 years ago

This would mean that Elpy would be unable to find completions for any installed packages (e.g. Django).

Hm. You should be able to add $VENV/lib*/python*/site-packages/ to PYTHONPATH and have it work, if you make sure the Python version is the same.

BinaryKhaos commented 9 years ago

That's pretty much what I said, yes. :-)

If you don't modify the PYTHONPATH for the support venv (as you called it), nothing in it has access to your project's data and thus completions and whatnot will naturally not work. This is exactly what I do locally. I use jedi and pylint (through Flycheck) that why (amongst others) -- just not with Elpy thus far since I stopped using it quite a while ago for other reasons but consider switching back and stop reinventing the wheel while also bringing in some of the stuff that I (and hopefully others) would care about (like this feature).

And you are absolutely right, the Python versions have to match for this work. That's the reason for the versioned support venvs and not just a single venv. But that's really not a problem imho and far better than having all this spread out amongst your project venvs.

The question remains: What do you think about this feature and would you care to include it in Elpy? Like I said, I would volunteer to implement it (if you would care to do it yourself, I wouldn't stop you naturally :P). A first iteration could simply implement the venv selection process without any automatic creation/installation if none does yet exist.

jorgenschaefer commented 9 years ago

That's pretty much what I said, yes. :-)

Then I misunderstood what you said, sorry. :-)

The question remains: What do you think about this feature and would you care to include it in Elpy?

Generally, having a way of installing development packages only once (per Python version) seems like a good idea to me. Whether I care to include it in Elpy depends a great deal on the specific implementation.

BinaryKhaos commented 9 years ago

That sounds like good news to me since an implementation can be tweaked and refined until it meets certain standards and requirements. :-)

Obviously, figuring out the Python version cannot be done through the Elpy python module, so that has to be done manually. Locally I do it through some Python code that interfaces via JSON -- rather simple actually. One could in theory try to parse the version string of the Python command line but I don't think this is either a clean way to do it, nor very robust to say the least. This means a bit of functionality redundancy in Elpy itself but there is no way around it I can see.

If you don't want to take a stab at it, I will see what I can flesh out and post a first PR once ready. I will try to keep the implementation as minimal as possible, naturally.

jorgenschaefer commented 9 years ago

M-x elpy-config already calls Python directly and returns the Python version, you can probably at least do something similar (if not the same, the code also checks PyPI). :-)

jorgenschaefer commented 9 years ago

Oh, and please go for it – I don't think I'll have the time to work on Elpy in the next weeks.

BinaryKhaos commented 9 years ago

elpy-config would be nice to reuse but I think for this purpose it is just too heavy weight. The Python version needs to be checked every time a py file is opened or the project's venv is changed. Keeping that in mind, having those actions perform several checks on PyPI each, is not something I would prefer to do.

But generally for the Python code part, I did the same locally (w/o the PyPI part naturally :P), so either I refactor elpy-config--get-config a bit to make it more selective in what it queries or I introduce a second variable with another code block for the query and another function. I would generally prefer to make elpy-config--get-config more selective but I fear this might add an unproportional amount of complexity. I will have to think about that for a bit if I can come up with an intuitive to use, read and elegant implementation idea for this. Passing a bitfield into the code block on which the queries are dependent would be one way to do it and be rather non-invasive but on the other hand rather limiting for the future and not very clear. Breaking up the code block into smaller pieces and assembling those on demand in elpy-config--get-config itself would be another way. Just thinking out loud...

jorgenschaefer commented 9 years ago

A generic function that takes a piece of Python code that prints a JSON object, reads said object, and populates a hash with it would be a good abstraction for this, I think.

BinaryKhaos commented 9 years ago

That's what I have locally -- just a bit more complex. It tries to ensure to always return a JSON object with either the actual data or an error, so even throwing invalid code at it works.

Your suggestion implies refactoring your currently monolithic code query block into smaller pieces (several lisp variables), if I am not mistaken?

jorgenschaefer commented 9 years ago

Your suggestion implies refactoring your currently monolithic code query block into smaller pieces (several lisp variables), if I am not mistaken?

I do not think so, but maybe I misunderstand what you said.

BinaryKhaos commented 9 years ago

Ah, okay. I guess I now understand what you mean. Basically refactoring the function elpy-config--get-config to call out to a generic Python bridge function instead of doing the work itself, so at least that part can be reused and the redundancy we talked about earlier is really only the additional code block, so to speak.

That might actually be the cleanest and simplest approach, imho. Even though splitting the monolithic code block into smaller modular pieces has its appeal too. But well...

jorgenschaefer commented 9 years ago

That's fine with me, too (anything that makes code more readable!), but I think unrelated to the issue we talked about :-)

BinaryKhaos commented 9 years ago

Just a quick heads up: This is still very high up on my agenda and not forgotten or anything (since I want to use it myself :P). I just need to find some free and continuous time to work on this. As soon as I have something presentable and working, I will update this ticket... hopefully sometime next week (no guarantees).

BinaryKhaos commented 9 years ago

Sorry for the huge delay. I got swamped by life and had to prioritize accordingly. :-( It is most definitely not forgotten. I actually planned to work on this today but didn't get around to it.

So: If no one else has interest, jumps in and takes over, I am still very keen on implementing this as soon as I get the time to do it.

jorgenschaefer commented 9 years ago

A simple (setenv "PYTHONPATH" ... path to venv .../lib/python2.7/site-packages/") seems to have done the core of this idea already, i.e. make rope/jedi/importmagic installed in one venv available in another. As a first step, I was thinking about providing an elpy-rpc-extra-pythonpaths (analog to python-shell-extra-pythonpaths) that is added, together with the path to Elpy, to the PYTHONPATH for RPC processes.

That does not include the setup of said virtualenv, but it would be a start. Does that sound good?

BinaryKhaos commented 9 years ago

Things are a bit more complicated though, I am afraid. You have to figure out which is the right Elpy tools venv to use, so you will need to query the current project's Python interpreter for its version and use that as a basis to construct a path from some kind of base path (e.g. user-emacs-directory/.elpy-tools-venv/x.y/...) and add that path to the PYTHONPATH. And if there is no matching tools venv, re-use the project's venv until the automatic installation of the tools venv is implemented. Also, you have to be careful since the introduction of Python's own venv tool, there has been a slight incompatibility with regards to its directory layout on x86_64 systems vs standard virtualenv for a time that can cause problems (see here).

This naturally has to be done at the right spot. Right now I am thinking that is elpy-rpc--open where the actual path to the Python interpreter is looked up and refactor that into its own function and do so some more involved logic in there.

Also, I do think that once the automatic installation is available, a separate Elpy tools venv should become the default and only way to ease up on the complexity of the code and logic. The question remains though, if elpy-rpc--open would be the right spot to (in)directly trigger such an installation due to a missing Elpy tools venv and how much of that should happen asynchronously and how?

What I think should be emphasized again is (since it is one of the core ideas) that the list of all Python packages that are going to be installed in the Elpy tools venv, should be constructed from a static core list of packages strictly required by Elpy and a user supplied list of packages (s)he wants to have installed as well, so one can build up on this feature too (like having additional linters available for configuration and use in Flycheck).

jorgenschaefer commented 9 years ago

You have to figure out which is the right Elpy tools venv to use

As a first step, you can easily have the user configure this per-project. This would be a trivial change to allow the user to set up a second virtualenv with Elpy tools without having to do all this automatic guessing.

BinaryKhaos commented 9 years ago

You are absolutely right but imho that would be a bad stepping-stone for the future development of this feature and give the wrong overall impression. Imho this feature should be as oblivious to the user as possible as long as (s)he doesn't need to add additional Python packages or otherwise build on this. Elpy should just work out-of-the-box with the least hassle possible for the user and the most flexibility possible if required by the user.

Having the requirement in place to configure this per project will automatically void the use-case where you have to do some quick work in a code-base that has not been setup as a proper project. With the automatic detection (which is really not too bad or involved, by the way) it will work just fine.

Also, if this feature would be introduced this way (as a per-project variable so to speak) first, it would make it a lot more difficult to later introduce the concept of automatic detection, installation and management of those Elpy tools venvs.

Just my two cents... I think it is really worth it to go a bit further for the first iteration, so one can easily build on this in later iterations and make life easier for the user as well.

BinaryKhaos commented 9 years ago

I had some time available today and started working on this. It is not yet ready and still needs some work but I will try to get a first version done in the next couple of days (sometime next week, the weekend is rather filled already), push it to my fork/branch and make a first pull-request for review, so we can discuss things further with some actual facts at hand. :-)

Again, no guarantees on any timelines though. I try my best... ;)

Have a nice weekend.

jorgenschaefer commented 9 years ago

Thank you for the work :-)