Add c++ optimised uniqueValues method which respects processing context

Remove processing vector.uniqueValues/vector.getUniqueValues and port usage to c++ method Should be much faster than the python method, as the c++ method takes advantage of handing off the unique values calculation to the provider source whenever possible
qgis · Apr 26, 2017 · f247a7c · f247a7c
1 parent 91679b3
commit f247a7c
Show file tree

Hide file tree

Showing 10 changed files with 95 additions and 66 deletions.
diff --git a/python/core/processing/qgsprocessingutils.sip b/python/core/processing/qgsprocessingutils.sip
@@ -115,6 +115,15 @@ class QgsProcessingUtils
  :rtype: long
 %End
 
+    static QList< QVariant > uniqueValues( QgsVectorLayer *layer, int fieldIndex, const QgsProcessingContext &context );
+%Docstring
+ Returns a list of unique values contained in a single field in a ``layer``, when
+ the settings from the supplied ``context`` are respected. E.g. if the
+ context is set to only use selected features, then calling this will
+ return unique values from selected features in the layer.
+ :rtype: list of QVariant
+%End
+
 };
 
 

diff --git a/python/plugins/processing/algs/qgis/RandomExtractWithinSubsets.py b/python/plugins/processing/algs/qgis/RandomExtractWithinSubsets.py
@@ -90,7 +90,7 @@ def processAlgorithm(self, context, feedback):
 
         features = QgsProcessingUtils.getFeatures(layer, context)
         featureCount = QgsProcessingUtils.featureCount(layer, context)
-        unique = vector.getUniqueValues(layer, context, index)
+        unique = QgsProcessingUtils.uniqueValues(layer, index, context)
         value = int(self.getParameterValue(self.NUMBER))
         if method == 0:
             if value > featureCount:

diff --git a/python/plugins/processing/algs/qgis/RandomSelectionWithinSubsets.py b/python/plugins/processing/algs/qgis/RandomSelectionWithinSubsets.py
@@ -90,7 +90,7 @@ def processAlgorithm(self, context, feedback):
         layer.removeSelection()
         index = layer.fields().lookupField(field)
 
-        unique = vector.getUniqueValues(layer, context, index)
+        unique = QgsProcessingUtils.uniqueValues(layer, index, context)
         featureCount = layer.featureCount()
 
         value = int(self.getParameterValue(self.NUMBER))

diff --git a/python/plugins/processing/algs/qgis/UniqueValues.py b/python/plugins/processing/algs/qgis/UniqueValues.py
@@ -31,13 +31,15 @@
 
 from qgis.PyQt.QtGui import QIcon
 
+from qgis.core import QgsProcessingUtils
+
 from processing.core.GeoAlgorithm import GeoAlgorithm
 from processing.core.parameters import ParameterVector
 from processing.core.parameters import ParameterTableField
 from processing.core.outputs import OutputHTML
 from processing.core.outputs import OutputNumber
 from processing.core.outputs import OutputString
-from processing.tools import dataobjects, vector
+from processing.tools import dataobjects
 
 pluginPath = os.path.split(os.path.split(os.path.dirname(__file__))[0])[0]
 
@@ -76,7 +78,7 @@ def processAlgorithm(self, context, feedback):
         layer = dataobjects.getLayerFromString(self.getParameterValue(self.INPUT_LAYER))
         fieldName = self.getParameterValue(self.FIELD_NAME)
         outputFile = self.getOutputValue(self.OUTPUT)
