Skip to content
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

Enable manifpy compilation when ROBOTOLOGY_USES_PYTHON is enabled #799

Closed
traversaro opened this issue Jun 15, 2021 · 6 comments · Fixed by #800
Closed

Enable manifpy compilation when ROBOTOLOGY_USES_PYTHON is enabled #799

traversaro opened this issue Jun 15, 2021 · 6 comments · Fixed by #800

Comments

@traversaro
Copy link
Member

traversaro commented Jun 15, 2021

Differently from most C++/Python projects that we support, manif does not permit to install from CMake its Python bindings (see https://github.com/artivis/manif/blob/0.0.4/python/CMakeLists.txt, and do not get afraid for the EIGEN_DEFAULT_TO_ROW_MAJOR as I was : ) ). Instead, the "installation" is happening in the setup.py by setting the CMAKE_LIBRARY_OUTPUT_DIRECTORY variable to the desired installation location.

So the options are:

  • Install manifpy by invoking the setup.py and specify manually the location where it should be installed
  • Patching the CMake build script of manif to permit to install the Python bindings directly from C++

Any opinion @GiulioRomualdi @diegoferigo ?

@traversaro
Copy link
Member Author

Sister issue: conda-forge/manif-feedstock#3 .

@diegoferigo
Copy link
Member

I'm not really an expert of packaging for conda. If I understood correctly, instead of getting files from the CMake install tree as we do for other projects, we have to pass through setuptools that will install the Python files in the enabled site-packages, and then package this folder with conda. Correct?

If this is ok in the conda-side, it seems to be the easiest solution at the moment. Note that I recently opened artivis/manif#228 that, depending if and how it will get implemented, could involve a new CMake machinery similar to what we are are used to have in our hybrid projects.

@traversaro
Copy link
Member Author

To clarify, the issue on manifpy packaging in conda is conda-forge/manif-feedstock#3 . This one is to install manifpy as part of the robotology-superbuild.

@traversaro
Copy link
Member Author

Calling a setup.py from the Buildmanif.cmake file or even creating a Buildmanifpy.cmake seems non obvious, as we would need to use features of ycm_ep_helper that I do not really know if they have been tested a lot in the past. Probably just modifying manifpy to support installing its Python bindings is the best option.

@diegoferigo
Copy link
Member

Maybe we can wait artivis/manif#233? However, this means that we cannot support the newly tagged release.

@traversaro
Copy link
Member Author

Maybe we can wait artivis/manif#233? However, this means that we cannot support the newly tagged release.

Cool, I did not saw that! This is not a big problem, if the PR is not merged soon I can quickly create a internal release on a fork and use that one, the proper release was just necessary for conda-forge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants