Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix incorrect aggregate values returned for empty sets (fix #16008)
Now empty sets return NULL values for invalid statistics

(cherry-picked from 64f8b4)
  • Loading branch information
nyalldawson committed Jan 11, 2017
1 parent cfc3a86 commit 651d1ed
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 18 deletions.
3 changes: 2 additions & 1 deletion src/app/qgsmergeattributesdialog.cpp
Expand Up @@ -362,7 +362,8 @@ QVariant QgsMergeAttributesDialog::calcStatistic( int col, QgsStatisticalSummary

summary.calculate( values );

return QVariant::fromValue( summary.statistic( stat ) );
double val = summary.statistic( stat );
return qIsNaN( val ) ? QVariant( QVariant::Double ) : val;
}

QVariant QgsMergeAttributesDialog::concatenationAttribute( int col )
Expand Down
3 changes: 2 additions & 1 deletion src/app/qgsstatisticalsummarydockwidget.cpp
Expand Up @@ -206,8 +206,9 @@ void QgsStatisticalSummaryDockWidget::updateNumericStatistics( bool selectedOnly
int row = 0;
Q_FOREACH ( QgsStatisticalSummary::Statistic stat, statsToDisplay )
{
double val = stats.statistic( stat );
addRow( row, QgsStatisticalSummary::displayName( stat ),
QString::number( stats.statistic( stat ) ),
qIsNaN( val ) ? QString() : QString::number( val ),
stats.count() != 0 );
row++;
}
Expand Down
3 changes: 2 additions & 1 deletion src/core/qgsaggregatecalculator.cpp
Expand Up @@ -402,7 +402,8 @@ QVariant QgsAggregateCalculator::calculateNumericAggregate( QgsFeatureIterator&
}
}
s.finalize();
return s.statistic( stat );
double val = s.statistic( stat );
return qIsNaN( val ) ? QVariant() : val;
}

