-
Notifications
You must be signed in to change notification settings - Fork 50
Add new generic add method #530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -235,6 +235,115 @@ void setFieldsFromPythonValues(Base* self, const py::kwargs& dict) | |
} | ||
} | ||
|
||
py::object add( 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"); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we consider to add a name generation mechanism (with +1) to avoid having name duplicated. |
||
/// 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); | ||
|
||
sofa::core::objectmodel::Base::SPtr createdObj; | ||
|
||
if (type == "Node") | ||
{ | ||
createdObj = simpleapi::createChild(self, desc); | ||
} | ||
else | ||
{ | ||
createdObj = 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(!createdObj) | ||
{ | ||
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(); | ||
createdObj->setInstanciationSourceFileName(finfo->filename); | ||
createdObj->setInstanciationSourceFilePos(finfo->line); | ||
|
||
if (name.empty()) | ||
{ | ||
const auto resolvedName = self->getNameHelper().resolveName(createdObj->getClassName(), name, sofa::core::ComponentNameHelper::Convention::python); | ||
createdObj->setName(resolvedName); | ||
} | ||
|
||
setFieldsFromPythonValues(createdObj.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(createdObj->countLoggedMessages({Message::Error})) | ||
{ | ||
throw py::value_error(createdObj->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 = createdObj->findData(dataName); | ||
BaseLink * l = createdObj->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 " << createdObj->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 " + createdObj->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 | ||
createdObj->parse(&desc); | ||
// Now we check that everything has been used. If not, then throw an error. | ||
checkParamUsage(desc, createdObj.get()); | ||
|
||
|
||
return PythonFactory::toPython(createdObj.get()); | ||
} | ||
|
||
|
||
class NumpyReprFixerRAII | ||
{ | ||
public: | ||
|
@@ -270,6 +379,7 @@ class NumpyReprFixerRAII | |
}; | ||
|
||
|
||
|
||
/// Implement the addObject function. | ||
py::object addObjectKwargs(Node* self, const std::string& type, const py::kwargs& kwargs) | ||
{ | ||
|
@@ -355,7 +465,7 @@ py::object addKwargs(Node* self, const py::object& callable, const py::kwargs& k | |
if(py::isinstance<py::str>(callable)) | ||
{ | ||
py::str type = callable; | ||
return addObjectKwargs(self, type, kwargs); | ||
return add(self, type, kwargs); | ||
} | ||
|
||
if (kwargs.contains("name")) | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -63,7 +63,39 @@ def d1(): | |||
|
||||
self.assertRaises(ValueError, d1) | ||||
root.init() | ||||
|
||||
|
||||
def test_generic_add_type_child(self): | ||||
root = Sofa.Core.Node("rootNode") | ||||
|
||||
FirstNode = root.add("Node",name="FirstNode") | ||||
|
||||
FirstNode.addObject("RequiredPlugin", name="Sofa.Component.StateContainer") | ||||
|
||||
self.assertEqual(root.children()[0].name.value, "FirstNode") | ||||
self.assertEqual(root.FirstNode.objects()[0].type.value, "RequiredPlugin") | ||||
self.assertEqual(root.FirstNode.objects()[0].name.value, "FirstNode") | ||||
|
||||
pass | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
|
||||
def test_generic_add_type_object(self): | ||||
root = Sofa.Core.Node("rootNode") | ||||
|
||||
root.add("RequiredPlugin", name="Sofa.Component.StateContainer") | ||||
self.assertEqual(root.objects()[0].type.value, "RequiredPlugin") | ||||
self.assertEqual(root.objects()[0].name.value, "FirstNode") | ||||
|
||||
pass | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
|
||||
def test_generic_add_real_object(self): | ||||
root = Sofa.Core.Node("rootNode") | ||||
|
||||
MyNode = Sofa.Core.Node("MyNode") | ||||
|
||||
root.add(MyNode) | ||||
self.assertEqual(root.children()[0].name.value, "MyNode") | ||||
|
||||
pass | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
|
||||
def test_addChild(self): | ||||
root = Sofa.Core.Node("rootNode") | ||||
root.addChild(Sofa.Core.Node("child1")) | ||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to add a second signature so we can pass real type instead of string.
But this could be in a second PR.