Use CMAKE_INSTALL_PREFIX for python-install target

[senselessDev]

Currently the python-install target wants to install to the default location of setup.py which is /usr/local (see here). That typically means you need to sudo make python-install (root privileges). The normal gtsam C++ code gets installed to CMAKE_INSTALL_PREFIX as usual with CMake (e.g. you could choose $HOME/.local for a per-user installation without root privileges).

If you don't specify any custom CMAKE_INSTALL_PREFIX, CMake should default to /usr/local on Linux which then should be the same behavior as before (on my machine it does). I am not sure about Windows though...

I propose to also use this CMake mechanism to select where the python bindings are going to be installed. Actually, the current readme says that it actually would work this way:

gtsam/python/README.md

Line 32 in d6edcea

- **NOTE**: if you don't want GTSAM to install to a system directory such as `/usr/local`, pass `-DCMAKE_INSTALL_PREFIX="./install"` to cmake to install GTSAM to a subdirectory of the build directory.

But it does not. This PR fixes that.

Edit: Just for reference, was already topic in #1059 (comment)

[varunagrawal]

I don't think this is the right approach. If you read the docs in detail, it says that the path to install to should be prefix/lib/pythonX.Y/site-packages. CMAKE_INSTALL_PREFIX is not the location to a python interpreter but a generic location to install the C++ library to.

Have you tried using this change and running some simple scripts in IPython?

[senselessDev]

Yes, of course I have tried that. In this case with $HOME/.local. It worked as expected.

To be honest, I don't really understand what you are talking about. There has to be no python interpreter located where packages are installed. In fact, if you do a pip install xyz --user it will install to $HOME/.local on Linux (reference). What obviously is required is that the Python executable knows where to search for the packages. Next to the default search paths, this can by modified/extended by PYTHONPATH or sys.path.

The only thing I can understand is that one might want to install the C++ library somewhere else than where the Python library is installed. But then a new CMake variable should be introduced from my point of view to choose the Python install path. Currently, you can not change the install path of the Python library at all. If you are not in a virtual environment, it's the system's Python library path which usually requires root privileges.

[varunagrawal]

So if you look at the answers in that SO reference you shared, it says that the install path with --user is in a directory tree under a python installation which is basically the site-packages directory. You could install the module anywhere but we prefer that the user need not update the PYTHONPATH since that can be error prone and hard to debug especially for those not familiar with Python's system level machinery.

If you wish to just not have to install to /use/local why not just add the --user flag to the install command in CMake?

[senselessDev]

So if you look at the answers in that SO reference you shared, it says that the install path with --user is in a directory tree under a python installation which is basically the site-packages directory.

I don't know where you have read that. Just to be clear:

In [1]: import sys

In [2]: sys.executable
Out[2]: '/usr/bin/python3'

In [3]: import site

In [4]: site.USER_SITE
Out[4]: '/home/xxxx/.local/lib/python3.6/site-packages'

--> The python installation is in /usr whereas the --user option installs to $HOME/.local.

You could install the module anywhere

And how? With the current CMakeLists you can't. It always installs to /usr/local.

but we prefer that the user need not update the PYTHONPATH since that can be error prone and hard to debug especially for those not familiar with Python's system level machinery.

Yes, therefore the default, i.e. the user specifies nothing, installation path should still be /usr/local. It is also the default of CMAKE_INSTALL_PREFIX. So it stays like that.

If you wish to just not have to install to /use/local why not just add the --user flag to the install command in CMake?

Because that will change the default behavior and probably won't work for everybody, e.g. in a virtual environment.

[varunagrawal]

So if you look at the answers in that SO reference you shared, it says that the install path with --user is in a directory tree under a python installation which is basically the site-packages directory.

I don't know where you have read that. Just to be clear:

In [1]: import sys

In [2]: sys.executable
Out[2]: '/usr/bin/python3'

In [3]: import site

In [4]: site.USER_SITE
Out[4]: '/home/xxxx/.local/lib/python3.6/site-packages'

--> The python installation is in /usr whereas the --user option installs to $HOME/.local.

From your own output, the install location is $HOME/.local/lib/python3.6/site-packages, not $HOME/.local. That is a key difference, we cannot simply ignore the rest of the path. :)

You could install the module anywhere

And how? With the current CMakeLists you can't. It always installs to /usr/local.

I meant this hypothetically, or by running pip install --prefix=<path> . from the appropriate directory. You don't need CMake or make in that case.

but we prefer that the user need not update the PYTHONPATH since that can be error prone and hard to debug especially for those not familiar with Python's system level machinery.

Yes, therefore the default, i.e. the user specifies nothing, installation path should still be /usr/local. It is also the default of CMAKE_INSTALL_PREFIX. So it stays like that.

Again, no. The default location will be /usr/local/lib/pythonX.Y/site-packages, not simply /usr/local, so it is NOT the same as CMAKE_INSTALL_PREFIX.

If you wish to just not have to install to /use/local why not just add the --user flag to the install command in CMake?

Because that will change the default behavior and probably won't work for everybody, e.g. in a virtual environment.

In that case, why not just install the nightly build or the package on PyPI? Your PR does not address this issue, rather it just puts the python module next to the C++ libraries. Given how python loads modules, I can foresee a name collision happening here where Python tries to import gtsam but will import the C++ directory and not the python/gtsam directory.

[senselessDev]

From your own output, the install location is $HOME/.local/lib/python3.6/site-packages, not $HOME/.local. That is a key difference, we cannot simply ignore the rest of the path. :)

Yes, but prefix is only $HOME/.local and the rest is added by python itself: https://docs.python.org/3/install/#alternate-installation-unix-the-prefix-scheme Have a look in the table. It is not called prefix without reason. And it is pretty standard in the whole Unix world. If you are setting CMAKE_INSTALL_PREFIX the executables won't end up there, but in this directory's bin sub-directory.

Again, no. The default location will be /usr/local/lib/pythonX.Y/site-packages, not simply /usr/local, so it is NOT the same as CMAKE_INSTALL_PREFIX.

Again, that's wrong. It's a prefix!

In that case, why not just install the nightly build or the package on PyPI?

Because my own development branch is not on PyPi?! It obviously is not about the average user but a useful addition for developers IMHO.

rather it just puts the python module next to the C++ libraries. Given how python loads modules, I can foresee a name collision happening here where Python tries to import gtsam but will import the C++ directory and not the python/gtsam directory.

No, it doesn't. See above. It is exhausting...
I could certainly live with a decision that we don't want it that way. But at least with a logical and correct argument please.

[ProfFan]

I think you guys probably are not talking on the same page here @senselessDev and @varunagrawal .

@senselessDev, the docs may be a bit misleading, setup.py will install to the prefix of the current interpreter (at cmake time). The 'default' on the docs is rather like an example.

So I think the current behavior is reasonable, though not perfect. Since the PYTHON_EXECUTABLE is obtained at configure time, not compile time, the final install location may be surprising for some users if they used the system python at cmake and switched to a venv when they do make python-install.

[varunagrawal]

Well you kept saying it would install to $HOME/.local and not that it is a prefix. Your exact response was

The python installation is in /usr whereas the --user option installs to $HOME/.local.

I am sorry this is exhausting for you, but it is equally exhausting for us when we are not given the proper information.

[dellaert]

All please keep discussion courteous and professional here, please.

[senselessDev]

@senselessDev, the docs may be a bit misleading, setup.py will install to the prefix of the current interpreter (at cmake time). The 'default' on the docs is rather like an example.

I don't know what exactly is misleading, but yes, that will usually default to the /usr/local prefix with the system's python installation, right? Which also is the 'default' CMAKE_INSTALL_PREFIX and that's why I thought it is a good match, nothing changes in the default case.

Since the PYTHON_EXECUTABLE is obtained at configure time, not compile time, the final install location may be surprising for some users if they used the system python at cmake and switched to a venv when they do make python-install.

Yes, that's true. Changing after configuring is not a good idea. But is that an argument against the --prefix option?

I assume from your answer that you don't like my idea, which is ok, but I don't get why. Can you elaborate? Is an additional CMake variable not a good idea to solve all issues then?

Well you kept saying it would install to $HOME/.local and not that it is a prefix.

I'd still say it installs in $HOME/.local. Your interpretation of 'install' = 'everything is in this exact folder' is not mine. That it is about a prefix should have been clear from the beginning IMO. The diff has the word 'prefix' included two times.

Your exact response was

The python installation is in /usr whereas the --user option installs to $HOME/.local.

I am sorry this is exhausting for you, but it is equally exhausting for us when we are not given the proper information.

And your python executable is /usr/python3? Interesting. That example is really not supportive of your allegation.
It is not professional behavior to blame me when you seem to have no idea about the topic discussed.

To summarize, for me it is not fun anymore to contribute to the project at the moment. Therefore, I will also not pursue this PR for now. Maybe the doc should be changed at least when it does not hold true anymore:

gtsam/python/README.md

Line 32 in d6edcea

- **NOTE**: if you don't want GTSAM to install to a system directory such as `/usr/local`, pass `-DCMAKE_INSTALL_PREFIX="./install"` to cmake to install GTSAM to a subdirectory of the build directory.

[dellaert]

@senselessDev I have to apologize for @varunagrawal 's language above. I think he was out of line, and then the discussion spiraled out of control. I'm OK with closing this PR.

@varunagrawal please do not respond here - talk to me offline if needed. Let's please leave it at this for this PR.