# Subject hierarchy listening to events

**URL:** https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364
**Category:** Development
**Created:** [July 3, 2018, 5:46pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364 "2018-07-03T17:46:44Z")
**Posts on this page:** 11
**Page:** 1

<div class="post-metadata">

### Author: ![Johan\_Andruejol](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/johan_andruejol/32/3639_2.png) [@Johan\_Andruejol](https://discourse.slicer.org/u/Johan_Andruejol)
#### Post date: [July 3, 2018, 5:46pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/1 "2018-07-03T17:46:44Z")

</div>

Hi all,

I am working a module that relies heavily on the subject hierarchy. I need to listen to its modified/added and removed item events. Here is an abstract of what I do:

```
class MyClass(VTKObservationMixin):
  def __init__ (self):
    VTKObservationMixin. __init__ (self)
    shNode = slicer.vtkMRMLSubjectHierarchyNode.GetSubjectHierarchyNode(slicer.mrmlScene)
    self.addObserver(shNode, shNode.SubjectHierarchyItemModifiedEvent, self.shItemModifiedEvent)
  
  @vtk.calldata_type(vtk.VTK_LONG)
  def shItemModifiedEvent(self, caller, eventId, callData=None):
    print("SH Node mod")
    print("SH item ID: {0}".format(callData))

obj = MyClass()

shNode = slicer.vtkMRMLSubjectHierarchyNode.GetSubjectHierarchyNode(slicer.mrmlScene)

itemID = shNode.CreateFolderItem(shNode.GetSceneItemID(), 'Test')

wasModifying = shNode.StartModify()
shNode.SetItemName(itemID, 'NewName')
shNode.SetItemAttribute(itemID, 'NewAttribute', 'a')
shNode.EndModify(wasModifying)

```

