BUG: Ensure DICOMUtils.loadLoadables do not return temporary nodes

[jcfr]

Since loader may add temporary nodes to the scene while loading a file, this function makes sure these are not included in the list of returned node IDs.

Example of such loaded is UltrasoundImage3dReaderFileReader adding a temporary node to construct the volume sequence.

See https://github.com/SlicerHeart/SlicerHeart/blob/52eeb23576c7eda14dbe3e602667f4c5b68c4475/UltrasoundImage3dReader/UltrasoundImage3dReader.py#L172-L173

[lassoan]

It looks OK to me, but I think this is an error case, so we should log a warning to make developers aware that something is not correct.

[jcfr]

re: so we should log a warning to make developers aware that something is not correct.

Nodes may have been removed in two cases:

1. An exception was raised during import and partially initialized nodes are removed
2. A temporary was legitimately created as part of the import. For, see UltrasoundImage3dReader.py¹

To "fix" UltrasoundImage3dReader.py, should we create a miniScene and only copy node into the main scene at the end of the reading process ? Similar to what is done in ReadDataRequestScene::Execute()² ?

We could then recommend this pattern for reader plugin creating multiple nodes.

Footnotes

1. https://github.com/SlicerHeart/SlicerHeart/blob/52eeb23576c7eda14dbe3e602667f4c5b68c4475/UltrasoundImage3dReader/UltrasoundImage3dReader.py#L172-L173 ↩

2. https://github.com/Slicer/Slicer/blob/cecf2c5f45af0a1361c1b1b9555eef82b9355eb7/Base/Logic/vtkSlicerApplicationLogicRequests.h#L293 ↩

[jcfr]

Following July 25th developer meeting, observing the scene node removed event should allow to more cleanly handle this.

[jcfr]

Thanks @sjh26 and @lassoan for the feedback, I will add a test for this and then this will be ready for integration.