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

Fixup PR 2547 #2

Conversation

rhaschke
Copy link

This comprises two commits to fixup open issues in moveit#2547.

- drop optional building of python bindings
- ignore undefined_target error
- use private includes/libs for pybind11 module
@PeterMitrano
Copy link

PeterMitrano commented Mar 31, 2021

works for me -- just to clarify though, the example you gave of

import moveit
moveit.core.xxx

is expected to not work, correct? Whereas import moveit.core and from moveit import core still work as expected

once that's confirmed I'll merge this and push it to the main PR

@rhaschke
Copy link
Author

Exactly.

@PeterMitrano PeterMitrano merged commit 55070f8 into UM-ARM-Lab:planning-scene-pybindings Mar 31, 2021
@rhaschke rhaschke deleted the pr/PeterMitrano/2547 branch April 6, 2021 19:07
@PeterMitrano
Copy link

@rhaschke I tried to extend the bindings to another package and somehow I'm running into this issue again, where the devel-space install doesn't work.

Right now both moveit_core and any new package (say moveit_ros_planning) both try to generate devel/lib/python3/dist-packages/moveit/__init__.py which are in conflict. I can't wrap my around how this was supposed to work? I really thought I tested it this already but I'm not sure. Any ideas?

@rhaschke
Copy link
Author

I can confirm that devel-space install doesn't work yet, because of conflicting __init__.py files generated by catkin during build.
These generated files inject the source path of the current package into the python search path, such that the source files are found when importing from devel space. However, this scheme doesn't work for namespaced packages. I'm not sure anybody is still maintaining the underlying catkin code 😞

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

Successfully merging this pull request may close these issues.

2 participants