LD_LIBRARY_PATH and PYTHONPATH #82
Conversation
|
I'm actually not sure if this is general enough. I mean, right now Should we use |
README.md
Outdated
| * [FAQs](#faqs) | ||
| * [Octave](#octave) | ||
| * [Python](#python) | ||
| * [FAQs](#faqs) |
There was a problem hiding this comment.
The indentation of FAQs seems wrong.
|
Can you add the documentation on |
Not a big expert of Python, but the |
|
@traversaro Done
Ow, ok. I am used to Arch Linux where |
From the PEP itself:
: D |
README.md
Outdated
| @@ -324,9 +326,11 @@ To use this binaries and libraries, you should update the necessary environment | |||
|
|
|||
| Set the environment variable `ROBOTOLOGY_SUPERBUILD_ROOT` so that it points to the directory where you cloned the robotology-superbuild repository. | |||
|
|
|||
| Append `$ROBOTOLOGY_SUPERBUILD_ROOT/build/install/bin` to your PATH. | |||
| Append `$ROBOTOLOGY_SUPERBUILD_INSTALL_PREFIX/bin` to your PATH. | |||
There was a problem hiding this comment.
I am a bit confused by all this changes to Windows section of the README, given that DYLD_LIBRARY_PATH does not exist on Windows. Did you intended to do those in the macOS section?
There was a problem hiding this comment.
Wrong Ctrl+F. My fault, thanks for catching it
There was a problem hiding this comment.
This README is huge, it's too easy to get lost in the sections 😥
README.md
Outdated
| Support for this dependency is enabled by the `ROBOTOLOGY_USES_PYTHON` CMake option. | ||
|
|
||
| ### Configuration | ||
| Add the `$ROBOTOLOGY_SUPERBUILD_INSTALL_PREFIX/lib/python2.7/dist-packages` to your `PYTHONPATH`. This is done automatically if you source the provided `setup.sh` script. |
There was a problem hiding this comment.
Prepend "On Linux or macOS" if you mention the setup.sh file (this part of the documentation also applies to Windows).
README.md
Outdated
| Add the `$ROBOTOLOGY_SUPERBUILD_INSTALL_PREFIX/lib/python2.7/dist-packages` to your `PYTHONPATH`. This is done automatically if you source the provided `setup.sh` script. | ||
|
|
||
| ### Check the installation | ||
| The folder above should contain `_*.so` files which correspond to the generated python bindings. Open a python interpreter and try to import modules. |
There was a problem hiding this comment.
Please specify the folder to which you are referring, for example "The folder mentioned in the configuration/previous section".
Furthermore _*.so files are only present in *nix system, so I would probably just mention the .py files.
Now that it is documented, we can make it visible.
|
@diegoferigo please ping when you address the review, a single commit does not generate a notification (at least, not a clear one) to the reviewer. I also added the commit 5caf24e , merging, thanks! |
|
|
||
| @cmakeif ROBOTOLOGY_USES_PYTHON | ||
| # Add the python bindings directory to the PYTHON_PATH | ||
| PYTHONPATH=${PYTHONPATH}:${ROBOTOLOGY_SUPERBUILD_INSTALL_PREFIX}/lib/python2.7/dist-packages |
There was a problem hiding this comment.
@diegoferigo Sorry, why doesn't this line contains the export command? For me When I ran import yarp in a Python interpreter it gave me an error. Then I added:
export PYTHONPATH=${PYTHONPATH}:${ROBOTOLOGY_SUPERBUILD_INSTALL_PREFIX}/lib/python2.7/dist-packages
in .bashrc and import yarp is accepted by Python now.
There was a problem hiding this comment.
The export should be there, you're right. No clue honestly why it's missing.
There was a problem hiding this comment.
I think it was just an error on my side. Thanks for noticing @HosameldinMohamed .
This PR adds to the
setup.shfiles these environment variables, addressing:and closing #28.
This should not be necessary but RPATH is broken for python bindings, at least in iDynTree (robotology/idyntree#241).
I couldn't manage to use the old PR #34 because I force pushed to the source branch while it was still closed, operation which apparently prevents reopening it.
cc @traversaro @anqingd @raffaello-camoriano