whi-llc / adjust

GNU General Public License v3.0
1 stars 2 forks source link

venv is assumed to be installed #3

Open varenius opened 2 years ago

varenius commented 2 years ago

Following the instructions, but on a machine without python3-venv installed, I get this output:

oper@gyller:~/adjust$ bash install 
The virtual environment was not created successfully because ensurepip is not
available.  On Debian/Ubuntu systems, you need to install the python3-venv
package using the following command.

    apt-get install python3-venv

You may need to use sudo with that command.  After installing the python3-venv
package, recreate your virtual environment.

Failing command: ['/home/oper/adjust/venv/bin/python3', '-Im', 'ensurepip', '--upgrade', '--default-pip']

install: line 3: venv/bin/activate: No such file or directory
Collecting numpy==1.19.0 (from -r requirements.txt (line 1))
  Downloading https://files.pythonhosted.org/packages/00/16/476826a84d545424084499763248abbbdc73d065168efed9aa71cdf2a7dc/numpy-1.19.0-cp36-cp36m-manylinux1_x86_64.whl (13.5MB)
    100% |████████████████████████████████| 13.5MB 153kB/s 
Collecting pandas==1.0.5 (from -r requirements.txt (line 2))
  Downloading https://files.pythonhosted.org/packages/c0/95/cb9820560a2713384ef49060b0087dfa2591c6db6f240215c2bce1f4211c/pandas-1.0.5-cp36-cp36m-manylinux1_x86_64.whl (10.1MB)
    100% |████████████████████████████████| 10.1MB 203kB/s 
Collecting python-dateutil==2.8.1 (from -r requirements.txt (line 3))
  Using cached https://files.pythonhosted.org/packages/d4/70/d60450c3dd48ef87586924207ae8907090de0b306af2bce5d134d78615cb/python_dateutil-2.8.1-py2.py3-none-any.whl
Collecting pytz==2020.1 (from -r requirements.txt (line 4))
  Downloading https://files.pythonhosted.org/packages/4f/a4/879454d49688e2fad93e59d7d4efda580b783c745fd2ec2a3adf87b0808d/pytz-2020.1-py2.py3-none-any.whl (510kB)
    100% |████████████████████████████████| 512kB 4.4MB/s 
Collecting six==1.15.0 (from -r requirements.txt (line 5))
  Downloading https://files.pythonhosted.org/packages/ee/ff/48bde5c0f013094d729fe4b0316ba2a24774b3ff1c52d924a8a4cb04078a/six-1.15.0-py2.py3-none-any.whl
Installing collected packages: numpy, pytz, six, python-dateutil, pandas
Successfully installed numpy-1.19.5 pandas-1.0.5 python-dateutil-2.8.1 pytz-2020.1 six-1.16.0
install: line 5: deactivate: command not found

It seems that the script should check if python3-venv (or virtualenv) is installed or not. Maybe worth checking?

whimwich commented 2 years ago

I am not sure I can anticipate the names of the package that various systems might need. I think python3 actually gave a pretty clear error message in your case.

My first suggestion would be to insert set -e as the second line in the script, then it would stop after the first error. I am not sure python3 returned an error in your case. Can you tell? If not, the . venv/bin/activiate line did and that would have stopped the script. Other suggestions are welcome.

BTW, my (newer? older?) Debian system (stretch) didn't seem to need python3-venv, but the script still failed without or without it installed. It can't find some of the requirements. I am somewhat surprised by that as I though that was one of the benefits of the virtual environment. I should probably being something more sophisticated with requirements.txt, but I don't know what.

varenius commented 2 years ago

I am sorry I was unclear about the problem. The problem, as I see it, is not that venv is needed. The problem is that the install script will do stuff, even if its requirements are not satisfied. I was just surprised by this. Because venv was not installed, this tool went ahead and installed python things in a different location than expected. I would normally assume a program to either prompt " are you sure you want to do X" or say "This program cannot run since it requires Y".

If it's up to the user to manually check that various python modules are installed before running this script, one could argue that using venv and requirements is somewhat not useful. It could be simpler, and perhaps more straightforward, to just have an install script that says "make sure you have the following python module versions installed in your python path" and then leave it to the user to install modules and any venvs, if desired. That would avoid the behaviour of the install script doing things outside the git directory without warning ;).

whimwich commented 2 years ago

I also am sorry I was not clearer. Using set -e will prevent it careening out of control like that if it can't install venv, or any other error happens. I could in addition add a more explanatory error message if setting up venv fails. It would have to be a generic message because I have no idea what any particular system would need to have installed. I could also add a warning about this in the README file. I am sure all that would be beneficial.

Can you let me know what version of Debian/Ubuntu you are using? It would also be helpful to know your version of python3. I thought you get a very helpful message for the failure of the venv to install.

My understanding is that the pip/venv approach is suppose to make you independent of what modues/versions are installed on your system. I am not an expert at this, but what I have works for my OSX computers and I had a user on a different system that was able to install fmout with the same approach.

I am somewhat disappointed to find that the pip/venv approach does work on stock Debian stretch (with or without the python3-venv package installed) because it can't find the right versions of some modules. I was hoping that pip/venv would provide a turn-key approach for getting the right modules/versions, but apparently I don't understand how this is suppose to work. Do you have any pip/venv experts in your shop who could advise?

We could include instructions for how to install whatever modules versions pip can find, but that might leave the user with versions that aren't compatible. (Gee, wouldn't something like pip/venv be a good solution for that?) It kind of belies the idea of verifying that the code works for a particular set of module/versions and then users can expect it to work on their systems.

varenius commented 2 years ago

I think I tested this on Linux Mint 20, Python 3.8.10. Indeed the failure message was very helpful.

Maybe all of this could be simply circumvented by adding some sort of "Check above for any error messages, then continue yes/no?" question to the install script? Just to give the option of not proceeding in case some errors are found.

As I'm now going on parental leave I cannot dig more into pip/venv expertise. But in general I think it's the right thing to do.