Closed dave992 closed 1 year ago
The design is intentional to decouple the different modules. The current wrappers follow the C++ library exactly, and do not attempt to make things more pythonic. In particular the memory management follows the underlying rules of the C++ library and will segfault if they are not followed correctly. We are thinking about making a higher level adapter that provides a more pythonic interface, but that would be a separate module from these wrappers.
The design is intentional to decouple the different modules.
Okay, I assumed the different ProfileDictionary_addProfile_*
were only there due to some difficulties exposing the addProfiles
function directly instead of it being intentional.
I am not really sure I understand how this decouples the modules. The compiler still needs to pull in the definition of the profiles to create those functions, so on the C++ side, the modules are still coupled. If Swig exposed the addProfiles
member functions directly, this would result in the same situation as far as I understand. Looking at the bindings it created, all Python code directly calls the compiled library and passes on all arguments as is. Only inside the compiled library, the corresponding overload is chosen if available.
The current wrappers follow the C++ library exactly and do not attempt to make things more pythonic.
I did not attempt to make the wrapper more Pythonic, just have a user have the exact same interface and member functions as in C++. For the ProfileDictionary
class specifically, there exists a ProfileDictionary::addProfiles
member function. This does not exist for the Python equivalent.
addProfiles is a template function. Templates in C++ are not generics, and the implementation is only generated when it is used. For C++, this is transparent and handled automatically by the compiler, but for SWIG it needs to be explicitly instantiated. Take a look at the Trajopt unit test. Note this import:
from tesseract_robotics.tesseract_motion_planners_trajopt import TrajOptDefaultPlanProfile, TrajOptDefaultCompositeProfile, \
TrajOptProblemGeneratorFn, TrajOptMotionPlanner, ProfileDictionary_addProfile_TrajOptPlanProfile, \
ProfileDictionary_addProfile_TrajOptCompositeProfile
Because the TrajoptDefaultPlanProfile exists in the tesseract_motion_planners_trajopt module, the corresponding ProfileDictionary_addProfile_TrajOptPlanProfile also exists in that module. This implementation detail of generating the addProfile implementation for TrajoptDefaultPlanProfile would be handled transparently by the C++ compiler, but needs to be handled manually in Python with SWIG.
addProfiles is a template function. Templates in C++ are not generics, and the implementation is only generated when it is used. For C++, this is transparent and handled automatically by the compiler, but for SWIG it needs to be explicitly instantiated
Yes, that's what I tried to describe.
However, by creating the ProfileDictionary_addProfile_TrajOptPlanProfile
function you also explicitly instantiate a function for the TrajoptDefaultPlanProfile
type. I am not sure how this is different to explicitly defining a templated variant for addProfiles
.
Effectively a function is moved from one module to another, disconnecting it from its original "parent" class. The connection between tesseract_command_language
and tesseract_motion_planners_trajopt
still exists inside the library to allow that function to exist, just as would've be the case when addProfile
is exposed directly.
I am not sure I understand how the import is relevant. The ProfileDictionary_addProfile_
is never used on its own as it needs an instance of ProfileDictionary
. If addProfile
was a member function of ProfileDictionary
in Python, it would not need any import as you can just use the object and call the function on it. I do not need to import to use an instance of it. Only if you need to create the object you need to import, but that's valid now as well.
Thanks for the answers in any case, it did make some elements more clear to me.
In #51 I saw a comment in the code of adding a
add_profile
function for theProfileDictionary
class.I have created a small example that could do that using the available wrapper functions and extending the
ProfileDictionary
class in Python. I assumed for this example it is possible to extend a .so library with Python code.I am curious if this approach of extending some of the wrapped classes in Python to mimic the C++ functionality is something you would consider for this repository or if you would rather update the SWIG configuration to achieve the same/similar.
The example as is would likely cause some name clash, but I assume that the C++
ProfileDictionary
could be placed in a separate namespace or be renamed in the binding to do effectively the same thing.