Skip to content

Commit 7cec3ef

Browse files
pierreloicqnyalldawson
pierreloicq
authored and
nyalldawson
committed
test classifySymmetri and changes to pass them; + editingFinished signal
1 parent cb382ed commit 7cec3ef

File tree

4 files changed

+90
-35
lines changed

4 files changed

+90
-35
lines changed

‎src/core/symbology/qgsgraduatedsymbolrenderer.cpp

+11-10
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ QgsSymbolList QgsGraduatedSymbolRenderer::symbols( QgsRenderContext &context ) c
529529
return lst;
530530
}
531531

532-
static void _makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, bool astride )
532+
void QgsGraduatedSymbolRenderer::makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, bool astride )
533533
{
534534
// remove the breaks that are above the existing opposite sign classes
535535
// to keep colors symmetrically balanced around symmetryPoint
@@ -538,15 +538,16 @@ static void _makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, b
538538

539539
if ( breaks.size() > 1 ) //to avoid crash when only 1 class
540540
{
541-
std::sort( breaks.begin(), breaks.end() );
541+
qSort( breaks.begin(), breaks.end() );
542542
// breaks contain the maximum of the distrib but not the minimum
543-
double distBelowSymmetricValue = std::abs( breaks[0] - symmetryPoint );
544-
double distAboveSymmetricValue = std::abs( breaks[ breaks.size() - 2 ] - symmetryPoint ) ;
545-
double absMin = std::min( std::abs( distAboveSymmetricValue ), std::abs( distBelowSymmetricValue ) );
543+
double distBelowSymmetricValue = qAbs( breaks[0] - symmetryPoint );
544+
double distAboveSymmetricValue = qAbs( breaks[ breaks.size() - 2 ] - symmetryPoint ) ;
545+
double absMin = qMin( qAbs( distAboveSymmetricValue ), qAbs( distBelowSymmetricValue ) );
546+
546547
// make symmetric
547548
for ( int i = 0; i <= breaks.size() - 2; ++i )
548549
{
549-
if ( std::abs( breaks.at( i ) - symmetryPoint ) > absMin )
550+
if ( qAbs( breaks.at( i ) - symmetryPoint ) >= ( absMin - qAbs( breaks[0] - breaks[1] ) / 100. ) )
550551
{
551552
breaks.removeAt( i );
552553
--i;
@@ -560,7 +561,7 @@ static void _makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, b
560561
}
561562
}
562563

563-
static QList<double> _calcEqualIntervalBreaks( double minimum, double maximum, int classes, bool useSymmetricMode, double symmetryPoint, bool astride )
564+
QList<double> QgsGraduatedSymbolRenderer::calcEqualIntervalBreaks( double minimum, double maximum, int classes, bool useSymmetricMode, double symmetryPoint, bool astride )
564565
{
565566
// Equal interval algorithm
566567
// 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
692693
symmetryPoint = mean; // otherwise symmetryPoint = symmetryPoint
693694

694695
QList<double> breaks = QgsSymbolLayerUtils::prettyBreaks( ( minimum - symmetryPoint ) / stdDev, ( maximum - symmetryPoint ) / stdDev, classes );
695-
_makeBreaksSymmetric( breaks, 0.0, astride ); //0.0 because breaks where computed on a centered distribution
696+
QgsGraduatedSymbolRenderer::makeBreaksSymmetric( breaks, 0.0, astride ); //0.0 because breaks where computed on a centered distribution
696697

697698
for ( int i = 0; i < breaks.count(); i++ ) //unNormalize breaks and put labels
698699
{
@@ -918,15 +919,15 @@ void QgsGraduatedSymbolRenderer::updateClasses( QgsVectorLayer *vlayer, Mode mod
918919

919920
if ( mode == EqualInterval )
920921
{
921-
breaks = _calcEqualIntervalBreaks( minimum, maximum, nclasses, mUseSymmetricMode, symmetryPoint, astride );
922+
breaks = QgsGraduatedSymbolRenderer::calcEqualIntervalBreaks( minimum, maximum, nclasses, mUseSymmetricMode, symmetryPoint, astride );
922923
}
923924
else if ( mode == Pretty )
924925
{
925926
breaks = QgsSymbolLayerUtils::prettyBreaks( minimum, maximum, nclasses );
926927
setListForCboPrettyBreaks( _breaksAsStrings( breaks ) );
927928

928929
if ( useSymmetricMode )
929-
_makeBreaksSymmetric( breaks, symmetryPoint, astride );
930+
QgsGraduatedSymbolRenderer::makeBreaksSymmetric( breaks, symmetryPoint, astride );
930931
}
931932
else if ( mode == Quantile || mode == Jenks || mode == StdDev )
932933
{

‎src/core/symbology/qgsgraduatedsymbolrenderer.h

+9
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,15 @@ class CORE_EXPORT QgsGraduatedSymbolRenderer : public QgsFeatureRenderer
270270
*/
271271
void setAstride( bool astride ) { mAstride = astride; }
272272

273+
/**
274+
* Remove the breaks that are above the existing opposite sign classes to keep colors symmetrically balanced around symmetryPoint
275+
* Does not put a break on the symmetryPoint
276+
* \since QGIS 3.4
277+
*/
278+
static void makeBreaksSymmetric( QList<double> &breaks, double symmetryPoint, bool astride );
279+
280+
static QList<double> calcEqualIntervalBreaks( double minimum, double maximum, int classes, bool useSymmetricMode, double symmetryPoint, bool astride );
281+
273282
/**
274283
* Recalculate classes for a layer
275284
* \param vlayer The layer being rendered (from which data values are calculated)

‎src/gui/symbology/qgsgraduatedsymbolrendererwidget.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ void QgsGraduatedSymbolRendererWidget::connectUpdateHandlers()
576576
connect( cbxClassifySymmetric, &QAbstractButton::toggled, this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
577577
connect( cbxAstride, &QAbstractButton::toggled, this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
578578
connect( cboSymmetryPointForPretty, static_cast<void ( QComboBox::* )( int )>( &QComboBox::activated ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
579-
connect( spinSymmetryPointForOtherMethods, static_cast<void( QgsDoubleSpinBox::* )( double )>( &QgsDoubleSpinBox::valueChanged ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
579+
connect( spinSymmetryPointForOtherMethods, static_cast<void( QgsDoubleSpinBox::* )()>( &QgsDoubleSpinBox::editingFinished ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
580580
}
581581

582582
// Connect/disconnect event handlers which trigger updating renderer
@@ -597,7 +597,7 @@ void QgsGraduatedSymbolRendererWidget::disconnectUpdateHandlers()
597597
disconnect( cbxClassifySymmetric, &QAbstractButton::toggled, this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
598598
disconnect( cbxAstride, &QAbstractButton::toggled, this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
599599
disconnect( cboSymmetryPointForPretty, static_cast<void ( QComboBox::* )( int )>( &QComboBox::activated ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
600-
disconnect( spinSymmetryPointForOtherMethods, static_cast<void( QgsDoubleSpinBox::* )( double )>( &QgsDoubleSpinBox::valueChanged ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
600+
disconnect( spinSymmetryPointForOtherMethods, static_cast<void( QgsDoubleSpinBox::* )()>( &QgsDoubleSpinBox::editingFinished ), this, &QgsGraduatedSymbolRendererWidget::classifyGraduated );
601601
}
602602

603603
void QgsGraduatedSymbolRendererWidget::updateUiFromRenderer( bool updateCount )

‎tests/src/core/testqgsgraduatedsymbolrenderer.cpp

+68-23
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
#include <QSettings>
2020

2121
#include "qgsgraduatedsymbolrenderer.h"
22+
#include "qgssymbollayerutils.h"
2223

23-
/** \ingroup UnitTests
24+
/**
25+
* \ingroup UnitTests
2426
* This is a unit test for the qgsGraduatedSymbolRenderer class.
2527
*/
2628

@@ -35,7 +37,7 @@ class TestQgsGraduatedSymbolRenderer: public QObject
3537
void cleanup();// will be called after every testfunction.
3638
void rangesOverlap();
3739
void rangesHaveGaps();
38-
void _makeBreaksSymmetric(QList<double> &breaks, double symmetryPoint, bool astride);
40+
void classifySymmetric();
3941

4042

4143
private:
@@ -141,28 +143,71 @@ void TestQgsGraduatedSymbolRenderer::rangesHaveGaps()
141143
QVERIFY( renderer.rangesHaveGaps() );
142144
}
143145

144-
void TestQgsGraduatedSymbolRenderer::_makeBreaksSymmetric(QList<double> &breaks, double symmetryPoint, bool astride)
146+
// this function is used only on breaks that already contain the symmetryPoint
147+
// calcEqualIntervalBreaks takes symmetryPoint as parameter
148+
void TestQgsGraduatedSymbolRenderer::classifySymmetric()
145149
{
146-
const QList<double> unchanged_breaks = {1235, 1023, 997, 800, 555, 10, 1, -5, -11, -423, -811};
147-
148-
// with astride = false
149-
breaks = unchanged_breaks;
150-
symmetryPoint = 12.0;
151-
astride = false;
152-
_makeBreaksSymmetric( breaks, symmetryPoint, astride );
153-
154-
QVERIFY( breaks.contains( symmetryPoint) );
155-
// /!\ breaks contain the maximum of the distrib but not the minimum ?
156-
QVERIFY( breaks.count() % 2 == 0 );
157-
158-
// with astride = true
159-
breaks = unchanged_breaks;
160-
symmetryPoint = 666.3;
161-
astride = true;
162-
_makeBreaksSymmetric( breaks, symmetryPoint, astride );
163-
164-
QVERIFY( breaks.contains( symmetryPoint) );
165-
QVERIFY( breaks.count() % 2 != 0 );
150+
// minimum < symmetryPointForEqualInterval < maximum
151+
// going below 1E-6 will result in a fail because C++ think 2.6e-06 - 2e-06 = 0
152+
QList<double> minimum = {15.30, 20, 20, 1111, 0.26, 0.000026, -1.56E10};
153+
QList<double> symmetryPointForEqualInterval = {122.6, 24.3, 26.3, 1563.3, 0.34, 0.000034, 0.56E10};
154+
QList<double> maximum = {253.6, 30, 30, 2222, 0.55, 0.000055, 1.25E10};
155+
156+
int newPosOfSymmetryPoint = 0;
157+
bool astride = false;
158+
double symmetryPoint = 0;
159+
bool useSymmetricMode = true;
160+
QList<double> breaks = {};
161+
162+
for ( int valTest = 0; valTest < minimum.size(); valTest++ )
163+
{
164+
//makes no sense with less than 3 classes
165+
for ( int nclasses = 3; nclasses < 30; nclasses++ )
166+
{
167+
// PRETTY BREAKS
168+
const QList<double> unchanged_breaks = QgsSymbolLayerUtils::prettyBreaks( minimum[valTest], maximum[valTest], nclasses );
169+
170+
// user can only choose a symmetryPoint which is part of the pretty breaks (this part is not tested here)
171+
// makes no sense to take the extreme breaks as symmetry point
172+
for ( int posOfSymmetryPoint = 1; posOfSymmetryPoint < unchanged_breaks.count() - 2; posOfSymmetryPoint++ )
173+
{
174+
symmetryPoint = unchanged_breaks[posOfSymmetryPoint];
175+
176+
// with astride = false
177+
astride = false;
178+
breaks = unchanged_breaks;
179+
QgsGraduatedSymbolRenderer::makeBreaksSymmetric( breaks, symmetryPoint, astride );
180+
QCOMPARE( breaks.count() % 2, 0 );
181+
// because the minimum is not in the breaks
182+
int newPosOfSymmetryPoint = breaks.count() / 2;
183+
QCOMPARE( breaks[ newPosOfSymmetryPoint - 1 ], symmetryPoint );
184+
185+
// with astride = true
186+
astride = true;
187+
breaks = unchanged_breaks;
188+
QgsGraduatedSymbolRenderer::makeBreaksSymmetric( breaks, symmetryPoint, astride );
189+
QCOMPARE( breaks.count() % 2, 1 );
190+
QVERIFY( !breaks.contains( symmetryPoint ) );
191+
}
192+
193+
// EQUAL INTERVALS
194+
useSymmetricMode = true;
195+
196+
// with astride = false
197+
astride = false;
198+
breaks = QgsGraduatedSymbolRenderer::calcEqualIntervalBreaks( minimum[valTest], maximum[valTest], nclasses, useSymmetricMode, symmetryPointForEqualInterval[valTest], astride );
199+
QCOMPARE( breaks.count() % 2, 0 );
200+
// because the minimum is not in the breaks
201+
newPosOfSymmetryPoint = breaks.count() / 2 ;
202+
QCOMPARE( breaks[ newPosOfSymmetryPoint - 1 ], symmetryPointForEqualInterval[valTest] );
203+
204+
// with astride = true
205+
astride = true;
206+
breaks = QgsGraduatedSymbolRenderer::calcEqualIntervalBreaks( minimum[valTest], maximum[valTest], nclasses, useSymmetricMode, symmetryPointForEqualInterval[valTest], astride );
207+
QCOMPARE( breaks.count() % 2, 1 );
208+
QVERIFY( !breaks.contains( symmetryPointForEqualInterval[valTest] ) );
209+
}
210+
}
166211
}
167212

168213
QGSTEST_MAIN( TestQgsGraduatedSymbolRenderer )

0 commit comments

Comments
 (0)