[processing] Fix inefficient values() method

Method was iterating over ever feature in a layer, including geometries and all attributes for EVERY attribute requested Add test and refactor so only one optimised iteration (eg no geometry, only required attributes) is used
qgis · Oct 12, 2016 · 05ea4be · 05ea4be · nirvn · Oct 12, 2016
1 parent 2665eb5
commit 05ea4be
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 7 deletions.
diff --git a/python/plugins/processing/tests/ToolsTest.py b/python/plugins/processing/tests/ToolsTest.py
@@ -84,6 +84,44 @@ def testFeatures(self):
 
         ProcessingConfig.setSettingValue(ProcessingConfig.USE_SELECTED, previous_value)
 
+    def testValues(self):
+        ProcessingConfig.initialize()
+
+        test_data = points2()
+        test_layer = QgsVectorLayer(test_data, 'test', 'ogr')
+
+        # field by index
+        res = vector.values(test_layer, 0)
+        self.assertEqual(res[0], [1, 2, 3, 4, 5, 6, 7, 8])
+
+        # field by name
+        res = vector.values(test_layer, 'id')
+        self.assertEqual(res['id'], [1, 2, 3, 4, 5, 6, 7, 8])
+
+        # two fields
+        res = vector.values(test_layer, 0, 3)
+        self.assertEqual(res[0], [1, 2, 3, 4, 5, 6, 7, 8])
+        self.assertEqual(res[3], [2, 1, 0, 2, 1, 0, 0, 0])
+
+        # two fields by name
+        res = vector.values(test_layer, 'id', 'id_2')
+        self.assertEqual(res['id'], [1, 2, 3, 4, 5, 6, 7, 8])
+        self.assertEqual(res['id_2'], [2, 1, 0, 2, 1, 0, 0, 0])
+
+        # two fields by name and index
+        res = vector.values(test_layer, 'id', 3)
+        self.assertEqual(res['id'], [1, 2, 3, 4, 5, 6, 7, 8])
+        self.assertEqual(res[3], [2, 1, 0, 2, 1, 0, 0, 0])
+
+        # test with selected features
+        previous_value = ProcessingConfig.getSetting(ProcessingConfig.USE_SELECTED)
+        ProcessingConfig.setSettingValue(ProcessingConfig.USE_SELECTED, True)
+        test_layer.selectByIds([2, 4, 6])
+        res = vector.values(test_layer, 0)
+        self.assertEqual(set(res[0]), set([5, 7, 3]))
+
+        ProcessingConfig.setSettingValue(ProcessingConfig.USE_SELECTED, previous_value)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/python/plugins/processing/tools/vector.py b/python/plugins/processing/tools/vector.py
@@ -163,17 +163,30 @@ def values(layer, *attributes):
     to a number.
     """
     ret = {}
+    indices = []
+    attr_keys = {}
     for attr in attributes:
         index = resolveFieldIndex(layer, attr)
-        values = []
-        feats = features(layer)
-        for feature in feats:
+        indices.append(index)
+        attr_keys[index] = attr
+
+    # use an optimised feature request
+    request = QgsFeatureRequest().setSubsetOfAttributes(indices).setFlags(QgsFeatureRequest.NoGeometry)
+
+    for feature in features(layer, request):
+        for i in indices:
+
+            # convert attribute value to number
             try:
-                v = float(feature.attributes()[index])
-                values.append(v)
+                v = float(feature.attributes()[i])
             except:
-                values.append(None)
-        ret[attr] = values
+                v = None
+
+            k = attr_keys[i]
+            if k in ret:
+                ret[k].append(v)
+            else:
+                ret[k] = [v]
     return ret