Thread for continuing discussion in PR1029:
@jcfr:
Should we simply disable the creation of node using node constructor from both C++ and python ? That would be consistent with the new behavior we are promoting.
@lassoan:
There are probably valid use cases for instantiating nodes directly, so probably we should not disable it.
@pieper:
Yes, instantiating a node and adding it to the scene seems like the logical pattern (it’s in lots of examples). But when you are using the utility method to create default display nodes then you should expect them to have the proper SetAndObserve* behavior and that depends on the scene being valid.
On the other hand, now that I look at it it’s odd that CreateDefaultStorageNode doesn’t call SetAndObserveStorageNodeID.
We could deprecate (or leave) the current methods and introduce CreateAndObserveDefault{Display,Storage}Node methods.
@cpinter:
It is indeed strange that there is no observation set in CreateDefaultStorageNode. Considering that CreateDefaultDisplayNodes does set the observation, I think this is an inconsistency that we should fix, so at least the two functions with the similar names do the same things. I know there might be issues in some places, but I’d rather add the observation in the CreateDefaultStorageNode functions to make them work the same way.
@jcfr:
This morning we also talked about adding a vtkMRMLScene::Reset(vtkMRMLNode*) that would allow to reset a node with the expected default properties.
Either way, I am wondering if we should we also replace the calls to vtkNew<vtkMRMLNAME> with the equivalent call using AddNewNodeByClass or by adding a call to the (to be added) vtkMRMLScene::Reset(vtkMRMLNode*) function ?