Skip to content

Commit 651d1ed

Browse files
nyalldawsonnyalldawson
committedJan 11, 2017
Fix incorrect aggregate values returned for empty sets (fix #16008)
Now empty sets return NULL values for invalid statistics (cherry-picked from 64f8b4)
1 parent cfc3a86 commit 651d1ed

8 files changed

+102
-18
lines changed
 

‎src/app/qgsmergeattributesdialog.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,8 @@ QVariant QgsMergeAttributesDialog::calcStatistic( int col, QgsStatisticalSummary
362362

363363
summary.calculate( values );
364364

365-
return QVariant::fromValue( summary.statistic( stat ) );
365+
double val = summary.statistic( stat );
366+
return qIsNaN( val ) ? QVariant( QVariant::Double ) : val;
366367
}
367368

368369
QVariant QgsMergeAttributesDialog::concatenationAttribute( int col )

‎src/app/qgsstatisticalsummarydockwidget.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,9 @@ void QgsStatisticalSummaryDockWidget::updateNumericStatistics( bool selectedOnly
206206
int row = 0;
207207
Q_FOREACH ( QgsStatisticalSummary::Statistic stat, statsToDisplay )
208208
{
209+
double val = stats.statistic( stat );
209210
addRow( row, QgsStatisticalSummary::displayName( stat ),
210-
QString::number( stats.statistic( stat ) ),
211+
qIsNaN( val ) ? QString() : QString::number( val ),
211212
stats.count() != 0 );
212213
row++;
213214
}

‎src/core/qgsaggregatecalculator.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,8 @@ QVariant QgsAggregateCalculator::calculateNumericAggregate( QgsFeatureIterator&
402402
}
403403
}
404404
s.finalize();
405-
return s.statistic( stat );
405+
double val = s.statistic( stat );
406+
return qIsNaN( val ) ? QVariant() : val;
406407
}
407408

