Bug report #16925
Field calculator "round" function wrong results if rounding to 8 or more decimals
| Status: | Closed | ||
|---|---|---|---|
| Priority: | Normal | ||
| Assignee: | - | ||
| Category: | Field calculator | ||
| Affected QGIS version: | 2.18.11 | Regression?: | No |
| Operating System: | Easy fix?: | No | |
| Pull Request or Patch supplied: | No | Resolution: | fixed/implemented |
| Crashes QGIS or corrupts data: | No | Copied to github as #: | 24824 |
Description
This is regardless of the datasource and field type, example:
round(40.70298315437546, 8) > -21.47483648
round(40.70298315437546, 7) > 40.7029832
Associated revisions
Use std::round instead of qRound
Now that our minimum VS studio version allowed supports std::round,
we should use that in place of Qt's qRound method.
Because:
- it doesn't truncate to int, resulting in unpredictable
behaviour (refs #16925)
- better to stick to standard c++ methods wherever possible,
since they're likely better supported and optimised by the
compilers
- it's a tiny reduction to the barrier for entry to QGIS
development (I'm sick of pointing out the need to use
qRound during PR reviews!)
History
#1
Updated by Alain Bertholom over 6 years ago
In core/qgsexpression.cpp fcnRound:
qRound(...) is called for rounding, an int is returned.
use the math round function instead of qRound should solve the problem.
The field calculator should return a floating point number in all case...
static QVariant fcnRound( const QVariantList& values, const QgsExpressionContext , QgsExpression parent )
{
if ( values.length() == 2 )
{
double number = getDoubleValue( values.at( 0 ), parent );
double scaler = pow( 10.0, getIntValue( values.at( 1 ), parent ) );
//return QVariant( qRound( number * scaler ) / scaler );
return QVariant( round( number * scaler ) / scaler );
}
if ( values.length() == 1 )
{
double number = getIntValue( values.at( 0 ), parent );
//return QVariant( qRound( number ) ).toInt();
return QVariant( round( number ) );
}return QVariant();
}#2
Updated by Nyall Dawson over 6 years ago
Nice fix Alain - can you open a pull request on GitHub with this change? That'll make it easy to review.
#3
Updated by Alain Bertholom over 6 years ago
On master, 2.18, or both?
#4
Updated by Jürgen Fischer over 6 years ago
Alain Bertholom wrote:
On master, 2.18, or both?
Note that 2.18 is built with VS2010 on Windows, which doesn't have std::round. See 8cb578f69, 0a1270b9f, ad437bfdff and 4f58f13822
#5
Updated by Nyall Dawson over 6 years ago
You can use qgsRound for 2.18, instead of qRound.
#6
Updated by Giovanni Manghi over 5 years ago
- Resolution set to fixed/implemented
- Status changed from Open to Closed
Works as expected on master.