Add Python bindings based on pybind11 #134

diegoferigo · 2020-11-05T16:17:35Z

This PR build upon #131 and provides a first working implementation of Python bindings for the following classes:

Contact
ContactList
ContactPhase
ContactPhaseList
QuinticSpline
IParameterHandler
StdImplementation
SwingFootPlanner
DCMPlanner
TimeVaryingDCMPlanner

Each of these classes are accompanied by some unit testing developed with pytest. I updated the previous example that was using unittest, I believe that pytest is superior in features and integration with other Python tools.

This is the first time I develop Python bindings with pybind11, I'm more used with SWIG. This means that the current implementation is not optimal, I'm still in the process of learning tricks for pybind11. All in all, I can say that coming from SWIG, there are many things that are much easier with pybind11.

Here below some caveats I found:

Since Python is a dynamically typed language, the IParametersHandler APIs logic to select the correct method based on the type cannot work (reliably). I had to define a different overloaded method for each of the supported parameter type.
Mapping manif types is still rudimentary, I tried to expose to Python the underlying Eigen container since pybind11 automatically performs the conversion from/to numpy.
The extensive usage of iterators create many headaches when the methods to bind operate on them. Due to this, ContactPhase::activeContacts is r/o, and some of the ContactList methods have not been exposed to Python. Be aware that the usage theContactList class, particularly, could differ than the C++ counterpart.
Inheritance schemes have been mostly neglected. The Python module is aware of the inheritaces from Advanceable and IParametersHandler, but it's not possible creating Python classes that inherit from them. This is not a big limitation unless we're interested in extending those classes in Python, and it could be addressed if and when we'll going to have this need.

Note: I removed the GenericContainer bindings initially added by @GiulioRomualdi since they're not currently needed and I still have to figure out how to deal with them.

cc @GiulioRomualdi @S-Dafarra @traversaro

bindings/CMakeLists.txt

src/ParametersHandler/include/BipedalLocomotion/ParametersHandler/IParametersHandler.h

src/ParametersHandler/YarpImplementation/src/YarpImplementation.cpp

src/Planners/include/BipedalLocomotion/Planners/Contact.h

S-Dafarra · 2020-11-05T17:56:22Z

Btw, thanks a lot @diegoferigo for this. The whole bindings code is a bit Aramaic for me 😁

bindings/CMakeLists.txt

traversaro · 2020-11-05T18:08:26Z

bindings/python/CMakeLists.txt

+# |-- __init__.py (generated from main bindings CMake file)
+# `-- bindings.<cpython_extension>
+#
+install(TARGETS pybind11_BipedalLocomotion DESTINATION ${PYTHON_INSTDIR})


Not part of the review, just a brainstorming not blocking the code to be merged in any way: I wonder if there is a way to install pybind11 bindings in a way that a library that depends on this one is able to write its own bindings that takes in input and return arguments defined in the pybind11 library of blf.

This is a good point, maybe an advanced and rather uncommon use-case but I think that pybind11 to some extent would allow that. Let me think, in a05407e I changed the structure of the project and it could come handy for this use case. Maybe by making bipedal_locomotion.h public and installing it, I think that downstream projects could use our bindings::Create* functions to add blf types in their modules.

I would leave this out from this first implementation, but thanks for making me think about this possibility.

src/Planners/include/BipedalLocomotion/Planners/Contact.h

bindings/CMakeLists.txt

...andler/YarpImplementation/include/BipedalLocomotion/ParametersHandler/YarpImplementation.tpp

...sHandler/YarpImplementation/include/BipedalLocomotion/ParametersHandler/YarpImplementation.h

DanielePucci · 2020-11-06T21:00:44Z

CC @dic-iit/dynamic-interaction-control

S-Dafarra

@diegoferigo this PR is starting to have two features that are pretty much perpendicular. On one side we have the bindings, and I love that.

On the other hand, we have the modifications on the parameters handler, where I have more concerns and that is enough for a second PR, IMHO.
The biggest concern is the handling of the groups.