-        values = vector.getUniqueValues(layer, context, layer.fields().lookupField(fieldName))
+        values = QgsProcessingUtils.uniqueValues(layer, layer.fields().lookupField(fieldName), context)
         self.createHTML(outputFile, values)
         self.setOutputValue(self.TOTAL_VALUES, len(values))
         self.setOutputValue(self.UNIQUE_VALUES, ';'.join([str(v) for v in

diff --git a/python/plugins/processing/algs/qgis/VectorSplit.py b/python/plugins/processing/algs/qgis/VectorSplit.py
@@ -75,7 +75,7 @@ def processAlgorithm(self, context, feedback):
         mkdir(directory)
 
         fieldIndex = layer.fields().lookupField(fieldName)
-        uniqueValues = vector.uniqueValues(layer, context, fieldIndex)
+        uniqueValues = QgsProcessingUtils.uniqueValues(layer, fieldIndex, context)
         baseName = os.path.join(directory, '{0}_{1}'.format(layer.name(), fieldName))
 
         fields = layer.fields()

diff --git a/python/plugins/processing/tests/ToolsTest.py b/python/plugins/processing/tests/ToolsTest.py
@@ -91,32 +91,6 @@ def testValues(self):
         res = vector.values(test_layer, context, 1)
         self.assertEqual(set(res[1]), set([5, 7, 3]))
 
-    def testUniqueValues(self):
-
-        context = QgsProcessingContext()
-        # disable check for geometry validity
-        context.setFlags(QgsProcessingContext.Flags(0))
-
-        test_data = points()
-        test_layer = QgsVectorLayer(test_data, 'test', 'ogr')
-
-        # field by index
-        v = vector.uniqueValues(test_layer, context, 2)
-        self.assertEqual(len(v), len(set(v)))
-        self.assertEqual(set(v), set([2, 1, 0]))
-
-        # field by name
-        v = vector.uniqueValues(test_layer, context, 'id2')
-        self.assertEqual(len(v), len(set(v)))
-        self.assertEqual(set(v), set([2, 1, 0]))
-
-        # test with selected features
-        context.setFlags(QgsProcessingContext.UseSelectionIfPresent)
-        test_layer.selectByIds([2, 4, 6])
-        v = vector.uniqueValues(test_layer, context, 'id')
-        self.assertEqual(len(v), len(set(v)))
-        self.assertEqual(set(v), set([5, 7, 3]))
-
     def testOgrLayerNameExtraction(self):
         outdir = tempfile.mkdtemp()
         self.cleanup_paths.append(outdir)

diff --git a/python/plugins/processing/tools/vector.py b/python/plugins/processing/tools/vector.py
@@ -85,32 +85,6 @@
 }
 
 
-def uniqueValues(layer, context, attribute):
-    """Returns a list of unique values for a given attribute.
-
-    Attribute can be defined using a field names or a zero-based
-    field index. It considers the existing selection.
-    :param context:
-    """
-
-    fieldIndex = resolveFieldIndex(layer, attribute)
-    if context.flags() & QgsProcessingContext.UseSelectionIfPresent \
-            and layer.selectedFeatureCount() > 0:
-
-        # iterate through selected features
-        values = []
-        request = QgsFeatureRequest().setSubsetOfAttributes([fieldIndex]).setFlags(QgsFeatureRequest.NoGeometry)
-        feats = QgsProcessingUtils.getFeatures(layer, context, request)
-        for feat in feats:
-            if feat.attributes()[fieldIndex] not in values:
-                values.append(feat.attributes()[fieldIndex])
-        return values
-    else:
-        # no selection, or not considering selecting
-        # so we can take advantage of provider side unique value optimisations
-        return layer.uniqueValues(fieldIndex)
-
-
 def resolveFieldIndex(layer, attr):
     """This method takes an object and returns the index field it
     refers to in a layer. If the passed object is an integer, it
@@ -295,15 +269,6 @@ def simpleMeasure(geom, method=0, ellips=None, crs=None):
     return (attr1, attr2)
 
 
-def getUniqueValues(layer, context, fieldIndex):
-    values = []
-    feats = QgsProcessingUtils.getFeatures(layer, context)
-    for feat in feats:
-        if feat.attributes()[fieldIndex] not in values:
-            values.append(feat.attributes()[fieldIndex])
-    return values
-
-
 def combineVectorFields(layerA, layerB):
     """Create single field map from two input field maps.
     """

diff --git a/src/core/processing/qgsprocessingutils.cpp b/src/core/processing/qgsprocessingutils.cpp
@@ -199,3 +199,33 @@ long QgsProcessingUtils::featureCount( QgsVectorLayer *layer, const QgsProcessin
     return layer->featureCount();
 }
 
+QList<QVariant> QgsProcessingUtils::uniqueValues( QgsVectorLayer *layer, int fieldIndex, const QgsProcessingContext &context )
+{
+  if ( !layer )
+    return QList<QVariant>();
+
+  if ( fieldIndex < 0 || fieldIndex >= layer->fields().count() )
+    return QList<QVariant>();
+
+  bool useSelection = context.flags() & QgsProcessingContext::UseSelectionIfPresent && layer->selectedFeatureCount() > 0;
+  if ( !useSelection )
+  {
+    // not using selection, so use provider optimised version
+    QList<QVariant> values;
+    layer->uniqueValues( fieldIndex, values );
+    return values;
+  }
+  else
+  {
+    // using selection, so we have to iterate through selected features
+    QSet<QVariant> values;
+    QgsFeature f;
+    QgsFeatureIterator it = layer->selectedFeaturesIterator( QgsFeatureRequest().setSubsetOfAttributes( QgsAttributeList() << fieldIndex ).setFlags( QgsFeatureRequest::NoGeometry ) );
+    while ( it.nextFeature( f ) )
+    {
+      values.insert( f.attribute( fieldIndex ) );
+    }
+    return values.toList();
+  }
+}
+
diff --git a/src/core/processing/qgsprocessingutils.h b/src/core/processing/qgsprocessingutils.h
@@ -122,6 +122,14 @@ class CORE_EXPORT QgsProcessingUtils
      */
     static long featureCount( QgsVectorLayer *layer, const QgsProcessingContext &context );
 
+    /**
+     * Returns a list of unique values contained in a single field in a \a layer, when
+     * the settings from the supplied \a context are respected. E.g. if the
+     * context is set to only use selected features, then calling this will
+     * return unique values from selected features in the layer.
+     */
+    static QList< QVariant > uniqueValues( QgsVectorLayer *layer, int fieldIndex, const QgsProcessingContext &context );
+
   private:
 
     static bool canUseLayer( const QgsRasterLayer *layer );

diff --git a/tests/src/core/testqgsprocessing.cpp b/tests/src/core/testqgsprocessing.cpp
@@ -106,6 +106,7 @@ class TestQgsProcessing: public QObject
     void mapLayerFromString();
     void algorithm();
     void features();
+    void uniqueValues();
 
   private:
 
@@ -528,5 +529,45 @@ void TestQgsProcessing::features()
   delete polyLayer;
 }
 
+void TestQgsProcessing::uniqueValues()
+{
+  QgsVectorLayer *layer = new QgsVectorLayer( "Point?field=a:integer&field=b:string", "v1", "memory" );
+  for ( int i = 0; i < 6; ++i )
+  {
+    QgsFeature f( i );
+    f.setAttributes( QgsAttributes() << i % 3 + 1 << QString( QChar( ( i % 3 ) + 65 ) ) );
+    layer->dataProvider()->addFeatures( QgsFeatureList() << f );
+  }
+
+  QgsProcessingContext context;
+  context.setFlags( QgsProcessingContext::Flags( 0 ) );
+
+  // some bad checks
+  QVERIFY( QgsProcessingUtils::uniqueValues( nullptr, 0, context ).isEmpty() );
+  QVERIFY( QgsProcessingUtils::uniqueValues( nullptr, -1, context ).isEmpty() );
+  QVERIFY( QgsProcessingUtils::uniqueValues( nullptr, 10001, context ).isEmpty() );
+  QVERIFY( QgsProcessingUtils::uniqueValues( layer, -1, context ).isEmpty() );
+  QVERIFY( QgsProcessingUtils::uniqueValues( layer, 10001, context ).isEmpty() );
+
+  // good checks
+  QCOMPARE( QgsProcessingUtils::uniqueValues( layer, 0, context ), QList< QVariant >() << 1 << 2 << 3 );
+  QCOMPARE( QgsProcessingUtils::uniqueValues( layer, 1, context ), QList< QVariant >() << QString( "A" ) << QString( "B" ) << QString( "C" ) );
+
+  //using only selected features
+  layer->selectByIds( QgsFeatureIds() << 1 << 2 << 4 );
+  // but not using selection yet...
+  QCOMPARE( QgsProcessingUtils::uniqueValues( layer, 0, context ), QList< QVariant >() << 1 << 2 << 3 );
+  QCOMPARE( QgsProcessingUtils::uniqueValues( layer, 1, context ), QList< QVariant >() << QString( "A" ) << QString( "B" ) << QString( "C" ) );
+
+  // selection and using selection
+  context.setFlags( QgsProcessingContext::UseSelectionIfPresent );
+  QVERIFY( QgsProcessingUtils::uniqueValues( layer, -1, context ).isEmpty() );
+  QVERIFY( QgsProcessingUtils::uniqueValues( layer, 10001, context ).isEmpty() );
+  QCOMPARE( QgsProcessingUtils::uniqueValues( layer, 0, context ), QList< QVariant >() << 1 << 2 );
+  QCOMPARE( QgsProcessingUtils::uniqueValues( layer, 1, context ), QList< QVariant >() << QString( "A" ) << QString( "B" ) );
+
+  delete layer;
+}
+
 QGSTEST_MAIN( TestQgsProcessing )
 #include "testqgsprocessing.moc"