Use real copy instead of converting to string for when calling addObject or addChild

Plugin/src/SofaPython3/DataHelper.cpp

@@ -41,15 +41,18 @@ std::string toSofaParsableString(const py::handle& p)
     if(py::isinstance<py::list>(p) || py::isinstance<py::tuple>(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<py::str>(p))
+    {
         return py::str(p);
+    }
 
     // If the object is a data field we link the data field
     if(py::isinstance<sofa::core::objectmodel::BaseData>(p))
@@ -74,14 +77,50 @@ std::string toSofaParsableString(const py::handle& 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)
+void fillBaseObjectdescription(sofa::core::objectmodel::BaseObjectDescription& desc, const py::dict& dict)
+{
+     for(auto kv : dict)
+     {
+        desc.setAttribute(py::str(kv.first), toSofaParsableString(kv.second));
+     }
+}
+
+void processKwargsForObjectCreation(const py::dict dict,
+                               py::list& parametersToLink,
+                               py::list& parametersToCopy,
+                               sofa::core::objectmodel::BaseObjectDescription& parametersAsString)
 {
+    auto typeHandleBaseData = py::detail::get_type_handle(typeid(BaseData), false);
+    auto typeHandleLinkPath = py::detail::get_type_handle(typeid(LinkPath), false);
     for(auto kv : dict)
     {
-        desc.setAttribute(py::str(kv.first), toSofaParsableString(kv.second));
+        if (py::isinstance(kv.second, typeHandleBaseData))
+            parametersToLink.append(kv.first);
+        else if (py::isinstance<py::str>(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<py::list>(kv.second))
+        {
+            bool isAllStrings = true;
+            for(auto data : kv.second)
+            {
+                if(!py::isinstance<py::str>(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);
     }
+    return;
 }
 
 std::ostream& operator<<(std::ostream& out, const py::buffer_info& p)

Plugin/src/SofaPython3/DataHelper.h

@@ -27,6 +27,7 @@
 #include <sofa/core/sptr.h>
 #include <sofa/helper/Factory.h>
 #include <sofa/core/objectmodel/Data.h>
+
 #include <sofa/core/objectmodel/Base.h>
 #include <sofa/core/objectmodel/BaseObject.h>
 #include <sofa/core/objectmodel/BaseNode.h>
@@ -39,9 +40,11 @@ namespace sofa {
     }
     namespace core {
         namespace objectmodel {
+            class Base;
             class BaseData;
 
 
+
             class SOFAPYTHON3_API PrefabLink
             {
             public:
@@ -210,6 +213,14 @@ void SOFAPYTHON3_API copyFromListScalar(BaseData& d, const AbstractTypeInfo& nfo
 
 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 pybind11::dict dict,
+                                                    pybind11::list& parametersToLink,
+                                                    pybind11::list& parametersToCopy,
+                                                    sofa::core::objectmodel::BaseObjectDescription& parametersAsString);
+
 //pybind11::object SOFAPYTHON3_API dataToPython(BaseData* d);
 
 /// RVO optimized function. Don't care about copy on the return code.

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"))
@@ -245,10 +245,11 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs
         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.
+    /// Prepare the description to hold the different python attributes as data field's
+    /// arguments then create the object.
     BaseObjectDescription desc {nullptr, type.c_str()};
-    fillBaseObjectdescription(desc, kwargs);
+
+    fillBaseObjectdescription(desc,kwargs);
     auto object = ObjectFactory::getInstance()->createObject(self, &desc);
 
     // After calling createObject the returned value can be either a nullptr
@@ -288,20 +289,122 @@ py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs
 
     for(auto a : kwargs)
     {
-        BaseData* d = object->findData(py::cast<std::string>(a.first));
-        if(d)
+        const std::string dataName = py::cast<std::string>(a.first);
+        BaseData * d = object->findData(dataName);
+        if (d)
+        {
             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;
+
+    //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);
+
+    // 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);
+
+    // 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}));
+    }
+
+    //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<std::string>(a.first);
+        BaseData * d = object->findData(dataName);
+        BaseLink * l = object->findLink(dataName);
+
+        if (d)
+        {
+            if (parametersToLink.contains(a.first))
+                d->setParent(a.second.cast<BaseData*>());
+            else if(parametersToCopy.contains(a.first))
+            {
+                try
+                {
+                    PythonFactory::fromPython(d, py::cast<py::object>(a.second));
+                }
+                catch (std::exception& e)
+                {
+                    msg_warning(self)<<"Creating " << type << " at " << object->getPathName() <<". An exception of type \""<<e.what()<<"\" was received while copying value input for data " << dataName << ". To fix this please reshape the list you are providing to fit the real intended data structure. \n    The compatibility layer will try to create the data by converting the input to string.";
+
+                    if ( !d->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);
+        }
+        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());
+}
+
+/// Implement the add(callable, xxx) function.
 py::object addKwargs(Node* self, const py::object& callable, const py::kwargs& kwargs)
 {
     if(py::isinstance<BaseObject*>(callable))
     {
         BaseObject* obj = py::cast<BaseObject*>(callable);
-        self->addObject(obj);
+        self->addObject(obj);    
         return py::cast(obj);
     }
 
@@ -364,21 +467,49 @@ 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);
     auto finfo = PythonEnvironment::getPythonCallingPointAsFileInfo();
     node->setInstanciationSourceFileName(finfo->filename);
     node->setInstanciationSourceFilePos(finfo->line);
 
-    checkParamUsage(desc, node.get());
-
     for(auto a : kwargs)
     {
-        BaseData* d = node->findData(py::cast<std::string>(a.first));
-        if(d)
+        const std::string dataName = py::cast<std::string>(a.first);
+        BaseData * d = node->findData(dataName);
+        BaseLink * l = node->findLink(dataName);
+
+        if (d)
+        {
+            if (parametersToLink.contains(a.first))
+                d->setParent(a.second.cast<BaseData*>());
+            else if(parametersToCopy.contains(a.first))
+            {
+                try
+                {
+                    PythonFactory::fromPython(d, py::cast<py::object>(a.second));
+                }
+                catch (std::exception& e)
+                {
+                    msg_warning(self)<<"Creating Node at " << node->getPathName() <<". An exception of type \""<<e.what()<<"\" was received while copying value input for data " << dataName << ". To fix this please reshape the list you are providing to fit the real intended data structure. \n    The compatibility layer will try to create the data by converting the input to string.";
+
+                    if ( !d->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);
 }
 

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)
 
     myRSSFF = NaiveRestShapeSpringsForcefield(name="Springs",

bindings/Sofa/tests/Simulation/Node.py

@@ -46,7 +46,7 @@ def test_init(self):
                 root = Sofa.Core.Node("rootNode")
                 root.addObject("RequiredPlugin", name="Sofa.Component.StateContainer")
                 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)