[graph] ListAttribute: replace '__getitem__ by 'at'

__getitem__ is spuriously called when the object is used inside a Qt model and exposed to QML, leading to unexpected behaviors

meshroom/core/graph.py

@@ -285,12 +285,15 @@ class ListAttribute(Attribute):
         super(ListAttribute, self).__init__(node, attributeDesc, isOutput, root, parent)
         self._value = ListModel(parent=self)
 
-    def __getitem__(self, item):
-        return self._value.at(item)
-
     def __len__(self):
         return len(self._value)
 
+    def at(self, idx):
+        """ Returns child attribute at index 'idx' """
+        # implement 'at' rather than '__getitem__'
+        # since the later is called spuriously when object is used in QML
+        return self._value.at(idx)
+
     def _set_value(self, value):
         self.desc.validateValue(value)
         self._value.clear()
@@ -317,7 +320,7 @@ class ListAttribute(Attribute):
     def remove(self, index, count=1):
         if self.node.graph:
             for i in range(index, index + count):
-                attr = self[i]
+                attr = self._value.at(i)
                 if attr.isLink:
                     self.node.graph.removeEdge(attr)  # delete edge if the attribute is linked
         self._value.removeAt(index, count)

meshroom/ui/graph.py

@@ -259,7 +259,7 @@ class UIGraph(QObject):
         if isinstance(dst, graph.ListAttribute):
             with self.groupedGraphModification("Insert and Add Edge on {}".format(dst.fullName())):
                 self.appendAttribute(dst)
-                self.push(commands.AddEdgeCommand(self._graph, src, dst[-1]))
+                self.push(commands.AddEdgeCommand(self._graph, src, dst.at(-1)))
         else:
             self.push(commands.AddEdgeCommand(self._graph, src, dst))
 

meshroom/ui/reconstruction.py

@@ -122,7 +122,7 @@ class LiveSfmManager(QObject):
     def imagePathsInCameraInit(self, node):
         """ Get images in the given CameraInit node. """
         assert node.nodeType == 'CameraInit'
-        return [vp.path.value for vp in node.viewpoints]
+        return [vp.path.value for vp in node.viewpoints.value]
 
     def imagesInStep(self):
         """ Get images in the current augmentation step. """
@@ -304,11 +304,11 @@ class Reconstruction(UIGraph):
 
     def allImagePaths(self):
         """ Get all image paths in the reconstruction. """
-        return [vp.path.value for node in self._cameraInits for vp in node.viewpoints]
+        return [vp.path.value for node in self._cameraInits for vp in node.viewpoints.value]
 
     def allViewIds(self):
         """ Get all view Ids involved in the reconstruction. """
-        return [vp.viewId.value for node in self._cameraInits for vp in node.viewpoints]
+        return [vp.viewId.value for node in self._cameraInits for vp in node.viewpoints.value]
 
     @Slot(QObject, graph.Node)
     def handleFilesDrop(self, drop, cameraInit):

tests/test_multiviewPipeline.py

@@ -17,10 +17,10 @@ def test_multiviewPipeline():
         {'path': '/non/existing/file2', 'intrinsicId': 55}
         ])
 
-    assert graph1.findNode('CameraInit').viewpoints[0].path.value == '/non/existing/fileA'
+    assert graph1.findNode('CameraInit').viewpoints.at(0).path.value == '/non/existing/fileA'
     assert len(graph2.findNode('CameraInit').viewpoints) == 0
-    assert graph3.findNode('CameraInit').viewpoints[0].path.value == '/non/existing/file1'
-    assert graph4.findNode('CameraInit').viewpoints[0].path.value == '/non/existing/file1'
+    assert graph3.findNode('CameraInit').viewpoints.at(0).path.value == '/non/existing/file1'
+    assert graph4.findNode('CameraInit').viewpoints.at(0).path.value == '/non/existing/file1'
 
     assert len(graph1.findNode('CameraInit').viewpoints) == 1
     assert len(graph2.findNode('CameraInit').viewpoints) == 0
@@ -28,15 +28,15 @@ def test_multiviewPipeline():
     assert len(graph4.findNode('CameraInit').viewpoints) == 2
 
     viewpoints = graph3.findNode('CameraInit').viewpoints
-    assert viewpoints[0].path.value == '/non/existing/file1'
+    assert viewpoints.at(0).path.value == '/non/existing/file1'
 
-    assert viewpoints[0].path.value == '/non/existing/file1'
-    assert viewpoints[0].intrinsicId.value == -1
-    assert viewpoints[1].path.value == '/non/existing/file2'
-    assert viewpoints[1].intrinsicId.value == -1
+    assert viewpoints.at(0).path.value == '/non/existing/file1'
+    assert viewpoints.at(0).intrinsicId.value == -1
+    assert viewpoints.at(1).path.value == '/non/existing/file2'
+    assert viewpoints.at(1).intrinsicId.value == -1
 
-    assert viewpoints[0].path.isDefault == False
-    assert viewpoints[0].intrinsicId.isDefault == True
+    assert not viewpoints.at(0).path.isDefault
+    assert viewpoints.at(0).intrinsicId.isDefault
     assert viewpoints.getPrimitiveValue(exportDefault=False) == [
         {"path": '/non/existing/file1'},
         {"path": '/non/existing/file2'},
@@ -44,10 +44,10 @@ def test_multiviewPipeline():
 
     for graph in (graph4, graph4b):
         viewpoints = graph.findNode('CameraInit').viewpoints
-        assert viewpoints[0].path.value == '/non/existing/file1'
-        assert viewpoints[0].intrinsicId.value == 50
-        assert viewpoints[1].path.value == '/non/existing/file2'
-        assert viewpoints[1].intrinsicId.value == 55
+        assert viewpoints.at(0).path.value == '/non/existing/file1'
+        assert viewpoints.at(0).intrinsicId.value == 50
+        assert viewpoints.at(1).path.value == '/non/existing/file2'
+        assert viewpoints.at(1).intrinsicId.value == 55
 
     # Ensure that all output UIDs are different as the input is different:
     # graph1 != graph2 != graph3 != graph4