Fixed in r21136 & r21137

vtkSlicerVolumeRenderingLogic::CreateVolumeRenderingDisplayNode() doesn't add the new node into the scene (and node must be de-referenced by the caller)
vtkSlicerVolumeLogic::CreateLabelVolume() has been renamed into vtkSlicerVolumeLogic::CreateAndAddLabelVolume()

We should not be making non-backward-compatible API changes during a feature freeze before a minor release -- it was good that you refactored all the code in slicer's repository, but what about extensions or people developing their own code on top of slicer?

There was a similar issue recently with GetPolyData in the vtkMRMLModelDisplayNode [1] - I heard from one of the developers of the iGyne module that this took them a while to figure out.

To avoid the issue, we should keep the old API in place as a deprecated method and even add a vtkWarningMessage so that developers will know to migrate to the new API but their code will not be broken.

[1] http://viewvc.slicer.org/viewvc.cgi/Slicer4/trunk/Libs/MRML/Core/vtkMRMLModelDisplayNode.h?r1=20909&r2=20910

Commited in r21138 and https://github.com/fedorov/ChangeTrackerPy/commit/f8845164051aade523b4686f7136ad9a005e943b

Backward compatibility functions added in r21139

Steve, very good point, I was going back and forth with the backward compatibility. Doesn't python throw an error when it can't find a method already ? It would also be great if we can have a mechanism to inform developer of obsolete methods.
Doesn't python throw an error when it can't find a method already ?
Maybe we should just advertise "http://slicer.org/doc/html/deprecated.html" a bit more.

Yes, python raises and exception when the method doesn't exist, but that doesn't point you to the new API method (just like a linker error doesn't).

If we really want to remove the method, we could do a stub that only prints the new suggested API at compile and/or run time. But I think that for stylistic renaming purposes it's better to provide the compatible API.

vtkMRMLScene::GetNodesByClass() and vtkMRMLScene::GetNodesByClassByName() are also misleading and are prone to memory leak (see 0002621 or r21124).
How about we rename them (and keep backward compatibility methods) into FindNodesByClass() and FindNodesByClassByName() ?

I agree that those methods are error prone and I've never cared for them. I wonder though if Find is really better than Get in this case? In some sense these are 'Get'ters in that they return pointers to nodes that are owned by the scene. It's the vtkCollection that is problematic in my view, so maybe we move to std containers of pointers?

Probably we should think more generally about the MRML API and representation. There are some use cases, like getting/observing nodes by class type(s) and scene views, that could probably benefit from new methods. The slicer.util.getNodes method in python probably should have a C++ implementation and a more carefully thought out set of functionality.

Yes, we should think more generally about the MRML API. I wouldn't mind using more std methods.

Nonetheless, in that particuliar case, I don't think that GetNodesByClass() is a simple accessor, it really browses the entire scene, and "filters" the nodes to return only the ones that match a criteria.
So there is an "action" happening, more than a simple access.
Moreover, this function can be costly as it browses the entire scene. In a lot of places, all the returned nodes are not even used. In the example of Andrei, he is interested in 1 item (the first). GetNthNode would have been faster. In a lot of cases, it is more efficient to use a simple iterator on the entire scene collection (e.g. scene->GetNodes()->GetNextItemAsObject(it))
That's why I thought FindXXX() was more appropriate (and somewhat respects the VTK naming convention). To me, FindXXX() still carry the idea that the found objects belong to the class.

Another solution is to not return anything, but take the vtkCollection as paramater.
But I still don't think such "search" method should be named "GetXXX"