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

Add python bindings option #1383

Closed

Conversation

ahoarau
Copy link
Contributor

@ahoarau ahoarau commented Jul 9, 2019

This just adds a python binding option that defaults to ON to be safe.
This is in part due to #1382

CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please motivate the relation to #1382?
How do you expect users to change this option? Usually, building with catkin you cannot easily pass options to individual packages.

Co-Authored-By: Robert Haschke <rhaschke@users.noreply.github.com>
@ahoarau
Copy link
Contributor Author

ahoarau commented Jul 16, 2019

I'm using the c++ librviz for one of my projet. I had the exact same error as #1382 that stopped me from compiling anything on melodic. One way to pass the option :

catkin config --cmake-args -DBUILD_PYTHON_BINDINGS=OFF

But it applies for all the packages in the ws, it's true. Better than nothing :)
Copying the sources in the project that needs librviz and adding it with add_subdirectory(rviz) might work just as well and allow to pass the option to it individually.

@ahoarau
Copy link
Contributor Author

ahoarau commented Aug 22, 2019

So are there any issues on this one ?

@rhaschke
Copy link
Contributor

The issue #1382 should be solved via ros-visualization/python_qt_binding#64.
Could you please verify? If so, I suggest to cancel this PR.

catkin config --cmake-args -DBUILD_PYTHON_BINDINGS=OFF

The option name is too generic. Better use something like RVIZ_PYTHON_BINDINGS.

@rhaschke
Copy link
Contributor

rhaschke commented Oct 3, 2019

Closing due to inactivity. Issue should be fixed as indicated in #1383 (comment)

@rhaschke rhaschke closed this Oct 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants