conduit icon indicating copy to clipboard operation
conduit copied to clipboard

consider stricter semantics for `Node::set`

Open cyrush opened this issue 3 years ago • 2 comments

before Node::update and Node::update_compatible existed, Node::set semantics were created to support the case where updating a compatible array would copy without modifying the number of elements as long as the input data type is compatible.

Here are the relevant details, ::init is called on every Node::set:

https://github.com/LLNL/conduit/blob/68b850ea502fd358e21e123c19284f6dfd06a075/src/libs/conduit/conduit_node.cpp#L16737

and here is DataType::compatible logic:

https://github.com/LLNL/conduit/blob/68b850ea502fd358e21e123c19284f6dfd06a075/src/libs/conduit/conduit_data_type.cpp#L653

The case that causes confusion is when the number of elements in the new array is less than the existing Node description. Folks might assume the Node will take on the exact description of what is set, but that description does not change.

We should consider changing this to better support the separate the use cases:

A few options:

  • Remove support for sub array copy in Node::set and Node::update, and support the case only via existing: Node::update_compatible()

  • Remove support for sub array copy in Node::set and Node::update, add Node::set_compatible() to explicitly support this case and also use Node::set_compatible() in Node::update_compatible().

Second one is more work, but I think it might be the best solution.

cyrush avatar Mar 22 '22 20:03 cyrush

q: how does this interact with prior set_external cases?

cyrush avatar Mar 22 '22 21:03 cyrush