This crashes on Slicer that I built manually.  
The issues comes from the`EndModify()` that will [re-invoke all the custom events](https://github.com/Slicer/Slicer/blob/master/Libs/MRML/Core/vtkMRMLNode.h#L478). Those events however are invoked without a calldata and that makes the application crash when it tries to convert the calldata to long.

I am not sure what can be done for this. The naive solution I see would be to never use the calldata to answer the callback of the subject hierarchy. But then we would need a method on the subject hierarchy node to know what was the item ID that was last modified ?  
Or should the [VTK python command handle](https://github.com/Kitware/VTK/blob/master/Wrapping/PythonCore/vtkPythonCommand.cxx#L169) the case where the calldata is NULL better ?

Thanks !  
Johan

---

<div class="post-metadata">

### Author: ![Johan\_Andruejol](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/johan_andruejol/32/3639_2.png) [@Johan\_Andruejol](https://discourse.slicer.org/u/Johan_Andruejol)
#### Post date: [July 18, 2018, 1:51pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/2 "2018-07-18T13:51:54Z")

</div>

@lassoan, @cpinter:  
I am thinking to implement a method that stores the ID of the last modified item so it’s always accessible through python. Since you implemented this node, I was wondering what you were thinking ?

---

<div class="post-metadata">

### Author: ![jcfr](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/jcfr/32/17825_2.png) [@jcfr](https://discourse.slicer.org/u/jcfr)
#### Post date: [July 18, 2018, 2:12pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/3 "2018-07-18T14:12:56Z")

</div>

After talking with @lassoan, the current mechanism is generic and can handle any event. As you first suggested, we should instead fix the VTK wrapping layer to that the call data is None if the pointer is null.

From the application perspective, this would mean that we can’t know the specific ID or name or … that was modified and the UI should be refreshed.

---

<div class="post-metadata">

### Author: ![jcfr](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/jcfr/32/17825_2.png) [@jcfr](https://discourse.slicer.org/u/jcfr)
#### Post date: [July 18, 2018, 2:21pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/4 "2018-07-18T14:21:14Z")

</div>

For all types, the code could then be changed from:

```auto
PyObject* callDataAsInt = PyLong_FromLong(*reinterpret_cast<long*>(callData));
arglist = BuildCallDataArgList(obj2, eventname, callDataAsInt);

```

to

```auto
PyObject* callDataAsInt = callData != nullptr ? PyLong_FromLong(*reinterpret_cast<long*>(callData)) : nullptr;
arglist = BuildCallDataArgList(obj2, eventname, callDataAsInt);

```

(or the more explicit multi-line version …)

---

<div class="post-metadata">

### Author: ![Johan\_Andruejol](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/johan_andruejol/32/3639_2.png) [@Johan\_Andruejol](https://discourse.slicer.org/u/Johan_Andruejol)
#### Post date: [July 18, 2018, 2:37pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/5 "2018-07-18T14:37:52Z")

</div>

That makes sense to me. I will create a PR.  
Thanks !

---

<div class="post-metadata">

### Author: ![cpinter](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/cpinter/32/7995_2.png) [@cpinter](https://discourse.slicer.org/u/cpinter)
#### Post date: [July 18, 2018, 2:54pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/6 "2018-07-18T14:54:56Z")

</div>

Thanks @jcfr and @lassoan for helping Johan! I just managed to dig through the administrative stuff and get back to real work after I got back from my long trip.

---

<div class="post-metadata">

### Author: ![Johan\_Andruejol](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/johan_andruejol/32/3639_2.png) [@Johan\_Andruejol](https://discourse.slicer.org/u/Johan_Andruejol)
#### Post date: [July 18, 2018, 5:03pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/7 "2018-07-18T17:03:19Z")

</div>

I created a PR on VTK ([https://gitlab.kitware.com/vtk/vtk/merge\_requests/4513](https://gitlab.kitware.com/vtk/vtk/merge_requests/4513)) and Slicer: [https://github.com/Slicer/VTK/pull/17](https://github.com/Slicer/VTK/pull/17).

---

<div class="post-metadata">

### Author: ![Johan\_Andruejol](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/johan_andruejol/32/3639_2.png) [@Johan\_Andruejol](https://discourse.slicer.org/u/Johan_Andruejol)
#### Post date: [July 26, 2018, 6:20pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/8 "2018-07-26T18:20:27Z")

</div>

So the NULL handling bug is actually already fixed on VTK9.

I have noticed however that running the following code does generate a lot of errors:

```
shNode = slicer.vtkMRMLSubjectHierarchyNode.GetSubjectHierarchyNode(slicer.mrmlScene)
itemID = shNode.CreateFolderItem(shNode.GetSceneItemID(), 'Test')

with slicer.util.NodeModify(shNode):
  shNode.SetItemName(itemID, 'NewName')
  shNode.SetItemAttribute(itemID, 'NewAttribute', 'a')

```

Errors:

```
double __cdecl qSlicerSubjectHierarchyFolderPlugin::canOwnSubjectHierarchyItem(__int64) const : Input item is invalid! 
double __cdecl qSlicerSubjectHierarchyChartsPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchyPlotChartPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
GetItemDataNode: Failed to find subject hierarchy item by ID 0

double __cdecl qSlicerSubjectHierarchyVolumesPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchyLabelMapsPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchyDiffusionTensorVolumesPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchyDICOMPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchyMarkupsPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchyModelsPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchySceneViewsPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchySegmentationsPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchySegmentsPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchyTablesPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
double __cdecl qSlicerSubjectHierarchyTransformsPlugin::canOwnSubjectHierarchyItem(__int64) const : Invalid input item 
SetItemOwnerPluginName: Failed to find subject hierarchy item by ID 0

```

@cpinter, @jcfr: I think the SH node should fail silently when you request an invalid item ID. What do you think ?

---

<div class="post-metadata">

### Author: ![cpinter](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/cpinter/32/7995_2.png) [@cpinter](https://discourse.slicer.org/u/cpinter)
#### Post date: [July 26, 2018, 6:41pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/9 "2018-07-26T18:41:05Z")

</div>

I have been thinking about this as well, but haven’t made this change, because trying to find a non-existent item is an operation that probably leads to errors, so logging an error can be useful.

I think instead of making this fail silently you could investigate why these operations want to access this non-existent item.

---

<div class="post-metadata">

### Author: ![Johan\_Andruejol](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/johan_andruejol/32/3639_2.png) [@Johan\_Andruejol](https://discourse.slicer.org/u/Johan_Andruejol)
#### Post date: [July 26, 2018, 7:16pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/10 "2018-07-26T19:16:08Z")

</div>

In this case, it’s because the `SubjectHierarchyItemModifiedEvent` is called with NULL calldata. This happens whenever you group modifications of the Subject Hierarchy Node together with a `StartModify()/EndModify()` (see here: [https://github.com/Slicer/Slicer/blob/master/Libs/MRML/Core/vtkMRMLNode.h#L478](https://github.com/Slicer/Slicer/blob/master/Libs/MRML/Core/vtkMRMLNode.h#L478)).

As I see it, we have two options:

- We simply ignore any event passed with NULL calldata. Users will have to be careful when using batch updated with the Subject Hierachy Node and call ItemModified() themselves if needed.
- We could re-implement `vtkMRMLNode::InvokeCustomModifiedEvent` and `vtkMRMLNode::InvokePendingModifiedEvent` in the Subject Hierarchy node to handle passing the item ID

I don’t feel that any of these are ideal, so feel free to suggest other ideas !

---

<div class="post-metadata">

### Author: ![lassoan](https://sea2.discourse-cdn.com/flex002/user_avatar/discourse.slicer.org/lassoan/32/13_2.png) [@lassoan](https://discourse.slicer.org/u/lassoan)
#### Post date: [July 26, 2018, 7:49pm UTC](https://discourse.slicer.org/t/subject-hierarchy-listening-to-events/3364/11 "2018-07-26T19:49:43Z")

</div>

Normally, any custom modified events with NULL calldata means that multiple updates happened, so a full update is needed. Therefore, for me the obvious solution is to handle this as a full update.

Batch processing is always has overhead, so it should only be used if either A. it is important to not invoke (custom) modified events before multiple modifications are completed; or B. to improve performance by replacing many small updates by one big update. If you find that You may decide to call Start/EndModify only when many items are modified (similarly how it is done [here](https://github.com/Slicer/Slicer/blob/master/Libs/MRML/Logic/vtkMRMLModelHierarchyLogic.cxx#L260), when batch processing mode is used only when more than a few models had to be updated).