408409
QVariant QgsAggregateCalculator::calculateStringAggregate( QgsFeatureIterator& fit, int attr, QgsExpression* expression,

‎src/core/qgsstatisticalsummary.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,19 @@ void QgsStatisticalSummary::addVariant( const QVariant& value )
107107
void QgsStatisticalSummary::finalize()
108108
{
109109
if ( mCount == 0 )
110+
{
111+
mMin = std::numeric_limits<double>::quiet_NaN();
112+
mMax = std::numeric_limits<double>::quiet_NaN();
113+
mMean = std::numeric_limits<double>::quiet_NaN();
114+
mMedian = std::numeric_limits<double>::quiet_NaN();
115+
mStdev = std::numeric_limits<double>::quiet_NaN();
116+
mSampleStdev = std::numeric_limits<double>::quiet_NaN();
117+
mMinority = std::numeric_limits<double>::quiet_NaN();
118+
mMajority = std::numeric_limits<double>::quiet_NaN();
119+
mFirstQuartile = std::numeric_limits<double>::quiet_NaN();
120+
mThirdQuartile = std::numeric_limits<double>::quiet_NaN();
110121
return;
122+
}
111123

112124
mMean = mSum / mCount;
113125

‎src/core/qgsstatisticalsummary.h

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,8 @@ class CORE_EXPORT QgsStatisticalSummary
135135

136136
/** Returns the value of a specified statistic
137137
* @param stat statistic to return
138-
* @returns calculated value of statistic
138+
* @returns calculated value of statistic. A NaN value may be returned for invalid
139+
* statistics.
139140
*/
140141
double statistic( Statistic stat ) const;
141142

@@ -152,35 +153,42 @@ class CORE_EXPORT QgsStatisticalSummary
152153
*/
153154
double sum() const { return mSum; }
154155

155-
/** Returns calculated mean of values
156+
/** Returns calculated mean of values. A NaN value may be returned if the mean cannot
157+
* be calculated.
156158
*/
157159
double mean() const { return mMean; }
158160

159161
/** Returns calculated median of values. This is only calculated if Statistic::Median has
160-
* been specified in the constructor or via setStatistics.
162+
* been specified in the constructor or via setStatistics. A NaN value may be returned if the median cannot
163+
* be calculated.
161164
*/
162165
double median() const { return mMedian; }
163166

164-
/** Returns calculated minimum from values.
167+
/** Returns calculated minimum from values. A NaN value may be returned if the minimum cannot
168+
* be calculated.
165169
*/
166170
double min() const { return mMin; }
167171

168-
/** Returns calculated maximum from values.
172+
/** Returns calculated maximum from values. A NaN value may be returned if the maximum cannot
173+
* be calculated.
169174
*/
170175
double max() const { return mMax; }
171176

172-
/** Returns calculated range (difference between maximum and minimum values).
177+
/** Returns calculated range (difference between maximum and minimum values). A NaN value may be returned if the range cannot
178+
* be calculated.
173179
*/
174-
double range() const { return mMax - mMin; }
180+
double range() const { return qIsNaN( mMax ) || qIsNaN( mMin ) ? std::numeric_limits<double>::quiet_NaN() : mMax - mMin; }
175181

176182
/** Returns population standard deviation. This is only calculated if Statistic::StDev has
177-
* been specified in the constructor or via setStatistics.
183+
* been specified in the constructor or via setStatistics. A NaN value may be returned if the standard deviation cannot
184+
* be calculated.
178185
* @see sampleStDev
179186
*/
180187
double stDev() const { return mStdev; }
181188

182189
/** Returns sample standard deviation. This is only calculated if Statistic::StDev has
183-
* been specified in the constructor or via setStatistics.
190+
* been specified in the constructor or via setStatistics. A NaN value may be returned if the standard deviation cannot
191+
* be calculated.
184192
* @see stDev
185193
*/
186194
double sampleStDev() const { return mSampleStdev; }
@@ -193,38 +201,43 @@ class CORE_EXPORT QgsStatisticalSummary
193201

194202
/** Returns minority of values. The minority is the value with least occurances in the list
195203
* This is only calculated if Statistic::Minority has been specified in the constructor
196-
* or via setStatistics.
204+
* or via setStatistics. A NaN value may be returned if the minority cannot
205+
* be calculated.
197206
* @see majority
198207
*/
199208
double minority() const { return mMinority; }
200209

201210
/** Returns majority of values. The majority is the value with most occurances in the list
202211
* This is only calculated if Statistic::Majority has been specified in the constructor
203-
* or via setStatistics.
212+
* or via setStatistics. A NaN value may be returned if the majority cannot
213+
* be calculated.
204214
* @see minority
205215
*/
206216
double majority() const { return mMajority; }
207217

208218
/** Returns the first quartile of the values. The quartile is calculated using the
209-
* "Tukey's hinges" method.
219+
* "Tukey's hinges" method. A NaN value may be returned if the first quartile cannot
220+
* be calculated.
210221
* @see thirdQuartile
211222
* @see interQuartileRange
212223
*/
213224
double firstQuartile() const { return mFirstQuartile; }
214225

215226
/** Returns the third quartile of the values. The quartile is calculated using the
216-
* "Tukey's hinges" method.
227+
* "Tukey's hinges" method. A NaN value may be returned if the third quartile cannot
228+
* be calculated.
217229
* @see firstQuartile
218230
* @see interQuartileRange
219231
*/
220232
double thirdQuartile() const { return mThirdQuartile; }
221233

222234
/** Returns the inter quartile range of the values. The quartiles are calculated using the
223-
* "Tukey's hinges" method.
235+
* "Tukey's hinges" method. A NaN value may be returned if the IQR cannot
236+
* be calculated.
224237
* @see firstQuartile
225238
* @see thirdQuartile
226239
*/
227-
double interQuartileRange() const { return mThirdQuartile - mFirstQuartile; }
240+
double interQuartileRange() const { return qIsNaN( mThirdQuartile ) || qIsNaN( mFirstQuartile ) ? std::numeric_limits<double>::quiet_NaN() : mThirdQuartile - mFirstQuartile; }
228241

229242
/** Returns the friendly display name for a statistic
230243
* @param statistic statistic to return name for

‎tests/src/core/testqgsexpression.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,6 +1270,8 @@ class TestQgsExpression: public QObject
12701270
QTest::newRow( "filter context" ) << "aggregate('test','sum',\"col1\", \"col1\" <= @test_var)" << false << QVariant( 13 );
12711271
QTest::newRow( "filter named" ) << "aggregate(layer:='test',aggregate:='sum',expression:=\"col1\", filter:=\"col1\" <= 10)" << false << QVariant( 13 );
12721272
QTest::newRow( "filter no matching" ) << "aggregate('test','sum',\"col1\", \"col1\" <= -10)" << false << QVariant( 0 );
1273+
QTest::newRow( "filter no matching max" ) << "aggregate('test','max',\"col1\", \"col1\" > 1000000 )" << false << QVariant();
1274+
12731275
}
12741276

12751277
void aggregate()
@@ -1349,6 +1351,7 @@ class TestQgsExpression: public QObject
13491351
QTest::newRow( "filter" ) << "sum(\"col1\", NULL, \"col1\" >= 5)" << false << QVariant( 13 );
13501352
QTest::newRow( "filter named" ) << "sum(expression:=\"col1\", filter:=\"col1\" >= 5)" << false << QVariant( 13 );
13511353
QTest::newRow( "filter no matching" ) << "sum(expression:=\"col1\", filter:=\"col1\" <= -5)" << false << QVariant( 0 );
1354+
QTest::newRow( "filter no matching max" ) << "maximum('test',\"xcvxcv\" * 2)" << false << QVariant();
13521355

13531356
QTest::newRow( "group by" ) << "sum(\"col1\", \"col3\")" << false << QVariant( 9 );
13541357
QTest::newRow( "group by and filter" ) << "sum(\"col1\", \"col3\", \"col1\">=3)" << false << QVariant( 7 );

‎tests/src/core/testqgsstatisticalsummary.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ class TestQgsStatisticSummary: public QObject
3636
void individualStatCalculations();
3737
void maxMin();
3838
void countMissing();
39+
void noValues();
3940

4041
private:
4142

@@ -299,5 +300,45 @@ void TestQgsStatisticSummary::countMissing()
299300
QCOMPARE( s.statistic( QgsStatisticalSummary::CountMissing ), 3.0 );
300301
}
301302

303+
void TestQgsStatisticSummary::noValues()
304+
{
305+
// test returned stats when no values present
306+
QgsStatisticalSummary s( QgsStatisticalSummary::All );
307+
s.finalize();
308+
309+
QCOMPARE( s.count(), 0 );
310+
QCOMPARE( s.statistic( QgsStatisticalSummary::Count ), 0.0 );
311+
QCOMPARE( s.countMissing(), 0 );
312+
QCOMPARE( s.statistic( QgsStatisticalSummary::CountMissing ), 0.0 );
313+
QCOMPARE( s.sum(), 0.0 );
314+
QCOMPARE( s.statistic( QgsStatisticalSummary::Sum ), 0.0 );
315+
QVERIFY( qIsNaN( s.mean() ) );
316+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Mean ) ) );
317+
QVERIFY( qIsNaN( s.median() ) );
318+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Median ) ) );
319+
QVERIFY( qIsNaN( s.stDev() ) );
320+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::StDev ) ) );
321+
QVERIFY( qIsNaN( s.sampleStDev() ) );
322+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::StDevSample ) ) );
323+
QVERIFY( qIsNaN( s.min() ) );
324+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Min ) ) );
325+
QVERIFY( qIsNaN( s.max() ) );
326+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Max ) ) );
327+
QVERIFY( qIsNaN( s.range() ) );
328+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Range ) ) );
329+
QVERIFY( qIsNaN( s.minority() ) );
330+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Minority ) ) );
331+
QVERIFY( qIsNaN( s.majority() ) );
332+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Majority ) ) );
333+
QCOMPARE( s.variety(), 0 );
334+
QCOMPARE( s.statistic( QgsStatisticalSummary::Variety ), 0.0 );
335+
QVERIFY( qIsNaN( s.firstQuartile() ) );
336+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::FirstQuartile ) ) );
337+
QVERIFY( qIsNaN( s.thirdQuartile() ) );
338+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::ThirdQuartile ) ) );
339+
QVERIFY( qIsNaN( s.interQuartileRange() ) );
340+
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::InterQuartileRange ) ) );
341+
}
342+
302343
QTEST_MAIN( TestQgsStatisticSummary )
303344
#include "testqgsstatisticalsummary.moc"

‎tests/src/python/test_qgsaggregatecalculator.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,18 @@ def testExpressionNoMatch(self):
381381
self.assertTrue(ok)
382382
self.assertEqual(val, 0)
383383

384+
# min
385+
agg = QgsAggregateCalculator(layer)
386+
val, ok = agg.calculate(QgsAggregateCalculator.Min, 'fldint * 2')
387+
self.assertTrue(ok)
388+
self.assertEqual(val, None)
389+
390+
# max
391+
agg = QgsAggregateCalculator(layer)
392+
val, ok = agg.calculate(QgsAggregateCalculator.Max, 'fldint * 2')
393+
self.assertTrue(ok)
394+
self.assertEqual(val, None)
395+
384396
def testStringToAggregate(self):
385397
""" test converting strings to aggregate types """
386398

0 commit comments

Comments
 (0)