Merge pull request #2475 from jorisv/topic/fix_libpython_option

Add pinocchio_python_parser target

CHANGELOG.md

@@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 
+### Added
+- Add `pinocchio_python_parser` target ([#2475](https://github.com/stack-of-tasks/pinocchio/pull/2475))
+
 ## [3.3.0] - 2024-11-06
 
 ### Added

CMakeLists.txt

@@ -134,9 +134,8 @@ cmake_dependent_option(
   BUILD_PYTHON_BINDINGS_WITH_BOOST_MPFR_SUPPORT
   "Build the Python interface with Boost.Multiprecision MPFR support" OFF BUILD_PYTHON_INTERFACE
   OFF)
-cmake_dependent_option(
-  BUILD_WITH_LIBPYTHON "Link to libpython to embed an interpreter for the python_parser feature" ON
-  "BUILD_PYTHON_INTERFACE" OFF)
+cmake_dependent_option(BUILD_WITH_LIBPYTHON "Build the library with Python format support" ON
+                       "BUILD_PYTHON_INTERFACE" OFF)
 if(APPLE)
   option(BUILD_WITH_ACCELERATE_SUPPORT "Build Pinocchio with the Accelerate support" OFF)
 else(APPLE)
@@ -164,6 +163,9 @@ endif()
 if(BUILD_WITH_AUTODIFF_SUPPORT)
   set(BUILD_WITH_CPPAD_SUPPORT TRUE)
 endif()
+if(BUILD_WITH_LIBPYTHON)
+  set(BUILD_WITH_PYTHON_PARSER_SUPPORT TRUE)
+endif()
 
 if(BUILD_WITH_EXTRA_SUPPORT)
   list(APPEND CFLAGS_OPTIONS "-DPINOCCHIO_WITH_EXTRA_SUPPORT")
@@ -301,7 +303,7 @@ if(BUILD_PYTHON_INTERFACE)
 
   message(STATUS "Python compiler: ${_python_implementation_value}")
   if(_python_implementation_value MATCHES "PyPy")
-    set(BUILD_WITH_LIBPYTHON OFF)
+    set(BUILD_WITH_PYTHON_PARSER_SUPPORT OFF)
     message(
       STATUS
         "PyPy detected, therefore libpython is not available and BUILD_WITH_LIBPYTHON set to OFF.")
@@ -364,15 +366,6 @@ if(BUILD_WITH_EXTRA_SUPPORT)
        ${${PROJECT_NAME}_BINDINGS_PYTHON_EXTRA_SOURCES})
 endif()
 
-# LibPython sources
-if(BUILD_WITH_LIBPYTHON)
-  list(APPEND ${PROJECT_NAME}_CORE_PUBLIC_HEADERS ${${PROJECT_NAME}_LIBPYTHON_PUBLIC_HEADERS})
-  list(APPEND ${PROJECT_NAME}_BINDINGS_PYTHON_SOURCES
-       ${${PROJECT_NAME}_BINDINGS_PYTHON_LIBPYTHON_SOURCES})
-  list(APPEND ${PROJECT_NAME}_BINDINGS_PYTHON_PUBLIC_HEADERS
-       ${${PROJECT_NAME}_BINDINGS_PYTHON_LIBPYTHON_PUBLIC_HEADERS})
-endif(BUILD_WITH_LIBPYTHON)
-
 # HPP-FCL sources
 if(BUILD_WITH_HPP_FCL_SUPPORT)
   if(ENABLE_TEMPLATE_INSTANTIATION)
@@ -499,6 +492,10 @@ if(BUILD_WITH_EXTRA_SUPPORT)
   export_variable(PINOCCHIO_USE_EXTRA ON)
   set(PACKAGE_EXTRA_MACROS "${PACKAGE_EXTRA_MACROS}\nset(PINOCCHIO_USE_EXTRA \"\")")
 endif()
+if(BUILD_WITH_PYTHON_PARSER_SUPPORT)
+  export_variable(PINOCCHIO_USE_PYTHON_PARSER ON)
+  set(PACKAGE_EXTRA_MACROS "${PACKAGE_EXTRA_MACROS}\nset(PINOCCHIO_USE_PYTHON_PARSER \"\")")
+endif()
 export_variable(pinocchio_VERSION ${pinocchio_VERSION})
 
 # Setup pkg-configs flags and libs

include/pinocchio/bindings/python/parsers/python.hpp

@@ -5,39 +5,12 @@
 #ifndef __pinocchio_python_parser_python_hpp__
 #define __pinocchio_python_parser_python_hpp__
 
-#include "pinocchio/multibody/model.hpp"
+#include "pinocchio/macros.hpp"
 
-#include <boost/python.hpp>
+// clang-format off
+PINOCCHIO_PRAGMA_DEPRECATED_HEADER(pinocchio/bindings/python/parsers/python.hpp, pinocchio/parsers/python.hpp)
+// clang-format on
 
-#if defined _WIN32
-  #ifdef pinocchio_pywrap_EXPORTS
-    #define PINOCCHIO_PYWRAP_DLLAPI __declspec(dllexport)
-  #else
-    #define PINOCCHIO_PYWRAP_DLLAPI __declspec(dllimport)
-  #endif // pinocchio_pywrap_EXPORTS
-#else
-  #define PINOCCHIO_PYWRAP_DLLAPI
-#endif // _WIN32
-
-namespace pinocchio
-{
-  namespace python
-  {
-    /// \brief Load a model from a Python script.
-    ///
-    /// This function raises a Python error in case of incistency in the Python code.
-    ///
-    /// \input filename The full path to the model file.
-    /// \input var_name Name of the Python variable which contains the model in the script.
-    ///
-    /// \returns The model constructed by the Python script.
-    ///
-    // TODO: look inside the context of Python and find an occurence of object Model
-    PINOCCHIO_PYWRAP_DLLAPI
-    Model buildModel(const std::string & filename, const std::string & var_name = "model");
-
-  } // namespace python
-
-} // namespace pinocchio
+#include "pinocchio/parsers/python.hpp"
 
 #endif // ifndef __pinocchio_python_parser_python_hpp__

include/pinocchio/parsers/python.hpp

@@ -5,6 +5,28 @@
 #ifndef __pinocchio_parser_python_hpp__
 #define __pinocchio_parser_python_hpp__
 
-#include "pinocchio/bindings/python/parsers/python.hpp"
+#include "pinocchio/python_parser/config.hpp"
+#include "pinocchio/multibody/model.hpp"
+
+namespace pinocchio
+{
+  namespace python
+  {
+    /// \brief Load a model from a Python script.
+    ///
+    /// This function raises a Python error in case of inconsistency in the Python code.
+    ///
+    /// \input filename The full path to the model file.
+    /// \input var_name Name of the Python variable which contains the model in the script.
+    ///
+    /// \returns The model constructed by the Python script.
+    ///
+    // TODO: look inside the context of Python and find an occurence of object Model
+    PINOCCHIO_PYTHON_PARSER_DLLAPI
+    Model buildModel(const std::string & filename, const std::string & var_name = "model");
+
+  } // namespace python
+
+} // namespace pinocchio
 
 #endif // ifndef __pinocchio_parser_python_hpp__

sources.cmake

@@ -366,9 +366,11 @@ set(${PROJECT_NAME}_SDF_PUBLIC_HEADERS
     ${PROJECT_SOURCE_DIR}/include/pinocchio/parsers/sdf/model.hxx
     ${PROJECT_SOURCE_DIR}/include/pinocchio/parsers/sdf/geometry.hxx)
 
-set(${PROJECT_NAME}_LIBPYTHON_PUBLIC_HEADERS
+set(${PROJECT_NAME}_PYTHON_PARSER_PUBLIC_HEADERS
     ${PROJECT_SOURCE_DIR}/include/pinocchio/parsers/python.hpp)
 
+set(${PROJECT_NAME}_PYTHON_PARSER_SOURCES ${PROJECT_SOURCE_DIR}/src/parsers/python/model.cpp)
+
 set(${PROJECT_NAME}_EXTRA_SOURCES ${PROJECT_SOURCE_DIR}/src/extra/reachable-workspace.cpp)
 
 set(${PROJECT_NAME}_EXTRA_PUBLIC_HEADERS
@@ -594,12 +596,6 @@ set(${PROJECT_NAME}_BINDINGS_PYTHON_SOURCES
     ${PROJECT_SOURCE_DIR}/bindings/python/parsers/mjcf/geometry.cpp
     ${PROJECT_SOURCE_DIR}/bindings/python/extra/expose-extras.cpp)
 
-set(${PROJECT_NAME}_BINDINGS_PYTHON_LIBPYTHON_SOURCES
-    ${PROJECT_SOURCE_DIR}/bindings/python/parsers/python/model.cpp)
-
-set(${PROJECT_NAME}_BINDINGS_PYTHON_LIBPYTHON_PUBLIC_HEADERS
-    ${PROJECT_SOURCE_DIR}/include/pinocchio/bindings/python/parsers/python.hpp)
-
 set(${PROJECT_NAME}_BINDINGS_PYTHON_HPP_FCL_SOURCES
     ${PROJECT_SOURCE_DIR}/bindings/python/collision/expose-broadphase.cpp
     ${PROJECT_SOURCE_DIR}/bindings/python/collision/expose-broadphase-callbacks.cpp

src/CMakeLists.txt

@@ -325,6 +325,20 @@ if(BUILD_WITH_CASADI_SUPPORT)
   target_link_libraries(${PROJECT_NAME}_casadi ${CASADI_SCOPE} casadi)
 endif()
 
+if(BUILD_WITH_PYTHON_PARSER_SUPPORT)
+  set(PYTHON_PARSER_LIB_NAME "${PROJECT_NAME}_python_parser")
+
+  pinocchio_target(
+    ${PYTHON_PARSER_LIB_NAME}
+    SCALAR default
+    SOURCES ${${PROJECT_NAME}_PYTHON_PARSER_SOURCES}
+            ${${PROJECT_NAME}_PYTHON_PARSER_PUBLIC_HEADERS})
+  pinocchio_config(python_parser ${PYTHON_PARSER_LIB_NAME})
+
+  target_link_libraries(${PYTHON_PARSER_LIB_NAME} PUBLIC ${PROJECT_NAME}_default Python3::Python)
+  target_link_boost_python(${PYTHON_PARSER_LIB_NAME} PUBLIC)
+endif()
+
 # Define main target (default, parsers, extra).
 add_library(${PROJECT_NAME} INTERFACE)
 add_library(${PROJECT_NAME}::${PROJECT_NAME} ALIAS ${PROJECT_NAME})

bindings/python/parsers/python/model.cpp → src/parsers/python/model.cpp

@@ -2,13 +2,13 @@
 // Copyright (c) 2016-2023 CNRS INRIA
 //
 
-#include "pinocchio/bindings/python/parsers/python.hpp"
+#include "pinocchio/parsers/python.hpp"
 
-#include <iostream>
-#include <Python.h>
 #include <boost/version.hpp>
 #include <boost/algorithm/string/predicate.hpp>
 
+#include <boost/python.hpp>
+
 // Boost 1.58
 #if BOOST_VERSION / 100 % 1000 == 58
   #include <fstream>
@@ -28,11 +28,6 @@ namespace pinocchio
       // Get a dict for the global namespace to exec further python code with
       bp::dict globals = bp::extract<bp::dict>(main_module.attr("__dict__"));
 
-      // We need to link to the pinocchio PyWrap. We delegate the dynamic loading to the python
-      // interpreter.
-      bp::object cpp_module(
-        (bp::handle<>(bp::borrowed(PyImport_AddModule("libpinocchio_pywrap")))));
-
       // That's it, you can exec your python script, starting with a model you
       // can update as you want.
       try
@@ -67,10 +62,6 @@ namespace pinocchio
 
       // close the interpreter
       // cf. https://github.com/numpy/numpy/issues/8097
-#if PY_MAJOR_VERSION < 3
-      Py_Finalize();
-#else
-
       PyObject * poMainModule = PyImport_AddModule("__main__");
       PyObject * poAttrList = PyObject_Dir(poMainModule);
       PyObject * poAttrIter = PyObject_GetIter(poAttrList);
@@ -94,8 +85,6 @@ namespace pinocchio
       }
       Py_DecRef(poAttrIter);
       Py_DecRef(poAttrList);
-#endif
-
       return model;
     }
   } // namespace python

unittest/CMakeLists.txt

@@ -38,6 +38,7 @@ function(ADD_PINOCCHIO_UNIT_TEST name)
       EXTRA
       COLLISION
       PARALLEL
+      PYTHON_PARSER
       PARSERS_OPTIONAL
       EXTRA_OPTIONAL
       COLLISION_OPTIONAL
@@ -93,6 +94,22 @@ function(ADD_PINOCCHIO_UNIT_TEST name)
     target_link_libraries(${TEST_NAME} PUBLIC ${PROJECT_NAME}_extra)
   endif()
 
+  if(unit_test_PYTHON_PARSER)
+    target_link_libraries(${TEST_NAME} PUBLIC ${PROJECT_NAME}_python_parser)
+    add_windows_dll_path_to_test(${TEST_NAME})
+    get_test_property(${TEST_NAME} ENVIRONMENT ENV_VARIABLES)
+    compute_pythonpath(PYTHON_ENV_VARIABLES "bindings/python")
+    list(APPEND ENV_VARIABLES ${PYTHON_ENV_VARIABLES})
+    if(WIN32)
+      # This line is mandatory because of Github action. The test run well on Windows + Conda. This
+      # hide something wrong. Maybe the test is linking against the wrong Python library or call the
+      # wrong interpreter.
+      get_filename_component(_PYTHONHOME ${PYTHON_EXECUTABLE} PATH)
+      list(APPEND ENV_VARIABLES "PYTHONHOME=${_PYTHONHOME}")
+    endif()
+    set_tests_properties(${TEST_NAME} PROPERTIES ENVIRONMENT "${ENV_VARIABLES}")
+  endif()
+
   modernize_target_link_libraries(
     ${TEST_NAME}
     SCOPE PRIVATE
@@ -222,24 +239,11 @@ if(BUILD_WITH_EXTRA_SUPPORT)
     COLLISION_OPTIONAL)
 endif()
 
-if(BUILD_WITH_LIBPYTHON)
-  add_pinocchio_unit_test(python_parser PACKAGES Python3::Python)
-  get_cpp_test_name(python_parser ${CMAKE_CURRENT_SOURCE_DIR} python_parser_target)
-  target_include_directories(${python_parser_target} SYSTEM PUBLIC ${PYTHON_INCLUDE_DIRS})
+if(BUILD_WITH_PYTHON_PARSER_SUPPORT)
+  add_pinocchio_unit_test(python_parser PYTHON_PARSER)
+endif()
 
-  target_link_libraries(${python_parser_target} PUBLIC ${PYWRAP}_default)
-  target_link_libraries(${python_parser_target} PUBLIC ${PYTHON_LIBRARIES})
-  get_test_property(${python_parser_target} ENVIRONMENT ENV_VARIABLES)
-  compute_pythonpath(PYTHON_ENV_VARIABLES "bindings/python")
-  list(APPEND ENV_VARIABLES ${PYTHON_ENV_VARIABLES})
-  if(WIN32)
-    # This line is mandatory because of Github action. The test run well on Windowns + Conda. This
-    # hide something wrong. Maybe the test is linking against the wrong Python library or call the
-    # wrong interpreter.
-    get_filename_component(_PYTHONHOME ${PYTHON_EXECUTABLE} PATH)
-    list(APPEND ENV_VARIABLES "PYTHONHOME=${_PYTHONHOME}")
-  endif()
-  set_tests_properties(${python_parser_target} PROPERTIES ENVIRONMENT "${ENV_VARIABLES}")
+if(BUILD_PYTHON_INTERFACE)
   add_subdirectory(python)
 endif()
 

unittest/python_parser.cpp

@@ -6,7 +6,7 @@
 
 #include "pinocchio/multibody/model.hpp"
 #include "pinocchio/multibody/data.hpp"
-#include "pinocchio/bindings/python/parsers/python.hpp"
+#include "pinocchio/parsers/python.hpp"
 
 #include <boost/test/unit_test.hpp>