From 24c22d3231a4c88d5d29c995b2f3fd7a1fb75ba7 Mon Sep 17 00:00:00 2001 From: Damien Marchal Date: Sat, 29 Aug 2020 08:49:19 +0200 Subject: [PATCH 01/11] [SofaPython3] Remove -at-best- the use of this infamous string conversion while creating an object. --- Plugin/src/SofaPython3/DataHelper.cpp | 32 +++- Plugin/src/SofaPython3/DataHelper.h | 177 +++++++----------- .../SofaPython3/Sofa/Core/Binding_Node.cpp | 21 ++- 3 files changed, 110 insertions(+), 120 deletions(-) diff --git a/Plugin/src/SofaPython3/DataHelper.cpp b/Plugin/src/SofaPython3/DataHelper.cpp index 3e73f0db..e418542e 100644 --- a/Plugin/src/SofaPython3/DataHelper.cpp +++ b/Plugin/src/SofaPython3/DataHelper.cpp @@ -45,25 +45,48 @@ std::string toSofaParsableString(const py::handle& p) if(py::isinstance(p) || py::isinstance(p)) { std::stringstream tmp; - for(auto pa : p){ + for(auto pa : p) + { tmp << toSofaParsableString(pa) << " "; } return tmp.str(); } //TODO(dmarchal) This conversion to string is so bad. if(py::isinstance(p)) + { return py::str(p); + } + return py::repr(p); } -/// RVO optimized function. Don't care about copy on the return code. void fillBaseObjectdescription(sofa::core::objectmodel::BaseObjectDescription& desc, - const py::dict& dict) + const py::dict& dict) + { + for(auto kv : dict) + { + desc.setAttribute(py::str(kv.first), toSofaParsableString(kv.second)); + } + + return; + } + +void processKwargsForObjectCreation(const py::dict dict, + py::list& parametersToLink, + py::list& parametersToCopy, + sofa::core::objectmodel::BaseObjectDescription& parametersAsString) { + auto t = py::detail::get_type_handle(typeid(BaseData), false); for(auto kv : dict) { - desc.setAttribute(py::str(kv.first), toSofaParsableString(kv.second)); + if (py::isinstance(kv.second, t)) + parametersToLink.append(kv.first); + else if (py::isinstance(kv.second)) + parametersAsString.setAttribute(py::str(kv.first), py::cast(kv.second)); + else + parametersToCopy.append(kv.first); } + return; } PythonTrampoline::~PythonTrampoline(){} @@ -76,6 +99,7 @@ void PythonTrampoline::setInstance(py::object s) pyobject = std::shared_ptr( s.ptr(), [](PyObject* ob) { + SOFA_UNUSED(ob); // runSofa Sofa/tests/pyfiles/ScriptController.py => CRASH // Py_DECREF(ob); }); diff --git a/Plugin/src/SofaPython3/DataHelper.h b/Plugin/src/SofaPython3/DataHelper.h index a73388c9..710a92ed 100644 --- a/Plugin/src/SofaPython3/DataHelper.h +++ b/Plugin/src/SofaPython3/DataHelper.h @@ -42,119 +42,63 @@ along with sofaqtquick. If not, see . ////////////////////////// FORWARD DECLARATION /////////////////////////// namespace sofa { - namespace defaulttype { - class AbstractTypeInfo; - } - namespace core { - namespace objectmodel { - class BaseData; - - - class SOFAPYTHON3_API PrefabLink - { - public: - PrefabLink() {} - PrefabLink(const Base::SPtr& targetBase) { m_targetBase = targetBase; } - PrefabLink(BaseLink* targetLink) { m_targetBase = targetLink->getLinkedBase(); } - PrefabLink(const std::string& targetPath) { m_targetPath = targetPath; } - - const Base::SPtr& getTargetBase() const { return m_targetBase; } - void setTargetBase(const Base::SPtr& targetBase) { m_targetBase = targetBase; } - - const std::string& getTargetPath() const { return m_targetPath; } - void setTargetPath(const std::string& targetPath) { m_targetPath = targetPath; } - - friend std::ostream& operator << ( std::ostream& out, const PrefabLink& l) - { - if (l.getTargetBase()) - { - auto bn = l.getTargetBase()->toBaseNode(); - auto bo = l.getTargetBase()->toBaseObject(); - out << "@" + (bn ? bn->getPathName() : bo->getPathName()); - } - out << l.getTargetPath(); - return out; - } - - friend std::istream& operator >> ( std::istream& in, PrefabLink& l) - { - std::string s; - in >> s; - l.setTargetPath(s); - return in; - } - - private: - Base::SPtr m_targetBase { nullptr }; - std::string m_targetPath {""}; - }; - - class SOFAPYTHON3_API DataLink : public Data - { - typedef Data Inherit; - - DataLink( const std::string& helpMsg="", bool isDisplayed=true, bool isReadOnly=false ) - : Inherit(helpMsg, isDisplayed, isReadOnly) - { - } - - DataLink( const std::string& value, const std::string& helpMsg="", bool isDisplayed=true, bool isReadOnly=false ) - : Inherit(value, helpMsg, isDisplayed, isReadOnly) - { - } - - explicit DataLink(const BaseData::BaseInitData& init) - : Inherit(init) - { - } - - const PrefabLink& getValue() const - { - updateIfDirty(); - if (m_value.getValue().getTargetBase()) return m_value.getValue(); - - auto self = const_cast(this); - - Base* dst = nullptr; - this->getOwner()->findLinkDest(dst, self->m_value.getValue().getTargetPath(), nullptr); - if (dst) { - auto edit = self->m_value.beginEdit(); - edit->setTargetBase(dst); - edit->setTargetPath(""); - self->m_value.endEdit(); - } - return m_value.getValue(); - } - - std::string getValueString() const - { - const auto& ptr = getValue(); - if (ptr.getTargetBase()) - { - auto bn = ptr.getTargetBase()->toBaseNode(); - auto bo = ptr.getTargetBase()->toBaseObject(); - return "@" + (bn ? bn->getPathName() : bo->getPathName()); - } - return ptr.getTargetPath(); - } - - - bool read(const std::string& value) - { - Base* dst; - auto data = m_value.beginEdit(); - if (this->getOwner()->findLinkDest(dst, value, nullptr) && dst != nullptr) - data->setTargetBase(dst); - else { - data->setTargetBase(nullptr); - data->setTargetPath(value); - } - return true; - } - }; +namespace defaulttype { +class AbstractTypeInfo; +} +namespace core { +namespace objectmodel { +class BaseData; + + +class SOFAPYTHON3_API PrefabLink +{ +public: + PrefabLink() {} + PrefabLink(const Base::SPtr& targetBase) { m_targetBase = targetBase; } + PrefabLink(BaseLink* targetLink) { m_targetBase = targetLink->getLinkedBase(); } + PrefabLink(const std::string& targetPath) { m_targetPath = targetPath; } + const Base::SPtr& getTargetBase() const { return m_targetBase; } + void setTargetBase(const Base::SPtr& targetBase) { m_targetBase = targetBase; } + + const std::string& getTargetPath() const { return m_targetPath; } + void setTargetPath(const std::string& targetPath) { m_targetPath = targetPath; } + + friend std::ostream& operator << ( std::ostream& out, const PrefabLink& l) + { + if (l.getTargetBase()) + { + auto bn = l.getTargetBase()->toBaseNode(); + auto bo = l.getTargetBase()->toBaseObject(); + out << "@" + (bn ? bn->getPathName() : bo->getPathName()); } + out << l.getTargetPath(); + return out; + } + + friend std::istream& operator >> ( std::istream& in, PrefabLink& l) + { + std::string s; + in >> s; + l.setTargetPath(s); + return in; } + +private: + Base::SPtr m_targetBase { nullptr }; + std::string m_targetPath {""}; +}; +} +} +namespace defaulttype +{ +template <> +struct DataTypeName +{ + static const char* name() { return "PrefabLink"; } +}; + +} } /////////////////////////////// DECLARATION ////////////////////////////// @@ -192,7 +136,7 @@ template class py_shared_ptr : public sofa::core::sptr SOFAPYTHON3_API void setItem2D(py::array a, py::slice slice, py::object o); SOFAPYTHON3_API void setItem2D(py::array a, const py::slice& slice, - const py::slice& slice1, py::object o); + const py::slice& slice1, py::object o); SOFAPYTHON3_API void setItem1D(py::array a, py::slice slice, py::object o); SOFAPYTHON3_API void setItem(py::array a, py::slice slice, py::object value); @@ -216,11 +160,16 @@ void SOFAPYTHON3_API copyFromListScalar(BaseData& d, const AbstractTypeInfo& nfo std::string SOFAPYTHON3_API toSofaParsableString(const py::handle& p); -//py::object SOFAPYTHON3_API dataToPython(BaseData* d); - /// RVO optimized function. Don't care about copy on the return code. void SOFAPYTHON3_API fillBaseObjectdescription(sofa::core::objectmodel::BaseObjectDescription& desc, - const py::dict& dict); + const py::dict& dict); + +/// Split the content of the dictionnary 'dict' in three set. +/// On containing the data to parent, one containing the data to copy and on containing the data to parse in the BaseObjectDescription +void SOFAPYTHON3_API processKwargsForObjectCreation(const py::dict dict, + py::list& parametersToLink, + py::list& parametersToCopy, + sofa::core::objectmodel::BaseObjectDescription& parametersAsString); template void copyScalar(BaseData* a, const AbstractTypeInfo& nfo, py::array_t src) diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp index d2a94f03..effa78f0 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp @@ -166,7 +166,9 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs /// Prepare the description to hold the different python attributes as data field's /// arguments then create the object. BaseObjectDescription desc {type.c_str(), type.c_str()}; - fillBaseObjectdescription(desc, kwargs); + py::list parametersToCopy; + py::list parametersToLink; + processKwargsForObjectCreation(kwargs, parametersToLink, parametersToCopy, desc); auto object = ObjectFactory::getInstance()->createObject(self, &desc); /// After calling createObject the returned value can be either a nullptr @@ -195,7 +197,13 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs { BaseData* d = object->findData(py::cast(a.first)); if(d) + { + if (parametersToLink.contains(a.first)) + d->setParent(a.second.cast()); + else if(parametersToCopy.contains(a.first)) + PythonFactory::fromPython(d, py::cast(a.second)); d->setPersistent(true); + } } return PythonFactory::toPython(object.get()); } @@ -258,7 +266,10 @@ py::object addChildKwargs(Node* self, const std::string& name, const py::kwargs& if (sofapython3::isProtectedKeyword(name)) throw py::value_error("addChild: Cannot call addChild with name " + name + ": Protected keyword"); BaseObjectDescription desc (name.c_str()); - fillBaseObjectdescription(desc,kwargs); + py::list parametersToCopy; + py::list parametersToLink; + processKwargsForObjectCreation(kwargs, parametersToLink, parametersToCopy, desc); + auto node=simpleapi::createChild(self, desc); checkParamUsage(desc); @@ -266,7 +277,13 @@ py::object addChildKwargs(Node* self, const std::string& name, const py::kwargs& { BaseData* d = node->findData(py::cast(a.first)); if(d) + { + if (parametersToLink.contains(a.first)) + d->setParent(a.second.cast()); + else if(parametersToCopy.contains(a.first)) + PythonFactory::fromPython(d, py::cast(a.second)); d->setPersistent(true); + } } return py::cast(node); From 61899a993f12c6acc721b0474631f9a77f9f9121 Mon Sep 17 00:00:00 2001 From: Damien Marchal Date: Mon, 31 Aug 2020 23:24:40 +0200 Subject: [PATCH 02/11] [bindings/Sofa] Return type_error when trying to create an object with invalid argument. (this was the previous behavior) --- bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp index effa78f0..dab438b9 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp @@ -157,17 +157,20 @@ py::object addObject(Node& self, BaseObject* object) /// Implement the addObject function. py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs& kwargs) { + /// Check that the name of the component is not one of the protected keywords (like: children, nodes,and others...) if (kwargs.contains("name")) { std::string name = py::str(kwargs["name"]); if (sofapython3::isProtectedKeyword(name)) throw py::value_error("addObject: Cannot call addObject with name " + name + ": Protected keyword"); } + /// Prepare the description to hold the different python attributes as data field's /// arguments then create the object. BaseObjectDescription desc {type.c_str(), type.c_str()}; py::list parametersToCopy; py::list parametersToLink; + processKwargsForObjectCreation(kwargs, parametersToLink, parametersToCopy, desc); auto object = ObjectFactory::getInstance()->createObject(self, &desc); @@ -203,18 +206,20 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs else if(parametersToCopy.contains(a.first)) PythonFactory::fromPython(d, py::cast(a.second)); d->setPersistent(true); + }else + { + throw py::type_error("Unknown Attribute: '"+py::cast(a.first)+"'"); } } return PythonFactory::toPython(object.get()); } -/// Implement the addObject function. +/// Implement the add(callable, xxx) function. py::object addKwargs(Node* self, const py::object& callable, const py::kwargs& kwargs) { if(py::isinstance(callable)) { BaseObject* obj = py::cast(callable); - self->addObject(obj); return py::cast(obj); } From b4b7caeea96abfb3def6edbfabc76fdb73b8da09 Mon Sep 17 00:00:00 2001 From: Damien Marchal Date: Mon, 31 Aug 2020 23:30:40 +0200 Subject: [PATCH 03/11] [bindings/Sofa/tests] Change few tests in ForceField and Node. The PR is breaking existing code. But the previously working code was, despite being conveniant was not valid and it was using a "flat" array as a multi-dimensionnal structure. It was not noticed before because the lists were converted into a 1D array then into a big string. Two options were possibles: a) not allowing this kind of code (using a 1D array to fill a 2D structure) b) implement kind of de-flattening a python lists into the corresponding multidimmensional matrix For the simplicity I choose option (a), so I updated the tests to use the 'right' syntax. --- bindings/Sofa/tests/Core/ForceField.py | 2 +- bindings/Sofa/tests/Simulation/Node.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/Sofa/tests/Core/ForceField.py b/bindings/Sofa/tests/Core/ForceField.py index 12cf3d85..ab1a26bb 100644 --- a/bindings/Sofa/tests/Core/ForceField.py +++ b/bindings/Sofa/tests/Core/ForceField.py @@ -30,7 +30,7 @@ def createParticle(node, node_name, use_implicit_scheme, use_iterative_solver): p = node.addChild(node_name) createIntegrationScheme(p, use_implicit_scheme) createSolver(p, use_iterative_solver) - dofs = p.addObject('MechanicalObject', name="MO", position=[0, 0, 0]) + dofs = p.addObject('MechanicalObject', name="MO", position=[[0, 0, 0]]) p.addObject('UniformMass', totalMass=1.0) print ("dofs.rest_position " + str(dofs.rest_position.value)) diff --git a/bindings/Sofa/tests/Simulation/Node.py b/bindings/Sofa/tests/Simulation/Node.py index 84745a0b..5c452985 100644 --- a/bindings/Sofa/tests/Simulation/Node.py +++ b/bindings/Sofa/tests/Simulation/Node.py @@ -45,7 +45,7 @@ def test_GetAttr(self): def test_init(self): root = Sofa.Core.Node("rootNode") c = root.addChild("child1") - c = c.addObject("MechanicalObject", name="MO", position=[0.0,1.0,2.0]*100) + c = c.addObject("MechanicalObject", name="MO", position=[[0.0,1.0,2.0]]*100) root.init() print("TYPE: "+str(len(c.position.value))) self.assertEqual(len(c.position.value), 100) From 0140ae3f12aa8b78a016dcd6378896794dde77ed Mon Sep 17 00:00:00 2001 From: Paul Baksic Date: Mon, 7 Jul 2025 15:48:30 +0200 Subject: [PATCH 04/11] revert changes to PrefabLink --- Plugin/src/SofaPython3/DataHelper.h | 82 +++++------------------------ 1 file changed, 14 insertions(+), 68 deletions(-) diff --git a/Plugin/src/SofaPython3/DataHelper.h b/Plugin/src/SofaPython3/DataHelper.h index 710a92ed..5444a4b2 100644 --- a/Plugin/src/SofaPython3/DataHelper.h +++ b/Plugin/src/SofaPython3/DataHelper.h @@ -34,71 +34,22 @@ along with sofaqtquick. If not, see . #include #include #include -#include -#include -#include #include "config.h" ////////////////////////// FORWARD DECLARATION /////////////////////////// namespace sofa { -namespace defaulttype { -class AbstractTypeInfo; -} -namespace core { -namespace objectmodel { -class BaseData; - - -class SOFAPYTHON3_API PrefabLink -{ -public: - PrefabLink() {} - PrefabLink(const Base::SPtr& targetBase) { m_targetBase = targetBase; } - PrefabLink(BaseLink* targetLink) { m_targetBase = targetLink->getLinkedBase(); } - PrefabLink(const std::string& targetPath) { m_targetPath = targetPath; } - - const Base::SPtr& getTargetBase() const { return m_targetBase; } - void setTargetBase(const Base::SPtr& targetBase) { m_targetBase = targetBase; } - - const std::string& getTargetPath() const { return m_targetPath; } - void setTargetPath(const std::string& targetPath) { m_targetPath = targetPath; } - - friend std::ostream& operator << ( std::ostream& out, const PrefabLink& l) - { - if (l.getTargetBase()) - { - auto bn = l.getTargetBase()->toBaseNode(); - auto bo = l.getTargetBase()->toBaseObject(); - out << "@" + (bn ? bn->getPathName() : bo->getPathName()); - } - out << l.getTargetPath(); - return out; + namespace defaulttype { + class AbstractTypeInfo; } - - friend std::istream& operator >> ( std::istream& in, PrefabLink& l) - { - std::string s; - in >> s; - l.setTargetPath(s); - return in; + namespace core { + namespace objectmodel { + class Base; + class BaseData; + class BaseNode; + class BaseObject; + } } - -private: - Base::SPtr m_targetBase { nullptr }; - std::string m_targetPath {""}; -}; -} -} -namespace defaulttype -{ -template <> -struct DataTypeName -{ - static const char* name() { return "PrefabLink"; } -}; - -} } /////////////////////////////// DECLARATION ////////////////////////////// @@ -136,7 +87,7 @@ template class py_shared_ptr : public sofa::core::sptr SOFAPYTHON3_API void setItem2D(py::array a, py::slice slice, py::object o); SOFAPYTHON3_API void setItem2D(py::array a, const py::slice& slice, - const py::slice& slice1, py::object o); + const py::slice& slice1, py::object o); SOFAPYTHON3_API void setItem1D(py::array a, py::slice slice, py::object o); SOFAPYTHON3_API void setItem(py::array a, py::slice slice, py::object value); @@ -160,16 +111,11 @@ void SOFAPYTHON3_API copyFromListScalar(BaseData& d, const AbstractTypeInfo& nfo std::string SOFAPYTHON3_API toSofaParsableString(const py::handle& p); +//py::object SOFAPYTHON3_API dataToPython(BaseData* d); + /// RVO optimized function. Don't care about copy on the return code. void SOFAPYTHON3_API fillBaseObjectdescription(sofa::core::objectmodel::BaseObjectDescription& desc, - const py::dict& dict); - -/// Split the content of the dictionnary 'dict' in three set. -/// On containing the data to parent, one containing the data to copy and on containing the data to parse in the BaseObjectDescription -void SOFAPYTHON3_API processKwargsForObjectCreation(const py::dict dict, - py::list& parametersToLink, - py::list& parametersToCopy, - sofa::core::objectmodel::BaseObjectDescription& parametersAsString); + const py::dict& dict); template void copyScalar(BaseData* a, const AbstractTypeInfo& nfo, py::array_t src) @@ -243,7 +189,7 @@ class scoped_writeonly_access ~scoped_writeonly_access(){ data->endEditVoidPtr(); } }; -SOFAPYTHON3_API BaseData* addData(py::object py_self, const std::string& name, py::object value = py::none(), py::object defaultValue = py::none(), const std::string& help = "", const std::string& group = "Property", std::string type = ""); +SOFAPYTHON3_API BaseData* addData(py::object py_self, const std::string& name, py::object value = py::object(), py::object defaultValue = py::object(), const std::string& help = "", const std::string& group = "Property", std::string type = ""); SOFAPYTHON3_API BaseLink* addLink(py::object py_self, const std::string& name, py::object value, const std::string& help); SOFAPYTHON3_API bool isProtectedKeyword(const std::string& name); From 2dd174367e909ba7ce8ed3b65eec51682d2ad319 Mon Sep 17 00:00:00 2001 From: Paul Baksic Date: Mon, 7 Jul 2025 16:41:20 +0200 Subject: [PATCH 05/11] Fix compilation --- Plugin/src/SofaPython3/DataHelper.cpp | 11 ++++------- Plugin/src/SofaPython3/DataHelper.h | 6 +++--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/Plugin/src/SofaPython3/DataHelper.cpp b/Plugin/src/SofaPython3/DataHelper.cpp index 2421b6d6..8ced6834 100644 --- a/Plugin/src/SofaPython3/DataHelper.cpp +++ b/Plugin/src/SofaPython3/DataHelper.cpp @@ -52,7 +52,7 @@ std::string toSofaParsableString(const py::handle& p) if(py::isinstance(p)) { return py::str(p); - + } // If the object is a data field we link the data field if(py::isinstance(p)) @@ -77,16 +77,13 @@ std::string toSofaParsableString(const py::handle& p) return py::repr(p); } -void fillBaseObjectdescription(sofa::core::objectmodel::BaseObjectDescription& desc, - const py::dict& dict) - { +void fillBaseObjectdescription(sofa::core::objectmodel::BaseObjectDescription& desc, const py::dict& dict) +{ for(auto kv : dict) { desc.setAttribute(py::str(kv.first), toSofaParsableString(kv.second)); } - - return; - } +} void processKwargsForObjectCreation(const py::dict dict, py::list& parametersToLink, diff --git a/Plugin/src/SofaPython3/DataHelper.h b/Plugin/src/SofaPython3/DataHelper.h index 353e0f75..585b7ac3 100644 --- a/Plugin/src/SofaPython3/DataHelper.h +++ b/Plugin/src/SofaPython3/DataHelper.h @@ -216,9 +216,9 @@ std::string SOFAPYTHON3_API toSofaParsableString(const pybind11::handle& p); /// Split the content of the dictionnary 'dict' in three set. /// On containing the data to parent, one containing the data to copy and on containing the data to parse in the BaseObjectDescription -void SOFAPYTHON3_API processKwargsForObjectCreation(const py::dict dict, - py::list& parametersToLink, - py::list& parametersToCopy, +void SOFAPYTHON3_API processKwargsForObjectCreation(const pybind11::dict dict, + pybind11::list& parametersToLink, + pybind11::list& parametersToCopy, sofa::core::objectmodel::BaseObjectDescription& parametersAsString); //pybind11::object SOFAPYTHON3_API dataToPython(BaseData* d); From cb5e0b0df31e1b48f1f2b963d388f82efe51aac0 Mon Sep 17 00:00:00 2001 From: Paul Baksic Date: Mon, 7 Jul 2025 17:22:23 +0200 Subject: [PATCH 06/11] Cannot deal with unknown attribute because some are dealt with by the factory itself (e.g. src, template, input...) --- bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp index a75f191c..802a48bb 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp @@ -291,17 +291,15 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs for(auto a : kwargs) { - BaseData* d = object->findData(py::cast(a.first)); - if(d) + const std::string dataName = py::cast(a.first); + BaseData * d = object->findData(dataName); + if (d) { if (parametersToLink.contains(a.first)) d->setParent(a.second.cast()); else if(parametersToCopy.contains(a.first)) PythonFactory::fromPython(d, py::cast(a.second)); d->setPersistent(true); - }else - { - throw py::type_error("Unknown Attribute: '"+py::cast(a.first)+"'"); } } return PythonFactory::toPython(object.get()); From e51194f995896a478c65cfbb488fb9dd1363bc76 Mon Sep 17 00:00:00 2001 From: Paul Baksic Date: Tue, 8 Jul 2025 11:06:21 +0200 Subject: [PATCH 07/11] Add compatibility layer --- .../SofaPython3/Sofa/Core/Binding_Node.cpp | 94 +++++++++++++++++-- 1 file changed, 86 insertions(+), 8 deletions(-) diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp index 802a48bb..f11749ca 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp @@ -236,7 +236,7 @@ void setFieldsFromPythonValues(Base* self, const py::kwargs& dict) } /// Implement the addObject function. -py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs& kwargs) +py::object addObjectKwargsToStrings(Node* self, const std::string& type, const py::kwargs& kwargs) { std::string name {}; if (kwargs.contains("name")) @@ -248,10 +248,8 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs /// Prepare the description to hold the different python attributes as data field's /// arguments then create the object. BaseObjectDescription desc {nullptr, type.c_str()}; - py::list parametersToCopy; - py::list parametersToLink; - processKwargsForObjectCreation(kwargs, parametersToLink, parametersToCopy, desc); + fillBaseObjectdescription(desc,kwargs); auto object = ObjectFactory::getInstance()->createObject(self, &desc); // After calling createObject the returned value can be either a nullptr @@ -295,13 +293,93 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs BaseData * d = object->findData(dataName); if (d) { - if (parametersToLink.contains(a.first)) - d->setParent(a.second.cast()); - else if(parametersToCopy.contains(a.first)) - PythonFactory::fromPython(d, py::cast(a.second)); d->setPersistent(true); } } + + return PythonFactory::toPython(object.get()); +} + +/// Implement the addObject function. +py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs& kwargs) +{ + std::string name {}; + if (kwargs.contains("name")) + { + name = py::str(kwargs["name"]); + if (sofapython3::isProtectedKeyword(name)) + throw py::value_error("Cannot call addObject with name " + name + ": Protected keyword"); + } + /// Prepare the description to hold the different python attributes as data field's + /// arguments then create the object. + BaseObjectDescription desc {nullptr, type.c_str()}; + py::list parametersToCopy; + py::list parametersToLink; + + processKwargsForObjectCreation(kwargs, parametersToLink, parametersToCopy, desc); + auto object = ObjectFactory::getInstance()->createObject(self, &desc); + + // After calling createObject the returned value can be either a nullptr + // or non-null but with error message or non-null. + // Let's first handle the case when the returned pointer is null. + if(!object) + { + std::stringstream tmp ; + for(auto& s : desc.getErrors()) + tmp << s << msgendl ; + throw py::value_error(tmp.str()); + } + + // Associates the emission location to the created object. + auto finfo = PythonEnvironment::getPythonCallingPointAsFileInfo(); + object->setInstanciationSourceFileName(finfo->filename); + object->setInstanciationSourceFilePos(finfo->line); + + if (name.empty()) + { + const auto resolvedName = self->getNameHelper().resolveName(object->getClassName(), name, sofa::core::ComponentNameHelper::Convention::python); + object->setName(resolvedName); + } + + setFieldsFromPythonValues(object.get(), kwargs); + + checkParamUsage(desc, object.get()); + + // Convert the logged messages in the object's internal logging into python exception. + // this is not a very fast way to do that...but well...python is slow anyway. And serious + // error management has a very high priority. If performance becomes an issue we will fix it + // when needed. + if(object->countLoggedMessages({Message::Error})) + { + throw py::value_error(object->getLoggedMessagesAsString({Message::Error})); + } + + + for(auto a : kwargs) + { + const std::string dataName = py::cast(a.first); + BaseData * d = object->findData(dataName); + + if (d) + { + if (parametersToLink.contains(a.first)) + d->setParent(a.second.cast()); + else if(parametersToCopy.contains(a.first)) + { + try + { + PythonFactory::fromPython(d, py::cast(a.second)); + } + catch (std::exception& e) + { + msg_warning(self)<<"Creating " << type << " named " << name <<". An exception of type \""<read(toSofaParsableString(a.second)); + } + } + d->setPersistent(true); + } + } return PythonFactory::toPython(object.get()); } From 8be00c5722f7c553413399e21ab9ab8bcb59675b Mon Sep 17 00:00:00 2001 From: Paul Baksic Date: Thu, 10 Jul 2025 09:13:20 +0200 Subject: [PATCH 08/11] Fix object instantiation when passed data is not a real one but is instead parsed by the object --- .../SofaPython3/Sofa/Core/Binding_Node.cpp | 73 +++++++++++++------ 1 file changed, 50 insertions(+), 23 deletions(-) diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp index f11749ca..5f763f69 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp @@ -316,7 +316,7 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs py::list parametersToCopy; py::list parametersToLink; - processKwargsForObjectCreation(kwargs, parametersToLink, parametersToCopy, desc); + processKwargsForObjectCreation( kwargs, parametersToLink, parametersToCopy, desc); auto object = ObjectFactory::getInstance()->createObject(self, &desc); // After calling createObject the returned value can be either a nullptr @@ -343,8 +343,6 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs setFieldsFromPythonValues(object.get(), kwargs); - checkParamUsage(desc, object.get()); - // Convert the logged messages in the object's internal logging into python exception. // this is not a very fast way to do that...but well...python is slow anyway. And serious // error management has a very high priority. If performance becomes an issue we will fix it @@ -359,27 +357,37 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs { const std::string dataName = py::cast(a.first); BaseData * d = object->findData(dataName); + BaseLink * l = object->findLink(dataName); - if (d) + if (d) + { + if (parametersToLink.contains(a.first)) + d->setParent(a.second.cast()); + else if(parametersToCopy.contains(a.first)) { - if (parametersToLink.contains(a.first)) - d->setParent(a.second.cast()); - else if(parametersToCopy.contains(a.first)) + try { - try - { - PythonFactory::fromPython(d, py::cast(a.second)); - } - catch (std::exception& e) - { - msg_warning(self)<<"Creating " << type << " named " << name <<". An exception of type \""<read(toSofaParsableString(a.second)); - } + PythonFactory::fromPython(d, py::cast(a.second)); + } + catch (std::exception& e) + { + msg_warning(self)<<"Creating " << type << " at " << object->getPathName() <<". An exception of type \""<read(toSofaParsableString(a.second))) + throw py::value_error("Cannot convert the input \""+ toSofaParsableString(a.second)+"\" to a valid value for data " + object->getPathName() + "." + dataName ); } - d->setPersistent(true); } + d->setPersistent(true); + } + else if (l == nullptr && parametersToCopy.contains(a.first)) + { + desc.setAttribute(dataName,toSofaParsableString(a.second) ); + } } + + object->parse(&desc); + checkParamUsage(desc, object.get()); + return PythonFactory::toPython(object.get()); } @@ -461,21 +469,40 @@ py::object addChildKwargs(Node* self, const std::string& name, const py::kwargs& node->setInstanciationSourceFileName(finfo->filename); node->setInstanciationSourceFilePos(finfo->line); - checkParamUsage(desc, node.get()); - for(auto a : kwargs) { - BaseData* d = node->findData(py::cast(a.first)); - if(d) + const std::string dataName = py::cast(a.first); + BaseData * d = node->findData(dataName); + BaseLink * l = node->findLink(dataName); + + if (d) { if (parametersToLink.contains(a.first)) d->setParent(a.second.cast()); else if(parametersToCopy.contains(a.first)) - PythonFactory::fromPython(d, py::cast(a.second)); + { + try + { + PythonFactory::fromPython(d, py::cast(a.second)); + } + catch (std::exception& e) + { + msg_warning(self)<<"Creating Node at " << node->getPathName() <<". An exception of type \""<read(toSofaParsableString(a.second))) + throw py::value_error("Cannot convert the input \""+ toSofaParsableString(a.second)+"\" to a valid value for data " + node->getPathName() + "." + dataName ); + } + } d->setPersistent(true); } + else if (l == nullptr && parametersToCopy.contains(a.first)) + { + desc.setAttribute(dataName,toSofaParsableString(a.second) ); + } } + node->parse(&desc); + checkParamUsage(desc, node.get()); return py::cast(node); } From 577296c57da58cee6e6c566dfd1e98aa4645facd Mon Sep 17 00:00:00 2001 From: Paul Baksic Date: Thu, 10 Jul 2025 12:04:25 +0200 Subject: [PATCH 09/11] PAss linkpath as string to descriptor to fix input/ouptu mechanism of mapping --- Plugin/src/SofaPython3/DataHelper.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Plugin/src/SofaPython3/DataHelper.cpp b/Plugin/src/SofaPython3/DataHelper.cpp index 8ced6834..1a79cb6b 100644 --- a/Plugin/src/SofaPython3/DataHelper.cpp +++ b/Plugin/src/SofaPython3/DataHelper.cpp @@ -90,13 +90,14 @@ void processKwargsForObjectCreation(const py::dict dict, py::list& parametersToCopy, sofa::core::objectmodel::BaseObjectDescription& parametersAsString) { - auto t = py::detail::get_type_handle(typeid(BaseData), false); + auto typeHandleBaseData = py::detail::get_type_handle(typeid(BaseData), false); + auto typeHandleLinkPath = py::detail::get_type_handle(typeid(LinkPath), false); for(auto kv : dict) { - if (py::isinstance(kv.second, t)) + if (py::isinstance(kv.second, typeHandleBaseData)) parametersToLink.append(kv.first); - else if (py::isinstance(kv.second)) - parametersAsString.setAttribute(py::str(kv.first), py::cast(kv.second)); + else if (py::isinstance(kv.second) || py::isinstance(kv.second, typeHandleLinkPath) ) + parametersAsString.setAttribute(py::str(kv.first), py::str(kv.second)); else parametersToCopy.append(kv.first); } From f266d5f278e6f976d1adad1fa9f8ffe1ff6add92 Mon Sep 17 00:00:00 2001 From: bakpaul Date: Thu, 10 Jul 2025 15:50:11 +0200 Subject: [PATCH 10/11] Fix multimapping passing a list of strings --- Plugin/src/SofaPython3/DataHelper.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Plugin/src/SofaPython3/DataHelper.cpp b/Plugin/src/SofaPython3/DataHelper.cpp index 1a79cb6b..b8fd2177 100644 --- a/Plugin/src/SofaPython3/DataHelper.cpp +++ b/Plugin/src/SofaPython3/DataHelper.cpp @@ -98,6 +98,25 @@ void processKwargsForObjectCreation(const py::dict dict, parametersToLink.append(kv.first); else if (py::isinstance(kv.second) || py::isinstance(kv.second, typeHandleLinkPath) ) parametersAsString.setAttribute(py::str(kv.first), py::str(kv.second)); + //This test is only required because of the multimappings that need the data "input" during the call to canCreate but it is given as a list of strings. + //So when a list of string is passed, then we use directly the conversion to string to be able to pass it directly in the BaseObjectDescription. + //TODO: remove this once the canCreate of Mapping class doesn't need to access data input and output + else if (py::isinstance(kv.second)) + { + bool isAllStrings = true; + for(auto data : kv.second) + { + if(!py::isinstance(data)) + { + isAllStrings = false; + break; + } + } + if(isAllStrings) + parametersAsString.setAttribute(py::str(kv.first), toSofaParsableString(kv.second)); + else + parametersToCopy.append(kv.first); + } else parametersToCopy.append(kv.first); } From c898a45dbd3a16db118ba3f94efa023cfc5478b4 Mon Sep 17 00:00:00 2001 From: bakpaul Date: Thu, 10 Jul 2025 16:23:35 +0200 Subject: [PATCH 11/11] Add comments --- bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp index 5f763f69..fdfd7c99 100644 --- a/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp +++ b/bindings/Sofa/src/SofaPython3/Sofa/Core/Binding_Node.cpp @@ -316,6 +316,9 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs py::list parametersToCopy; py::list parametersToLink; + //This method will sort the input kwargs. + //It will keep all strings, and list of strings as string and put them to desc so the factory can use them + //during canCreate and parse calls (important for template, src, input/output of mapping) processKwargsForObjectCreation( kwargs, parametersToLink, parametersToCopy, desc); auto object = ObjectFactory::getInstance()->createObject(self, &desc); @@ -352,7 +355,7 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs throw py::value_error(object->getLoggedMessagesAsString({Message::Error})); } - + //Now for all the data that have not been passed by object descriptor, we pass them to the object for(auto a : kwargs) { const std::string dataName = py::cast(a.first); @@ -381,11 +384,15 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs } else if (l == nullptr && parametersToCopy.contains(a.first)) { + // This case happens when the object overrides the method parse which + // expect some arguments in desc instead of using datas to expose variation points desc.setAttribute(dataName,toSofaParsableString(a.second) ); } } + // Let the object parse the desc one last time now that 'real' all data has been set object->parse(&desc); + // Now we check that everything has been used. If not, then throw an error. checkParamUsage(desc, object.get()); return PythonFactory::toPython(object.get());