test classifySymmetri and changes to pass them; + editingFinished signal

qgis · Sep 14, 2018 · 7cec3ef · 7cec3ef
1 parent cb382ed
commit 7cec3ef
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 35 deletions.
diff --git a/src/core/symbology/qgsgraduatedsymbolrenderer.cpp b/src/core/symbology/qgsgraduatedsymbolrenderer.cpp
@@ -529,7 +529,7 @@ QgsSymbolList QgsGraduatedSymbolRenderer::symbols( QgsRenderContext &context ) c
   return lst;
 }
 
-static void _makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, bool astride )
+void QgsGraduatedSymbolRenderer::makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, bool astride )
 {
   // remove the breaks that are above the existing opposite sign classes
   // to keep colors symmetrically balanced around symmetryPoint
@@ -538,15 +538,16 @@ static void _makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, b
 
   if ( breaks.size() > 1 ) //to avoid crash when only 1 class
   {
-    std::sort( breaks.begin(), breaks.end() );
+    qSort( breaks.begin(), breaks.end() );
     // breaks contain the maximum of the distrib but not the minimum
-    double distBelowSymmetricValue = std::abs( breaks[0] - symmetryPoint );
-    double distAboveSymmetricValue = std::abs( breaks[ breaks.size() - 2 ] - symmetryPoint ) ;
-    double absMin = std::min( std::abs( distAboveSymmetricValue ), std::abs( distBelowSymmetricValue ) );
+    double distBelowSymmetricValue = qAbs( breaks[0] - symmetryPoint );
+    double distAboveSymmetricValue = qAbs( breaks[ breaks.size() - 2 ] - symmetryPoint ) ;
+    double absMin = qMin( qAbs( distAboveSymmetricValue ), qAbs( distBelowSymmetricValue ) );
+
     // make symmetric
     for ( int i = 0; i <= breaks.size() - 2; ++i )
     {
-      if ( std::abs( breaks.at( i ) - symmetryPoint ) > absMin )
+      if ( qAbs( breaks.at( i ) - symmetryPoint ) >= ( absMin - qAbs( breaks[0] - breaks[1] ) / 100. ) )
       {
         breaks.removeAt( i );
         --i;
@@ -560,7 +561,7 @@ static void _makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, b
   }
 }
 
-static QList<double> _calcEqualIntervalBreaks( double minimum, double maximum, int classes, bool useSymmetricMode, double symmetryPoint, bool astride )
+QList<double> QgsGraduatedSymbolRenderer::calcEqualIntervalBreaks( double minimum, double maximum, int classes, bool useSymmetricMode, double symmetryPoint, bool astride )
 {
   // Equal interval algorithm
   // Returns breaks based on dividing the range ('minimum' to 'maximum') into 'classes' parts.
@@ -692,7 +693,7 @@ static QList<double> _calcStdDevBreaks( QList<double> values, int classes, QList
     symmetryPoint = mean; // otherwise symmetryPoint = symmetryPoint
 
   QList<double> breaks = QgsSymbolLayerUtils::prettyBreaks( ( minimum - symmetryPoint ) / stdDev, ( maximum - symmetryPoint ) / stdDev, classes );
-  _makeBreaksSymmetric( breaks, 0.0, astride ); //0.0 because breaks where computed on a centered distribution
+  QgsGraduatedSymbolRenderer::makeBreaksSymmetric( breaks, 0.0, astride ); //0.0 because breaks where computed on a centered distribution
 
   for ( int i = 0; i < breaks.count(); i++ ) //unNormalize breaks and put labels
   {
@@ -918,15 +919,15 @@ void QgsGraduatedSymbolRenderer::updateClasses( QgsVectorLayer *vlayer, Mode mod
 
   if ( mode == EqualInterval )
   {
-    breaks = _calcEqualIntervalBreaks( minimum, maximum, nclasses, mUseSymmetricMode, symmetryPoint, astride );
+    breaks = QgsGraduatedSymbolRenderer::calcEqualIntervalBreaks( minimum, maximum, nclasses, mUseSymmetricMode, symmetryPoint, astride );
   }
   else if ( mode == Pretty )
   {
     breaks = QgsSymbolLayerUtils::prettyBreaks( minimum, maximum, nclasses );
     setListForCboPrettyBreaks( _breaksAsStrings( breaks ) );
 
     if ( useSymmetricMode )
-      _makeBreaksSymmetric( breaks, symmetryPoint, astride );
+      QgsGraduatedSymbolRenderer::makeBreaksSymmetric( breaks, symmetryPoint, astride );
   }
   else if ( mode == Quantile || mode == Jenks || mode == StdDev )
   {

diff --git a/src/core/symbology/qgsgraduatedsymbolrenderer.h b/src/core/symbology/qgsgraduatedsymbolrenderer.h
@@ -270,6 +270,15 @@ class CORE_EXPORT QgsGraduatedSymbolRenderer : public QgsFeatureRenderer
      */
     void setAstride( bool astride ) { mAstride = astride; }
 
+    /**
+     * Remove the breaks that are above the existing opposite sign classes to keep colors symmetrically balanced around symmetryPoint
+     * Does not put a break on the symmetryPoint
+     * \since QGIS 3.4
+     */
+    static void makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, bool astride );
+
+    static QList<double> calcEqualIntervalBreaks( double minimum, double maximum, int classes, bool useSymmetricMode, double symmetryPoint, bool astride );
+
     /**
      * Recalculate classes for a layer
      * \param vlayer  The layer being rendered (from which data values are calculated)

diff --git a/src/gui/symbology/qgsgraduatedsymbolrendererwidget.cpp b/src/gui/symbology/qgsgraduatedsymbolrendererwidget.cpp
@@ -576,7 +576,7 @@ void QgsGraduatedSymbolRendererWidget::connectUpdateHandlers()
   connect( cbxClassifySymmetric, &QAbstractButton::toggled, this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
   connect( cbxAstride, &QAbstractButton::toggled, this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
   connect( cboSymmetryPointForPretty, static_cast<void ( QComboBox::* )( int )>( &QComboBox::activated ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
-  connect( spinSymmetryPointForOtherMethods, static_cast<void( QgsDoubleSpinBox::* )( double )>( &QgsDoubleSpinBox::valueChanged ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
+  connect( spinSymmetryPointForOtherMethods, static_cast<void( QgsDoubleSpinBox::* )()>( &QgsDoubleSpinBox::editingFinished ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
 }
 
 // Connect/disconnect event handlers which trigger updating renderer
@@ -597,7 +597,7 @@ void QgsGraduatedSymbolRendererWidget::disconnectUpdateHandlers()
   disconnect( cbxClassifySymmetric, &QAbstractButton::toggled, this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
   disconnect( cbxAstride, &QAbstractButton::toggled, this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
   disconnect( cboSymmetryPointForPretty, static_cast<void ( QComboBox::* )( int )>( &QComboBox::activated ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
-  disconnect( spinSymmetryPointForOtherMethods, static_cast<void( QgsDoubleSpinBox::* )( double )>( &QgsDoubleSpinBox::valueChanged ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
+  disconnect( spinSymmetryPointForOtherMethods, static_cast<void( QgsDoubleSpinBox::* )()>( &QgsDoubleSpinBox::editingFinished ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
 }
 
 void QgsGraduatedSymbolRendererWidget::updateUiFromRenderer( bool updateCount )

diff --git a/tests/src/core/testqgsgraduatedsymbolrenderer.cpp b/tests/src/core/testqgsgraduatedsymbolrenderer.cpp
@@ -19,8 +19,10 @@
 #include <QSettings>
 
 #include "qgsgraduatedsymbolrenderer.h"
+#include "qgssymbollayerutils.h"
 
-/** \ingroup UnitTests
+/**
+ * \ingroup UnitTests
  * This is a unit test for the qgsGraduatedSymbolRenderer class.
  */
 
@@ -35,7 +37,7 @@ class TestQgsGraduatedSymbolRenderer: public QObject
     void cleanup();// will be called after every testfunction.
     void rangesOverlap();
     void rangesHaveGaps();
-    void _makeBreaksSymmetric(QList<double> &breaks, double symmetryPoint, bool astride);
+    void classifySymmetric();
 
 
   private:
@@ -141,28 +143,71 @@ void TestQgsGraduatedSymbolRenderer::rangesHaveGaps()
   QVERIFY( renderer.rangesHaveGaps() );
 }
 
-void TestQgsGraduatedSymbolRenderer::_makeBreaksSymmetric(QList<double> &breaks, double symmetryPoint, bool astride)
+// this function is used only on breaks that already contain the symmetryPoint
+// calcEqualIntervalBreaks takes symmetryPoint as parameter
+void TestQgsGraduatedSymbolRenderer::classifySymmetric()
 {
-  const QList<double> unchanged_breaks = {1235, 1023, 997, 800, 555, 10, 1, -5, -11, -423, -811};
-
-  // with astride = false
-  breaks = unchanged_breaks;
-  symmetryPoint = 12.0;
-  astride = false;
-  _makeBreaksSymmetric( breaks, symmetryPoint, astride );
-
-  QVERIFY( breaks.contains( symmetryPoint) );
-  // /!\ breaks contain the maximum of the distrib but not the minimum ?
-  QVERIFY( breaks.count() % 2 == 0 ); 
-
-  // with astride = true
-  breaks = unchanged_breaks;
-  symmetryPoint = 666.3;
-  astride = true;  
-  _makeBreaksSymmetric( breaks, symmetryPoint, astride );
-
-  QVERIFY( breaks.contains( symmetryPoint) );
-  QVERIFY( breaks.count() % 2 != 0 );   
+  // minimum < symmetryPointForEqualInterval < maximum
+  // going below 1E-6 will result in a fail because C++ think 2.6e-06 - 2e-06 = 0
+  QList<double> minimum =                       {15.30, 20,   20,     1111, 0.26, 0.000026, -1.56E10};
+  QList<double> symmetryPointForEqualInterval = {122.6, 24.3, 26.3, 1563.3, 0.34, 0.000034, 0.56E10};
+  QList<double> maximum =                       {253.6, 30,   30,     2222, 0.55, 0.000055, 1.25E10};
+
+  int newPosOfSymmetryPoint = 0;
+  bool astride = false;
+  double symmetryPoint = 0;
+  bool useSymmetricMode = true;
+  QList<double> breaks = {};
+
+  for ( int valTest = 0; valTest < minimum.size(); valTest++ )
+  {
+    //makes no sense with less than 3 classes
+    for ( int nclasses = 3; nclasses < 30; nclasses++ )
+    {
+      // PRETTY BREAKS
+      const QList<double> unchanged_breaks = QgsSymbolLayerUtils::prettyBreaks( minimum[valTest], maximum[valTest], nclasses );
+
+      // user can only choose a symmetryPoint which is part of the pretty breaks (this part is not tested here)
+      // makes no sense to take the extreme breaks as symmetry point
+      for ( int posOfSymmetryPoint = 1; posOfSymmetryPoint < unchanged_breaks.count() - 2; posOfSymmetryPoint++ )
+      {
+        symmetryPoint = unchanged_breaks[posOfSymmetryPoint];
+
+        // with astride = false
+        astride = false;
+        breaks = unchanged_breaks;
+        QgsGraduatedSymbolRenderer::makeBreaksSymmetric( breaks, symmetryPoint, astride );
+        QCOMPARE( breaks.count() % 2, 0 );
+        // because the minimum is not in the breaks
+        int newPosOfSymmetryPoint = breaks.count() / 2;
+        QCOMPARE( breaks[ newPosOfSymmetryPoint - 1 ], symmetryPoint );
+
+        // with astride = true
+        astride = true;
+        breaks = unchanged_breaks;
+        QgsGraduatedSymbolRenderer::makeBreaksSymmetric( breaks, symmetryPoint, astride );
+        QCOMPARE( breaks.count() % 2, 1 );
+        QVERIFY( !breaks.contains( symmetryPoint ) );
+      }
+
+      // EQUAL INTERVALS
+      useSymmetricMode = true;
+
+      // with astride = false
+      astride = false;
+      breaks = QgsGraduatedSymbolRenderer::calcEqualIntervalBreaks( minimum[valTest], maximum[valTest], nclasses, useSymmetricMode, symmetryPointForEqualInterval[valTest], astride );
+      QCOMPARE( breaks.count() % 2, 0 );
+      // because the minimum is not in the breaks
+      newPosOfSymmetryPoint = breaks.count() / 2 ;
+      QCOMPARE( breaks[ newPosOfSymmetryPoint - 1 ], symmetryPointForEqualInterval[valTest] );
+
+      // with astride = true
+      astride = true;
+      breaks = QgsGraduatedSymbolRenderer::calcEqualIntervalBreaks( minimum[valTest], maximum[valTest], nclasses, useSymmetricMode, symmetryPointForEqualInterval[valTest], astride );
+      QCOMPARE( breaks.count() % 2, 1 );
+      QVERIFY( !breaks.contains( symmetryPointForEqualInterval[valTest] ) );
+    }
+  }
 }
 
 QGSTEST_MAIN( TestQgsGraduatedSymbolRenderer )