cmake/BipedalLocomotionFrameworkFindDependencies.cmake

src/ParametersHandler/YarpImplementation/src/YarpImplementation.cpp

GiulioRomualdi

@diegoferigo, thank you for the effort. You can find some comments through the code.

I completely understand the reason why you decided to use pybind11 instead of swig, however I'm afraid of the amount of code that has to be written if a new class/method will be implemented in the future. For instance, if I'm going to implement new methods in the TimeVarying DCM planner before having them in python the bindings/python/TimeVaryingDCMPlanner.cpp has to be changed (and I'm too lazy to do the same modification twice, also because it may be not trivial. E.g. definition of lambda functions.)

bindings/CMakeLists.txt

GiulioRomualdi · 2020-11-09T17:07:21Z

bindings/python/QuinticSpline.cpp

+        .def("evaluate_point",
+             py::overload_cast<const double&,
+                               Eigen::Ref<Eigen::VectorXd>,
+                               Eigen::Ref<Eigen::VectorXd>,
+                               Eigen::Ref<Eigen::VectorXd>>(
+                 &QuinticSpline::evaluatePoint));


Probably here we can change the python signature to something like

pos, vel, acc = spline.evaluate_point(t)

I think it can be easily done with a lambda function, right?

I didn't edit the implementation of this class after you propose it in the previous pull request (now that I see it, I just used the py::overload_cast instead of the ugly pre-C++14 syntax for overloads). I'll check if it is possible returning a std::tuple<double, double, double>.

Done in 69fa094. Just to understand, why 4 elements?

I've just realized that the dimension of pos is not always 4. It depends on the size of the knots that should be equal to the size of the initial/final velocity/acceleration.

As far as I remember there is no way to retrieve the size of the points, we may think to add a method. in this case, the code may become

Eigen::VectorXd pos(qs.size()); ...

How do you suggest to proceed?

I think you can revert the previous implementation and that I can figure out how to change it.
I can do it just after this PR get merged.

I've rebased this PR to remove 69fa094.

GiulioRomualdi · 2020-11-09T17:20:35Z

bindings/python/SwingFootPlaner.cpp

+        .def_readwrite("is_in_contact", &SwingFootPlannerState::isInContact)
+        .def_property(
+            "transform",
+            [](const SwingFootPlannerState& s) { return s.transform.coeffs(); },


I think that this might be error-prone. Indeed s.tranform is a mainf::SE3d object
https://github.com/dic-iit/bipedal-locomotion-framework/blob/be1e0a2ea0077e3ed745ec6891aac3e71d35824a/src/Planners/include/BipedalLocomotion/Planners/SwingFootPlanner.h#L31
the method coeffs() allow you to access the row coefficient of the SE3d element (7 numbers that represent quaternion + position) The first three-elements represent the translation and the others the quaternion (and actually I don't remember if the conversion is [w x y z] or [x y z w]).

As pointed by @artivis (the maintainer of manif), manually setting the values of the quaternion may lead to a non-unitary quaternion and consequentially to bugs that are complex to spot artivis/manif#160. (cc @prashanthr05 )

Back in time, I implemented a set of setters for the SE3d object that automatically handled all these problems: artivis/manif#166

I think we should continue in this direction.

Thanks for your comment @GiulioRomualdi, I never used manif and I'm not familiar with it.

I addressed your comment in 114225a by first exposing what we need of manif::SE3d and then in 278891f by updating the pybind configuration of the methods that were using that type. Now, since Python is aware of that, we don't have to use lambdas to convert exposed eigen types (supported by pybind) to manif. You can see the result in the updated Python tests of 597b78e.

...andler/YarpImplementation/include/BipedalLocomotion/ParametersHandler/YarpImplementation.tpp

src/ParametersHandler/include/BipedalLocomotion/ParametersHandler/IParametersHandler.h

src/ParametersHandler/src/StdImplementation.cpp

diegoferigo · 2020-11-09T18:33:10Z

Thanks @GiulioRomualdi for the comments

I completely understand the reason why you decided to use pybind11 instead of swig, however I'm afraid of the amount of code that has to be written if a new class/method will be implemented in the future. For instance, if I'm going to implement new methods in the TimeVarying DCM planner before having them in python the bindings/python/TimeVaryingDCMPlanner.cpp has to be changed (and I'm too lazy to do the same modification twice, also because it may be not trivial. E.g. definition of lambda functions.)

I guess this is the price to pay for the decision to use modern standards. To my knowledge there are no valid / widely used / thoroughly documented alternatives to pybind11 for our case. Keep in mind that new contributions to C++ do not strictly need to include also the Python counterpart. If you want to do it, perfect, otherwise the next person that will need the method from Python will implement the missing method. The lambda function is required only if the method uses non-common data types like those from manif. Or maybe I'm just too unfamiliar with pybind11 that I cannot see other ways.

diegoferigo · 2020-11-11T12:10:46Z

Sorry guys but the situation for the get*Names methods is a mess, I spent already too many hours on it and I feel frustrated from the limitations I encountered. To properly implement it, I have to change anyway few things of the storage. I don't personally need this functionality of the YARP backend, I'll remove from this PR all the related code and proceed with just the bindings.

I'll open a PR with the current implementation to keep track of the current code (with the corner case) in case anyone wants to get it back. Feel free to close the new PR right after I open it.

bindings/python/ParametersHandler.cpp

diegoferigo · 2020-11-12T12:46:09Z

I think after the last commits this PR is finalized. Let me know if you have any other comment to address.

S-Dafarra

After these two small things, we are good to merge for me!

bindings/python/bipedal_locomotion.cpp

cmake/BipedalLocomotionFrameworkFindDependencies.cmake

…n using python

GiulioRomualdi · 2020-11-17T21:51:20Z

Hi @diegoferigo I try to replicate the CI error on my laptop. Here I compile in release

ctest -R PythonBindingsUnitTest --verbose
UpdateCTestConfiguration  from :/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build/DartConfiguration.tcl
Parse Config file:/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build/DartConfiguration.tcl
UpdateCTestConfiguration  from :/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build/DartConfiguration.tcl
Parse Config file:/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build/DartConfiguration.tcl
Test project /home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 22
    Start 22: PythonBindingsUnitTest

22: Test command: /usr/bin/python3.6 "-m" "pytest" "/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/bindings/python/tests"
22: Test timeout computed to be: 1500
22: ============================= test session starts ==============================
22: platform linux -- Python 3.6.9, pytest-5.4.3, py-1.9.0, pluggy-0.13.1 -- /usr/bin/python3.6
22: cachedir: .pytest_cache
22: rootdir: /home/gromualdi/robot-code/bipedal-locomotion-framework_swig/bindings/python/tests, inifile: pytest.ini
22: plugins: repeat-0.8.0, cov-2.10.0, rerunfailures-9.0, colcon-core-0.5.10, xonsh-0.6.0
22: collecting ... collected 19 items
22: 
22: ../bindings/python/tests/test_base_types.py::test_se3 FAILED             [  5%]
22: ../bindings/python/tests/test_contact.py::test_contact PASSED            [ 10%]
22: ../bindings/python/tests/test_contact.py::test_contact_list PASSED       [ 15%]
22: ../bindings/python/tests/test_contact.py::test_contact_phase PASSED      [ 21%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_bool PASSED [ 26%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_int PASSED [ 31%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_float PASSED [ 36%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_string PASSED [ 42%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_bool PASSED [ 47%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_int PASSED [ 52%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_float PASSED [ 57%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_string PASSED [ 63%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_mixed PASSED [ 68%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_clear PASSED [ 73%]
22: ../bindings/python/tests/test_quintic_spline.py::test_quintic_spline PASSED [ 78%]
22: ../bindings/python/tests/test_swing_foot_planner.py::test_swing_foot_planner_state PASSED [ 84%]
22: ../bindings/python/tests/test_swing_foot_planner.py::test_swing_foot_planner PASSED [ 89%]
22: ../bindings/python/tests/test_time_verying_dcm_planner.py::test_dcm_planner_state PASSED [ 94%]
22: ../bindings/python/tests/test_time_verying_dcm_planner.py::test_time_varying_dcm_planner PASSED [100%]
22: 
22: =================================== FAILURES ===================================
22: ___________________________________ test_se3 ___________________________________
22: 
22:     def test_se3():
22:     
22:         # Test the default constructor
22:         transform_default = blf.SE3()
22:         assert transform_default.position == pytest.approx(np.array([0, 0, 0]))
22:         assert transform_default.quaternion == pytest.approx(np.array([0, 0, 0, 1]))
22:     
22:         # Test the custom constructor
22:         transform = blf.SE3(position=np.array([0, 1, 2.]),
22:                             quaternion=np.array([0, 1., 0, 0.]))
22:         assert transform.position == pytest.approx([0, 1, 2])
22:         assert transform.quaternion == pytest.approx([0, 1., 0, 0.])
22:     
22:         # Test the position setter
22:         transform.position = np.array([-1, 42.0, 3.14])
22:         assert transform.position == pytest.approx([-1, 42.0, 3.14])
22:     
22:         # Create a quaternion
22:         quaternion_not_normalized = np.array([1, 1, 0, 0])
22:         quaternion = quaternion_not_normalized / np.linalg.norm(quaternion_not_normalized)
22:     
22:         # Setting a quaternion that is not normalized raises an exception
22:         with pytest.raises(ValueError):
22: >           transform.quaternion = quaternion_not_normalized
22: E           Failed: DID NOT RAISE <class 'ValueError'>
22: 
22: ../bindings/python/tests/test_base_types.py:31: Failed
22: =============================== warnings summary ===============================
22: test_swing_foot_planner.py::test_swing_foot_planner
22: test_swing_foot_planner.py::test_swing_foot_planner
22: test_swing_foot_planner.py::test_swing_foot_planner
22:   /usr/lib/python3.6/importlib/_bootstrap.py:219: RuntimeWarning: numpy.ufunc size changed, may indicate binary incompatibility. Expected 192 from C header, got 216 from PyObject
22:     return f(*args, **kwds)
22: 
22: -- Docs: https://docs.pytest.org/en/latest/warnings.html
22: =================== 1 failed, 18 passed, 3 warnings in 2.21s ===================
1/1 Test #22: PythonBindingsUnitTest ...........***Failed    2.86 sec

0% tests passed, 1 tests failed out of 1

Total Test time (real) =   2.86 sec

The following tests FAILED:
	 22 - PythonBindingsUnitTest (Failed)
Errors while running CTest

Indeed if I compile it in debug the test passes

ctest -R PythonBindingsUnitTest --verbose
UpdateCTestConfiguration  from :/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build/DartConfiguration.tcl
Parse Config file:/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build/DartConfiguration.tcl
UpdateCTestConfiguration  from :/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build/DartConfiguration.tcl
Parse Config file:/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build/DartConfiguration.tcl
Test project /home/gromualdi/robot-code/bipedal-locomotion-framework_swig/build
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 22
    Start 22: PythonBindingsUnitTest

22: Test command: /usr/bin/python3.6 "-m" "pytest" "/home/gromualdi/robot-code/bipedal-locomotion-framework_swig/bindings/python/tests"
22: Test timeout computed to be: 1500
22: ============================= test session starts ==============================
22: platform linux -- Python 3.6.9, pytest-5.4.3, py-1.9.0, pluggy-0.13.1 -- /usr/bin/python3.6
22: cachedir: .pytest_cache
22: rootdir: /home/gromualdi/robot-code/bipedal-locomotion-framework_swig/bindings/python/tests, inifile: pytest.ini
22: plugins: repeat-0.8.0, cov-2.10.0, rerunfailures-9.0, colcon-core-0.5.10, xonsh-0.6.0
22: collecting ... collected 19 items
22: 
22: ../bindings/python/tests/test_base_types.py::test_se3 PASSED             [  5%]
22: ../bindings/python/tests/test_contact.py::test_contact PASSED            [ 10%]
22: ../bindings/python/tests/test_contact.py::test_contact_list PASSED       [ 15%]
22: ../bindings/python/tests/test_contact.py::test_contact_phase PASSED      [ 21%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_bool PASSED [ 26%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_int PASSED [ 31%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_float PASSED [ 36%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_string PASSED [ 42%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_bool PASSED [ 47%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_int PASSED [ 52%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_float PASSED [ 57%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_string PASSED [ 63%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_vector_mixed PASSED [ 68%]
22: ../bindings/python/tests/test_parameters_handler_std.py::test_clear PASSED [ 73%]
22: ../bindings/python/tests/test_quintic_spline.py::test_quintic_spline PASSED [ 78%]
22: ../bindings/python/tests/test_swing_foot_planner.py::test_swing_foot_planner_state PASSED [ 84%]
22: ../bindings/python/tests/test_swing_foot_planner.py::test_swing_foot_planner PASSED [ 89%]
22: ../bindings/python/tests/test_time_verying_dcm_planner.py::test_dcm_planner_state PASSED [ 94%]
22: ../bindings/python/tests/test_time_verying_dcm_planner.py::test_time_varying_dcm_planner PASSED [100%]
22: 
22: =============================== warnings summary ===============================
22: test_swing_foot_planner.py::test_swing_foot_planner
22: test_swing_foot_planner.py::test_swing_foot_planner
22: test_swing_foot_planner.py::test_swing_foot_planner
22:   /usr/lib/python3.6/importlib/_bootstrap.py:219: RuntimeWarning: numpy.ufunc size changed, may indicate binary incompatibility. Expected 192 from C header, got 216 from PyObject
22:     return f(*args, **kwds)
22: 
22: -- Docs: https://docs.pytest.org/en/latest/warnings.html
22: ======================== 19 passed, 3 warnings in 2.33s ========================
1/1 Test #22: PythonBindingsUnitTest ...........   Passed    2.85 sec

The following tests passed:
	PythonBindingsUnitTest

100% tests passed, 0 tests failed out of 1

Total Test time (real) =   2.85 sec

traversaro · 2020-11-17T22:05:56Z

22: /usr/lib/python3.6/importlib/_bootstrap.py:219: RuntimeWarning: numpy.ufunc size changed, may indicate binary incompatibility. Expected 192 from C header, got 216 from PyObject

This seems like we are mixing the numpy of pip and the one from apt.

GiulioRomualdi · 2020-11-17T22:13:46Z

@diegoferigo

I investigated on #134 (comment) and I discovered this.
The checks of a unitary quaternion are handled in manif thought MANIF_ASSERT
Please check here: https://github.com/artivis/manif/blob/44bdfebff0fbc56cb189f680212257dc7f20ea58/include/manif/impl/se3/SE3_base.h#L242-L252
however, the MANIF_ASSERT macro is "correctly" called only if MANIF_NO_DEBUG is not defined https://github.com/artivis/manif/blob/6d07bc65cc5b25c49f1231021be5e61132e5f777/include/manif/impl/macro.h#L86-L95

Last but not least MANIF_NO_DEBUG is defined only if the preprocessor NDEBUG is not defined (e.g. in releases) https://github.com/artivis/manif/blob/6d07bc65cc5b25c49f1231021be5e61132e5f777/include/manif/impl/macro.h#L6-L10

Finally, since manif is a template-only library it is compiled with blf so if blf is compiled in release NDEBUG is not defined -> MANIF_NO_DEBUG is defined and MANIF_ASSERT does nothing.

Long story short that exception will never raise in release.

diegoferigo · 2020-11-17T22:20:45Z

Thanks @GiulioRomualdi for the feedback, brilliant! I initially though something similar too, but it could not explain why also the test that compiles BLF in Debug fails with the same error. Do you have any idea?

GiulioRomualdi · 2020-11-17T22:21:27Z

bindings/python/tests/test_time_verying_dcm_planner.py

@@ -0,0 +1,107 @@
+import pytest


Typo in the file name

ouch :) good catch

GiulioRomualdi · 2020-11-17T22:35:19Z

bindings/python/BaseTypes.cpp

+            })
+        .def_property(
+            "quaternion",
+            [](const manif::SE3d& se3) -> Eigen::Vector4d { return se3.coeffs().segment<4>(3); },


Remember that manif stores the quaternion like eigen.

https://eigen.tuxfamily.org/dox/classEigen_1_1Quaternion.html#a4e6800985f1205f94c2e83260ebe90cf

When I inspected manif code, I saw it takes the quaternion and position and serializes them to the coeffs vector. So, when we extract the quaternion, I take the raw data.

Pybind11 does not know the type Eigen::Quaternion so we have in any case to map to to a Eigen::Vector4d if we want automatic conversion to a numpy type.

GiulioRomualdi · 2020-11-17T22:44:36Z

@diegoferigo

Thanks @GiulioRomualdi for the feedback, brilliant! I initially though something similar too, but it could not explain why also the test that compiles BLF in Debug fails with the same error. Do you have any idea?

Actually checking the ci.yml file there seems that -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} is missing on Linux and mac 😢
https://github.com/dic-iit/bipedal-locomotion-framework/blob/3b4822ee977c56f3ba745ad2cedb10f102829bbd/.github/workflows/ci.yml#L300-L319
could you try to add it in the yml file?

diegoferigo · 2020-11-18T08:44:26Z

Thanks @GiulioRomualdi for the feedback, brilliant! I initially though something similar too, but it could not explain why also the test that compiles BLF in Debug fails with the same error. Do you have any idea?

Actually checking the ci.yml file there seems that -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} is missing on Linux and mac 😢

Now everything starts to make sense, thanks @GiulioRomualdi. I saw you merged here master already, let's see if it works at least Debug.

Then, I hope I can squash all those commits, I never tried after a merge commit :) I'm used to push and squash on fork branches, and if needed, rebase.

diegoferigo · 2020-11-18T09:11:19Z

Debug is indeed working, thanks @GiulioRomualdi! I'll fix the PR when I can get back to it.

From https://github.com/Kitware/CMake/tree/v3.18.4/Modules

The assertion is raised only if the project was compiled in Debug. Therefore, testing it would fail in other modes.

diegoferigo · 2020-11-18T14:17:44Z

@GiulioRomualdi in the latest force-push I merged (again) master and squashed all the test commits I did few days ago. Then, I fixed the failing test and disabled Windows (note that all the dependencies are there in case someone wants to finalize its status, only the CMake feature is disables for this platform).

I also moved the find_package of pybind11 after the Python3 one since I think it messes up the process. I explain it better. I think that find_package(pybind11) internally calls something similar to find_package(Python). The problem is that it always finds the system's interpreter if pybind11 was installed with apt on Ubuntu. This happens regardless of the BLF configuration of FindPython3. In CI, Python is installed in /opt, and this enables finding and testing different Python versions. This problem prevents Ubuntu to find the right interpreter. It is likely a subtle pybind11 bug that is too demanding to catch, 0ba025b is a quick workaround.

Edit: I found some related documentation in the pybind11 website.

diegoferigo · 2020-11-18T19:01:48Z

d6d200c fixes also the last problem on macOS. The binary of pytest could be taken also from a Python installation that is not what CMake detects. I added a new Findpytest.cmake file and use to find any pytest in the PATH. CI is green 🎉

GiulioRomualdi · 2020-11-18T20:26:14Z

Perfect @diegoferigo . Just to recap. On Ubuntu and macOS the bindigs are compiled and the test run without error. On windows the bindigs and the test are disabled, right?

diegoferigo · 2020-11-18T20:41:40Z

Perfect @diegoferigo . Just to recap. On Ubuntu and macOS the bindigs are compiled and the test run without error. On windows the bindigs and the test are disabled, right?

Correct.

GiulioRomualdi · 2020-11-18T21:13:27Z

I think we can merge. I then open an issue to track the compilation of the bindings on windows.

@diegoferigo @traversaro @S-Dafarra please give me the 👍 if you agree

diegoferigo · 2020-11-18T21:23:54Z

I then open an issue to track the compilation of the bindings on windows.

For the records, compilation is successful, but the bindings do not work as intended. This workflow is the latest on windows that failed. I don't have experience with Python on Windows and I have no clue why only there things differ.

GiulioRomualdi · 2020-11-19T09:05:42Z

Today it's an important day! We have the first working python bindings for the project. Thank you a lot guys! And thank you @diegoferigo for keeping iterating

GiulioRomualdi requested review from traversaro, GiulioRomualdi and S-Dafarra November 5, 2020 16:34

GiulioRomualdi assigned diegoferigo Nov 5, 2020

diegoferigo force-pushed the diegoferigo/pybind11 branch from 726a005 to 0f06adb Compare November 5, 2020 17:26

S-Dafarra reviewed Nov 5, 2020

View reviewed changes

bindings/CMakeLists.txt Outdated Show resolved Hide resolved

bindings/CMakeLists.txt Outdated Show resolved Hide resolved

S-Dafarra reviewed Nov 5, 2020

View reviewed changes

traversaro reviewed Nov 5, 2020

View reviewed changes

bindings/CMakeLists.txt Outdated Show resolved Hide resolved

traversaro reviewed Nov 5, 2020

View reviewed changes

src/Planners/include/BipedalLocomotion/Planners/Contact.h Outdated Show resolved Hide resolved

diegoferigo force-pushed the diegoferigo/pybind11 branch from 0f06adb to f9ffcd8 Compare November 5, 2020 20:56

S-Dafarra reviewed Nov 6, 2020

View reviewed changes

bindings/CMakeLists.txt Outdated Show resolved Hide resolved

bindings/CMakeLists.txt Show resolved Hide resolved

...andler/YarpImplementation/include/BipedalLocomotion/ParametersHandler/YarpImplementation.tpp Outdated Show resolved Hide resolved

S-Dafarra reviewed Nov 6, 2020

View reviewed changes

...sHandler/YarpImplementation/include/BipedalLocomotion/ParametersHandler/YarpImplementation.h Outdated Show resolved Hide resolved

diegoferigo force-pushed the diegoferigo/pybind11 branch from 5eeb3ef to 4865a32 Compare November 6, 2020 20:53

diegoferigo force-pushed the diegoferigo/pybind11 branch from b48b5f2 to 098c28f Compare November 6, 2020 21:20

S-Dafarra reviewed Nov 9, 2020

View reviewed changes

cmake/BipedalLocomotionFrameworkFindDependencies.cmake Outdated Show resolved Hide resolved

src/ParametersHandler/YarpImplementation/src/YarpImplementation.cpp Outdated Show resolved Hide resolved

GiulioRomualdi reviewed Nov 9, 2020

View reviewed changes

diegoferigo force-pushed the diegoferigo/pybind11 branch from ebaf487 to f52478a Compare November 11, 2020 12:51

S-Dafarra reviewed Nov 11, 2020

View reviewed changes

bindings/python/ParametersHandler.cpp Outdated Show resolved Hide resolved

diegoferigo force-pushed the diegoferigo/pybind11 branch from 41c4270 to 5f65237 Compare November 11, 2020 15:39

S-Dafarra reviewed Nov 12, 2020

View reviewed changes

bindings/python/bipedal_locomotion.cpp Outdated Show resolved Hide resolved

cmake/BipedalLocomotionFrameworkFindDependencies.cmake Outdated Show resolved Hide resolved

diegoferigo force-pushed the diegoferigo/pybind11 branch from 597b78e to e3e37a5 Compare November 12, 2020 14:54

GiulioRomualdi added 3 commits November 12, 2020 15:55

First working version of python bindings

b65a14d

Implement a simple test for the QuinticSpline trajectory generation i…

ec4a163

…n using python

Enable the compilation of the bindings

3a85160

GiulioRomualdi reviewed Nov 17, 2020

View reviewed changes

GiulioRomualdi mentioned this pull request Nov 17, 2020

Fix the Debug compilation in GitHub CI #145

Merged

diegoferigo added 6 commits November 18, 2020 15:04

Merge remote-tracking branch 'origin/master' into diegoferigo/pybind11

5437064

Test the Python bindings in the CI pipeline

c95844d

Vendor FindPython3

f743094

From https://github.com/Kitware/CMake/tree/v3.18.4/Modules

Fix vendored FindPython3 to work in an external project

4995211

Let CMake find pybind after python

0ba025b

Fix python test

ba9232f

The assertion is raised only if the project was compiled in Debug. Therefore, testing it would fail in other modes.

diegoferigo force-pushed the diegoferigo/pybind11 branch from 70d563d to ba9232f Compare November 18, 2020 14:09

Find pytest executable from the system to test the bindings

d6d200c

GiulioRomualdi merged commit 381ad46 into ami-iit:master Nov 19, 2020

diegoferigo deleted the diegoferigo/pybind11 branch November 19, 2020 09:07

GiulioRomualdi mentioned this pull request Nov 23, 2020

Implement the machinery for the generation of the Python bindings #129

Closed

GiulioRomualdi mentioned this pull request Feb 4, 2021

Python Bindings discussion artivis/manif#187

Closed

6 tasks

lrapetti mentioned this pull request Jun 7, 2021

Add python bindings for HDE library robotology/human-dynamics-estimation#255

Open

Add Python bindings based on pybind11 #134

Add Python bindings based on pybind11 #134

Conversation

diegoferigo commented Nov 5, 2020 • edited Loading

S-Dafarra commented Nov 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielePucci commented Nov 6, 2020

S-Dafarra left a comment

Choose a reason for hiding this comment

GiulioRomualdi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GiulioRomualdi Nov 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegoferigo commented Nov 9, 2020

diegoferigo commented Nov 11, 2020 • edited Loading

diegoferigo commented Nov 12, 2020

S-Dafarra left a comment

Choose a reason for hiding this comment

GiulioRomualdi commented Nov 17, 2020 • edited Loading

traversaro commented Nov 17, 2020

GiulioRomualdi commented Nov 17, 2020 • edited Loading

diegoferigo commented Nov 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegoferigo Nov 17, 2020 • edited Loading

Choose a reason for hiding this comment

GiulioRomualdi commented Nov 17, 2020

diegoferigo commented Nov 18, 2020 • edited Loading

diegoferigo commented Nov 18, 2020

diegoferigo commented Nov 18, 2020 • edited Loading

diegoferigo commented Nov 18, 2020

GiulioRomualdi commented Nov 18, 2020

diegoferigo commented Nov 18, 2020

GiulioRomualdi commented Nov 18, 2020 • edited Loading

diegoferigo commented Nov 18, 2020

GiulioRomualdi commented Nov 19, 2020

diegoferigo commented Nov 5, 2020 •

edited

Loading

GiulioRomualdi Nov 13, 2020 •

edited

Loading

diegoferigo commented Nov 11, 2020 •

edited

Loading

GiulioRomualdi commented Nov 17, 2020 •

edited

Loading

GiulioRomualdi commented Nov 17, 2020 •

edited

Loading

diegoferigo Nov 17, 2020 •

edited

Loading

diegoferigo commented Nov 18, 2020 •

edited

Loading

diegoferigo commented Nov 18, 2020 •

edited

Loading

GiulioRomualdi commented Nov 18, 2020 •

edited

Loading