Move python bindings into separate cmake target #6220

morxa · 2020-04-10T11:16:20Z

Move python bindings into a separate cmake target so we can install them separately (see RFE: Move python cmake target into its own namespace #6124).
Also install the python bindings into python's sitearch, not in /usr/lib64 (or /usr/local/lib64).

Resolves #6124.

dorodnic · 2020-04-12T10:50:00Z

Thank you, @morxa
I think it's good, @ev-mp @lramati ?

lramati · 2020-04-12T11:01:15Z

lgtm, but @ev-mp would know more about the packaging side of our CMake infrastructure

morxa · 2020-05-19T06:40:13Z

Any progress here?

ev-mp

@morxa hellom
I'm testing this PR on top of the latest v2.35.0 with Ubuntu18 LTS and get the following interrupt:

git/librealsense/build$ sudo make uninstall && make clean &&  cmake -DBUILD_EXAMPLES=true -DCMAKE_BULID_TYPE=Debug -DBUILD_PYTHON_BINDINGS=true && make -j$(($(nproc)-1))
[sudo] password for user: 
-- Internet connection identified
-- Info: REALSENSE_VERSION_STRING=2.35.0
-- Setting Unix configurations
-- using RS2_USE_V4L2_BACKEND
CMake Error at wrappers/python/CMakeLists.txt:10 (find_package):
  By not providing "FindPython.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "Python", but
  CMake did not find one.

  Could not find a package configuration file provided by "Python" with any
  of the following names:

    PythonConfig.cmake
    python-config.cmake

  Add the installation prefix of "Python" to CMAKE_PREFIX_PATH or set
  "Python_DIR" to a directory containing one of the above files.  If "Python"
  provides a separate development package or SDK, be sure it has been
  installed.


-- Configuring incomplete, errors occurred!
See also "/home/user/git/librealsense/build/CMakeFiles/CMakeOutput.log".
Makefile:3874: recipe for target 'cmake_check_build_system' failed
make: *** [cmake_check_build_system] Error 1

Python 2.7 and 3.7 are installed (including -dev)
Please advise

morxa · 2020-06-02T13:03:52Z

Right, I just realized the new cmake Python module requires cmake 3.12. Ubuntu 18.04 ships with cmake 3.10.

I guess you still want to support older cmake versions?

ev-mp · 2020-06-02T16:27:44Z

We do keep support for older cmake version (Ubuntu 14).
There are couple of exceptions that require higher version (Cuda->3.8), which means CUDA-enabled optimization won't be available there.
But with standard cmake flags we expect the SDK to be compiled out of the box on Ubuntu 16. (I can't recall which cmake version it has built-in).

BTW, the error I got was on Ubuntu 18 with cmake 3.10.2

morxa · 2020-06-02T17:33:35Z

OK, I'll try to update the PR to also work with older cmake versions.

morxa · 2020-06-15T08:15:49Z

@ev-mp can you please check my new changes? Thanks!

ev-mp · 2020-06-17T16:26:31Z

wrappers/python/CMakeLists.txt

    message(WARNING "Python Bindings being built despite unset option because they are required for python documentation")
 endif()

+if (CMAKE_VERSION VERSION_GREATER_EQUAL 3.12)


@morxa , I moved the testing to the Debian builds but still - the follow statement is not compatible with Cmake v3.5 on Ubuntu 16 and produces the following error

16:49:53 CMake Error at wrappers/python/CMakeLists.txt:10 (if): 16:49:53 if given arguments: 16:49:53 16:49:53 "CMAKE_VERSION" "VERSION_GREATER_EQUAL" "3.12" 16:49:53 16:49:53 Unknown arguments specified 16:49:53 16:49:53 16:49:53 -- Configuring incomplete, errors occurred!

Invoked with the following configuration (python 3.5)
16:49:52 cmake config:../../ -G "Unix Makefiles" -DPYTHON_EXECUTABLE=/usr/bin/python3.5 -DCMAKE_BUILD_TYPE=RelWithDebInfo -DBUILD_EXAMPLES=true -DBUILD_SHARED_LIBS=false -DBUILD_WITH_OPENMP=false -DENFORCE_METADATA=true -DBUILD_CSHARP_BINDINGS=false -DBUILD_PYTHON_BINDINGS=true -DBUILD_PYTHON_DOCS=false -DBUILD_WITH_TM2=true -DCMAKE_INSTALL_PREFIX=../../realsense2

Older cmake does not know VERSION_GREATER_EQUAL, only VERSION_GREATER. :/

I inversed the logic, can you check again? Thanks!

I'll try it again

@morxa ,please rebase it with v2.35.2 or later - the gaps are too big to bridge...

ev-mp · 2020-06-22T08:07:15Z

@morxa hello, the PR now verified on Debians and now I'm also testing it with Pypi.
The only thing I've found so far is that if you invoke the debian target it still builds the examples. It could be an issue with my workspace, but if not - this needs to be addressed in order to build python wrapper on headless distributions.
Additionally, the PR is still stuck on Travis, apparently due to #6595.
Will you be able to rebase it on the current development or at least the above PR?
Sorry for the extra-effort.
Thanks

Python looks for the bindings in its SITEARCH directory, not in LIBDIR. Use Python_SITEARCH/pyrealsense2 as default but allow overriding the install dir by setting PYTHON_INSTALL_DIR.

Having the python bindings in their own target removes the dependency on the bindings if the consumer library does not use the python bindings.

This allows using `find_package(pyrealsense2)` in consumers of the python bindings.

With cmake < 3.12, the FindPython module does not exist yet. Use FindPythonInterp and FindPythonLibs instead.

morxa · 2020-06-22T12:04:39Z

I rebased on development.

I'm not sure I understand the problem with the examples, what do you mean by debian target? I don't see a debian make target. Also, which examples do you mean? The python examples?

ev-mp

I was referring to realsense SDK targets - when invoking make pyrealsense2 I got realsense-viewer built as well. Now when I build it is not reproduced so I think it was indeed a cmake cache glitch.
Anyhow, it's good to go now.
Thanks again for your contribution.

morxa · 2020-06-22T19:30:27Z

Great :) Thanks for the review!

dorodnic added the python label Apr 12, 2020

dorodnic requested a review from ev-mp April 16, 2020 14:47

morxa mentioned this pull request Apr 21, 2020

RFE: Move python cmake target into its own namespace #6124

Closed

ev-mp reviewed Jun 1, 2020

View reviewed changes

morxa requested a review from ev-mp June 2, 2020 18:14

ev-mp reviewed Jun 17, 2020

View reviewed changes

morxa force-pushed the move-python-bindings-into-separate-cmake-target branch 2 times, most recently from 43f1c5e to 7e786da Compare June 17, 2020 20:11

ev-mp changed the base branch from master to development June 21, 2020 19:05

morxa added 4 commits June 22, 2020 13:50

Install python wrapper to python's SITEARCH not to LIBDIR

6032f5b

Python looks for the bindings in its SITEARCH directory, not in LIBDIR. Use Python_SITEARCH/pyrealsense2 as default but allow overriding the install dir by setting PYTHON_INSTALL_DIR.

Move python bindings into their own cmake target

60e721f

Having the python bindings in their own target removes the dependency on the bindings if the consumer library does not use the python bindings.

Add package config file for pyrealsense2

bec7025

This allows using `find_package(pyrealsense2)` in consumers of the python bindings.

Adapt Python detection to older cmake versions

3a4d1e6

With cmake < 3.12, the FindPython module does not exist yet. Use FindPythonInterp and FindPythonLibs instead.

morxa force-pushed the move-python-bindings-into-separate-cmake-target branch from 7e786da to 3a4d1e6 Compare June 22, 2020 11:59

ev-mp approved these changes Jun 22, 2020

View reviewed changes

ev-mp merged commit 76fe540 into IntelRealSense:development Jun 22, 2020

morxa deleted the move-python-bindings-into-separate-cmake-target branch June 22, 2020 19:30

RealSenseSupport mentioned this pull request Jul 16, 2020

AttributeError: module 'pyrealsense2' has no attribute 'pipeline' #6820

Closed

Move python bindings into separate cmake target #6220

Move python bindings into separate cmake target #6220

Uh oh!

Conversation

morxa commented Apr 10, 2020

Uh oh!

dorodnic commented Apr 12, 2020

Uh oh!

lramati commented Apr 12, 2020

Uh oh!

morxa commented May 19, 2020

Uh oh!

ev-mp left a comment

Choose a reason for hiding this comment

Uh oh!

morxa commented Jun 2, 2020

Uh oh!

ev-mp commented Jun 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

morxa commented Jun 2, 2020

Uh oh!

morxa commented Jun 15, 2020

Uh oh!

ev-mp Jun 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morxa Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

ev-mp Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

ev-mp Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

morxa Jun 17, 2020

Choose a reason for hiding this comment

Uh oh!

ev-mp commented Jun 22, 2020

Uh oh!

morxa commented Jun 22, 2020

Uh oh!

ev-mp left a comment

Choose a reason for hiding this comment

Uh oh!

morxa commented Jun 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ev-mp commented Jun 2, 2020 •

edited

Loading

ev-mp Jun 17, 2020 •

edited

Loading