-
Notifications
You must be signed in to change notification settings - Fork 62
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
Better Python installation instructions #1622
Conversation
doc/install/build_install.rst
Outdated
# E.g if the output was /home/xxx/myenv/env/lib/python3.9/site-packages, we | ||
# want to set pyprefix=/home/xxx/myenv/env/ | ||
python -c "import sysconfig; print(sysconfig.get_path('platlib'))" | ||
# Get the user site-package directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
# Manually set the prefix under which the python package will be installed.
# In this case, use the user site-package directory found by querying Python.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
site.getsitepackages()
does not in fact return user site-package directories (ever?), but I'll make it clearer it's up to the user to understand what they're feeding in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A side comment: has the import site; print(site.getsitepackages()[0])
command been tested on a range of different OS combinations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly confused, I suppose everyone is 😂
# want to set pyprefix=/home/xxx/myenv/env/ | ||
python -c "import sysconfig; print(sysconfig.get_path('platlib'))" | ||
# Get the user site-package directory | ||
pyprefix=`python3 -c 'import site; print(site.getsitepackages()[0])'` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So import site; print(site.getsitepackages()[0])
is the new query? IIRC there was a problem with virtualenv
s and the site
module. Is this case covered?
You also mention that only pip
finds the correct location in all cases, but we're not querying pip here?
The 2 commands you mention provide different outputs for me:
> python -m site --user-site
/home/robin/.local/lib/python3.9/site-packages
python -c "import site; print(site.getsitepackages()[0])"
/home/robin/.pyenv/versions/vvv/lib/python3.9/site-packages
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very confused, which is why I propose that Pythonic installation just uses pip
, which uses whatever magic is current to get at the right Pythonic path.
Indeed, those commands produce different outputs, because you may want to install the package differently. Either as a local user package (-m site --user-site
), or in the venv you're in (which for me would be -c "import site; print(site.getsitepackages()[0])"
). At least, I assume those are the two main options that a Python user is interested in, besides a system install into the default CMake prefix.
The point of this PR is to make clear that using CMake to install the Python package is going to be very unpythonic, and there is little we can do to help with that (because, what I though solved this issue once and for all in fact didn't). If Pythonic is what you want, you should really use pip. Does the documentation succeed in conveying that message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I think it does. I googled "arbor python installation" and basically got pip3 install arbor
which is good 👍 Then I went to the docs homepage and read through the "Get Arbor" text which also pointed me to the Python installation and pip3 install arbor
or to build from source, where these paths seem to make their entry and the note in this PR reasonably explains that the path might be tricky to get right but the explanation of -DARB_PYTHON_PATH_LIB
is there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Of course, let me know if you find out a better way.
No, and it is there merely as a suggestion. The fact that none of these are guaranteed to work is very much the point. That line being embedded in an example made that clear enough, I hoped, but if it is not, I can add an extra remark that there is zero guarantee it does what you think it might be doing. |
The old command, If there is a case where it doesn't work, why replace it with another approach that is also imperfect? |
It is not replaced. It is explained that according to the official docs and a particular case it does not do what we thought it was doing. |
It is directly replaced in the example that we provide for a method to get the appropriate installation directory. The previous version had been tested extensively by many of us in a broad range of environments, and had not been observed to break. So, while like every approach that isn't So the justification for making this change is still a mystery to me: why is the new approach better? Does it work around an observed problem? Or is it a stylistic change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still confused about why the approach, which we had tested and shown produced the least surprising results in the past, and is used internally by our CMake to generate the full installation path for Python packages, is replaced in the example with another approach which is apparently also not perfect, and has not been tested widely.
.. note:: | ||
The location of libraries under a prefix in only guaranteed to be standard for Python's global library location. | ||
Therefore, correct installation of the Python package to any other location using ``CMAKE_INSTALL_PREFIX``, | ||
such as user directory (e.g. `~/.local`), a Python or Conda virtual environment, may result in installation to a wrong path. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is confusing. If the correct path, i.e. the path where your venv/conda/system/ is installed is passed, won't it install correctly?
Of course, the devil is in the details about how you find this path, if you want to go setting it manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the issue is that under the prefix, the /lib[arch]
dir is up to the implementation/Python distro. We don't know whether it will be /lib
or /lib64
, hence the warning in the Python docs. The only way to be sure of the path, is to not assume a directory layout under the prefix and ask Python for the full path, for instance with python3 -m site --user-site
, and set ARB_PYTHON_LIB_PATH
. (I did not know this, until I ran into such a Python distro.)
So, I think it's important to advise the user that setting a prefix is not guaranteed to work for installs except in /usr
, where Python does promise the directory layout we all know and love. The only way to be sure it's installed in a path that is used, is to 1. use pip, not CMake; or 2. prod at Python yourself to find the particular path it uses and set it with ARB_PYTHON_LIB_PATH
. The issue with 2. is that there is no single function to obtain this path (also depends on whether you want a user, venv, system install).
Python packaging...
Cause
sysconfig.get_path('platlib',vars={'base':pfx,'platbase':pfx})
is not guaranteed to produce a correct path for non-system installations (the global site-packages directory as Python calls it). For user installations, the user site-packages directory is used, which can be obtained withpython -m site --user-site
. Confusingly, when in a Python or Conda virtual environment, Python is not guaranteed to have thesite
package (although it did on my system during the typing of this comment) or is not guaranteed to respect the venv boundary (I can confirm). Moreover,platlib
directory obtained withsysconfig
only has meaning for the global site-packages directory. Although for Ubuntu, Fedora, CentOS and Arch this coincides with how they have apparently patched Python, this is again not a guarantee. OpenSuse indeed have no platlib for local installs, so Arbor is copied to a place (~/.local/lib64
) that system Python does not use (which is~/.local/lib
).pip
does get the right path in all the above circumstances (global, user, venv site-package dirs), and I am too tired of chasing this issue that I am not going to bother finding out how it determines that path. It somehow knows which item to choose from the list of global site-package dirs you obtain withsite.getsitepackages()
.Our
sysconfig.get_path('platlib',vars={'base':pfx,'platbase':pfx})
only produced the right path on the tested platform by coincidence.Solution
I've modified to docs to always use
pip
to install the Arbor Python package, because that is the best or only way to get the most Pythonic installation experience. Only on the CMake page have I left a hint of what to feed ARB_PYTHON_LIB_PATH, and that by using CMake, you accept unpythonicness.Sources:
Closes #1405