SofaPython3 icon indicating copy to clipboard operation
SofaPython3 copied to clipboard

What is the expected behavior when passing a Data as argument in the object behavior ?

Open damienmarchal opened this issue 3 years ago • 4 comments

What should be the behavior when passing other data field as argument during object creation.

Example:

def createScene(root):
    root.addObject("MeshObjLoader", filename="mesh/cube.obj", name="loader");
    a = root.addObject("PointSetTopologyContainer", position=root.loader.position, name="topo") 
    b = root.addObject("PointSetTopologyContainer", position=root.loader.position.value, name="topo") 

To me a should make a link, while b should copy the value. If you share this few then ... the current implementation need to be fixed

damienmarchal avatar Sep 22 '21 14:09 damienmarchal

I also share your view @damienmarchal. It feels natural like this. When you pass an object or data as a parameter, you pass a "reference" to it. When you pass a value, you make a copy. +1 for me

jnbrunet avatar Sep 23 '21 01:09 jnbrunet

Thanks @jnbrunet for the feedback.

Yesterday I made several tests to see the look and feel and made

def createScene(root):
    root.addObject("MeshObjLoader", filename="mesh/cube.obj", name="loader");
    a = root.addObject("PointSetTopologyContainer", position=root.loader.position.linkpath, name="topo") 
    b = root.addObject("PointSetTopologyContainer", position=root.loader.position.value, name="topo") 

So there is an explicit version for the two cases. And for the implicit one:

    a = root.addObject("PointSetTopologyContainer", position=root.loader.position, name="topo") 

We need to decide if we continue to copy values (what we are doing right now) or if we make a link (more breaking ?). With link-by-default, the performances will be very good but users has to understand what a link and fallback to value copy-value if this cause issues in their scenes . with copy-by-default approach the performances will be bad and user will have to op-in for the fastest (link-base) version.

damienmarchal avatar Sep 23 '21 07:09 damienmarchal

Another alternative could be to first deprecate (with a warning message) the existing implicit version then replace it with an exception saying that this ambiugous and force to replace it with one of the explicit ones.

a = root.addObject("PointSetTopologyContainer", position=root.loader.position.linkpath, name="topo") 
b = root.addObject("PointSetTopologyContainer", position=root.loader.position.value, name="topo") 

damienmarchal avatar Sep 23 '21 12:09 damienmarchal

Hey @damienmarchal

I'm curious in which cases the user would prefer a copy against a (COW anyway?) link. I have a feeling that a link would be more appropriate for most needs, hence creating less frustrations for the user, but I may be wrong.

From experience, people will naturally set the attribute to the source data directly, i.e.:

node.addObject("MechanicalObject", position=node.loader.position)

And forget about it. It is also much simpler to read. But it does come with an implicit decision made for the user.

A warning here would, I think, generate a "What the f. is that? Which one should I use? I just want this component positions..." In the user's head.

I am usually in favor of explicit against implicit, but here I think it is a good case of simplicity outweighs the cost of implicity (the cost which I think is minimal anyway...) .

My 2 cents! Whatever you choose, it will be a great improvement.

jnbrunet avatar Sep 24 '21 03:09 jnbrunet