QVariant QgsAggregateCalculator::calculateStringAggregate( QgsFeatureIterator& fit, int attr, QgsExpression* expression,
Expand Down
12 changes: 12 additions & 0 deletions src/core/qgsstatisticalsummary.cpp
Expand Up @@ -107,7 +107,19 @@ void QgsStatisticalSummary::addVariant( const QVariant& value )
void QgsStatisticalSummary::finalize()
{
if ( mCount == 0 )
{
mMin = std::numeric_limits<double>::quiet_NaN();
mMax = std::numeric_limits<double>::quiet_NaN();
mMean = std::numeric_limits<double>::quiet_NaN();
mMedian = std::numeric_limits<double>::quiet_NaN();
mStdev = std::numeric_limits<double>::quiet_NaN();
mSampleStdev = std::numeric_limits<double>::quiet_NaN();
mMinority = std::numeric_limits<double>::quiet_NaN();
mMajority = std::numeric_limits<double>::quiet_NaN();
mFirstQuartile = std::numeric_limits<double>::quiet_NaN();
mThirdQuartile = std::numeric_limits<double>::quiet_NaN();
return;
}

mMean = mSum / mCount;

Expand Down
43 changes: 28 additions & 15 deletions src/core/qgsstatisticalsummary.h
Expand Up @@ -135,7 +135,8 @@ class CORE_EXPORT QgsStatisticalSummary

/** Returns the value of a specified statistic
* @param stat statistic to return
* @returns calculated value of statistic
* @returns calculated value of statistic. A NaN value may be returned for invalid
* statistics.
*/
double statistic( Statistic stat ) const;

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

/** Returns calculated mean of values
/** Returns calculated mean of values. A NaN value may be returned if the mean cannot
* be calculated.
*/
double mean() const { return mMean; }

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

/** Returns calculated minimum from values.
/** Returns calculated minimum from values. A NaN value may be returned if the minimum cannot
* be calculated.
*/
double min() const { return mMin; }

/** Returns calculated maximum from values.
/** Returns calculated maximum from values. A NaN value may be returned if the maximum cannot
* be calculated.
*/
double max() const { return mMax; }

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

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

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

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

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

/** Returns the first quartile of the values. The quartile is calculated using the
* "Tukey's hinges" method.
* "Tukey's hinges" method. A NaN value may be returned if the first quartile cannot
* be calculated.
* @see thirdQuartile
* @see interQuartileRange
*/
double firstQuartile() const { return mFirstQuartile; }

/** Returns the third quartile of the values. The quartile is calculated using the
* "Tukey's hinges" method.
* "Tukey's hinges" method. A NaN value may be returned if the third quartile cannot
* be calculated.
* @see firstQuartile
* @see interQuartileRange
*/
double thirdQuartile() const { return mThirdQuartile; }

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

/** Returns the friendly display name for a statistic
* @param statistic statistic to return name for
Expand Down
3 changes: 3 additions & 0 deletions tests/src/core/testqgsexpression.cpp
Expand Up @@ -1270,6 +1270,8 @@ class TestQgsExpression: public QObject
QTest::newRow( "filter context" ) << "aggregate('test','sum',\"col1\", \"col1\" <= @test_var)" << false << QVariant( 13 );
QTest::newRow( "filter named" ) << "aggregate(layer:='test',aggregate:='sum',expression:=\"col1\", filter:=\"col1\" <= 10)" << false << QVariant( 13 );
QTest::newRow( "filter no matching" ) << "aggregate('test','sum',\"col1\", \"col1\" <= -10)" << false << QVariant( 0 );
QTest::newRow( "filter no matching max" ) << "aggregate('test','max',\"col1\", \"col1\" > 1000000 )" << false << QVariant();

}

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

QTest::newRow( "group by" ) << "sum(\"col1\", \"col3\")" << false << QVariant( 9 );
QTest::newRow( "group by and filter" ) << "sum(\"col1\", \"col3\", \"col1\">=3)" << false << QVariant( 7 );
Expand Down
41 changes: 41 additions & 0 deletions tests/src/core/testqgsstatisticalsummary.cpp
Expand Up @@ -36,6 +36,7 @@ class TestQgsStatisticSummary: public QObject
void individualStatCalculations();
void maxMin();
void countMissing();
void noValues();

private:

Expand Down Expand Up @@ -299,5 +300,45 @@ void TestQgsStatisticSummary::countMissing()
QCOMPARE( s.statistic( QgsStatisticalSummary::CountMissing ), 3.0 );
}

void TestQgsStatisticSummary::noValues()
{
// test returned stats when no values present
QgsStatisticalSummary s( QgsStatisticalSummary::All );
s.finalize();

QCOMPARE( s.count(), 0 );
QCOMPARE( s.statistic( QgsStatisticalSummary::Count ), 0.0 );
QCOMPARE( s.countMissing(), 0 );
QCOMPARE( s.statistic( QgsStatisticalSummary::CountMissing ), 0.0 );
QCOMPARE( s.sum(), 0.0 );
QCOMPARE( s.statistic( QgsStatisticalSummary::Sum ), 0.0 );
QVERIFY( qIsNaN( s.mean() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Mean ) ) );
QVERIFY( qIsNaN( s.median() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Median ) ) );
QVERIFY( qIsNaN( s.stDev() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::StDev ) ) );
QVERIFY( qIsNaN( s.sampleStDev() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::StDevSample ) ) );
QVERIFY( qIsNaN( s.min() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Min ) ) );
QVERIFY( qIsNaN( s.max() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Max ) ) );
QVERIFY( qIsNaN( s.range() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Range ) ) );
QVERIFY( qIsNaN( s.minority() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Minority ) ) );
QVERIFY( qIsNaN( s.majority() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::Majority ) ) );
QCOMPARE( s.variety(), 0 );
QCOMPARE( s.statistic( QgsStatisticalSummary::Variety ), 0.0 );
QVERIFY( qIsNaN( s.firstQuartile() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::FirstQuartile ) ) );
QVERIFY( qIsNaN( s.thirdQuartile() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::ThirdQuartile ) ) );
QVERIFY( qIsNaN( s.interQuartileRange() ) );
QVERIFY( qIsNaN( s.statistic( QgsStatisticalSummary::InterQuartileRange ) ) );
}

QTEST_MAIN( TestQgsStatisticSummary )
#include "testqgsstatisticalsummary.moc"
12 changes: 12 additions & 0 deletions tests/src/python/test_qgsaggregatecalculator.py
Expand Up @@ -381,6 +381,18 @@ def testExpressionNoMatch(self):
self.assertTrue(ok)
self.assertEqual(val, 0)

# min
agg = QgsAggregateCalculator(layer)
val, ok = agg.calculate(QgsAggregateCalculator.Min, 'fldint * 2')
self.assertTrue(ok)
self.assertEqual(val, None)

# max
agg = QgsAggregateCalculator(layer)
val, ok = agg.calculate(QgsAggregateCalculator.Max, 'fldint * 2')
self.assertTrue(ok)
self.assertEqual(val, None)

def testStringToAggregate(self):
""" test converting strings to aggregate types """

Expand Down

0 comments on commit 651d1ed

Please sign in to comment.