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 CMake macro for generating pybind11 bindings #215

Open
Bi0T1N opened this issue Mar 23, 2022 · 2 comments
Open

Add CMake macro for generating pybind11 bindings #215

Bi0T1N opened this issue Mar 23, 2022 · 2 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed scripting Scripting interfaces to Ignition

Comments

@Bi0T1N
Copy link
Contributor

Bi0T1N commented Mar 23, 2022

Desired behavior

Right now there is some code duplication in ign-gazebo and ign-math for finding pybind11. I assume the goal is to provide Python interfaces also for other packages in the future, thus it makes sense to have a common macro for it.

Implementation suggestion

Since PythonLibs is deprecated, the snippet should also use FindPython3 which will be provided with PR #213 .
It might be useful to extend the new IgnPython to provide several arguments as input in order to reuse the existing code but this might require some additional effort than creating an independent IgnPythonBindings macro.

@Bi0T1N Bi0T1N added the enhancement New feature or request label Mar 23, 2022
@chapulina chapulina added the scripting Scripting interfaces to Ignition label Mar 23, 2022
@chapulina chapulina added the help wanted Extra attention is needed label Mar 23, 2022
@j-rivero
Copy link
Contributor

Right now there is some code duplication in ign-gazebo and ign-math for finding pybind11. I assume the goal is to provide Python interfaces also for other packages in the future, thus it makes sense to have a common macro for it.

While packaging new versions of ign-gazebo/ign-math I had the same thought. It would be great if we can de-duplicate the code and use a unify macro for it.

It might be useful to extend the new IgnPython to provide several arguments as input in order to reuse the existing code but this might require some additional effort than creating an independent IgnPythonBindings macro.

Currently IgnPython works as a CMake Module working just by including it. We can add a macro in the same file and avoid the creation of a new file in the repository. That could make sense since there are probably no use cases where someone needs to use the python bindings macro but don't want to check for a python3 installation.

@Bi0T1N
Copy link
Contributor Author

Bi0T1N commented Apr 2, 2022

It might be useful to extend the new IgnPython to provide several arguments as input in order to reuse the existing code but this might require some additional effort than creating an independent IgnPythonBindings macro.

Currently IgnPython works as a CMake Module working just by including it. We can add a macro in the same file and avoid the creation of a new file in the repository. That could make sense since there are probably no use cases where someone needs to use the python bindings macro but don't want to check for a python3 installation.

I made the needed changes for pybind11 in gazebosim/gz-math#402 to fix the related issue.
There is a small difference, we need find_package(Python3 QUIET COMPONENTS Interpreter Development) instead of find_package(Python3 QUIET COMPONENTS Interpreter) = find_package(Python3 QUIET) which is used in ign-cmake. We could use a macro which also searches for Development but in my opinion it'd be nicer to use a different include statement.
macro:

include(IgnPython)
search_pybind11_library() # does a find_package(Python3) twice, once without Development and once with (+ there are even more options like numpy/different developments)

separate module:

include(IgnPythonBindings)

I don't know but might it be possible to do include(IgnPython *my_arguments_list*)? Then the items to be searched for can directly be declared within the include command.

However, for similarity between different Python 'includes' I think it would make sense to have a common way on how to use/find Python3 - a single macro for each case of finding Python3, a macro with several arguments to specify the needs or separate and specific include modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed scripting Scripting interfaces to Ignition
Projects
None yet
Development

No branches or pull requests

3 participants