Skip to content

Commit

Permalink
Don't use Qt 5.7's QVariant::toString for floats (#69)
Browse files Browse the repository at this point in the history
Qt 5.7 added `QLocale::FloatingPointShortest` (see qt/qtbase@726fed0),
and updated `QVariant` to use that (instead of the previous
implementation added in Qt 5.5) when converting floats and doubles to
string, resulting in slightly different output, and QCOMPARE unit test
failures.

In this change, we use the Qt 5.5 / 5.6 implementation because its at
least as accurate as 5.7+, and implementing a 5.7-compatible version
would be a major undertaking (needing to duplicate the third-party
double-conversion code Qt borrows from the V8 project).
  • Loading branch information
pcolby committed Aug 20, 2016
1 parent fc63b89 commit 8a09bfb
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 18 deletions.
24 changes: 18 additions & 6 deletions src/polar/v2/trainingsession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,25 @@
#include <zlib.h>
#endif

// As of Qt 5.5 increased the accuracy of QVariant::toString output for
// doubles and floats (see qtproject/qtbase@8153386). Here replicate
// that behaviour for pre-5.5 for a) better accuracy in older Qt versions
// and b) consistent output (eg in unit tests) across all Qt 5.x versions.
#if (QT_VERSION >= QT_VERSION_CHECK(5, 5, 0))
// Qt 5.5 increased the accuracy of QVariant::toString output for floats and
// doubles (see qtproject/qtbase@8153386), resulting in slightly different
// output, and QCOMPARE unit test failures.
// https://github.com/qt/qtbase/commit/8153386397087ce4f5c8997992edf5c1fd38b8db
//
// Qt 5.7 added QLocale::FloatingPointShortest (see qt/qtbase@726fed0), and
// updated QVariant to use that (instead of the Qt 5.5 change above) when
// converting floats and doubles to string, again resulting in slightly
// different output, and QCOMPARE unit test failures.
// https://github.com/qt/qtbase/commit/726fed0d67013cbfac7921d3d4613ca83406fb0f
//
// So, QVariant floats and doubles convert (and compare) differently between
// Qt 5.[0-4], 5.[5,6], and 5.7+. Here we use the Qt 5.5 / 5.6 implementation
// because its at least as accurate as 5.7+, and implementing a 5.7-compatible
// fallback would be a major undertaking (needing to duplicate the third-party
// double-conversion code Qt borrows from the V8 project).
#if (QT_VERSION >= QT_VERSION_CHECK(5, 5, 0)) && (QT_VERSION < QT_VERSION_CHECK(5, 7, 0))
#define VARIANT_TO_STRING(v) v.toString()
#else // Fallback implementation based closely on Qt 5.5's qvariant.h
#else // Fallback implementation based closely on Qt 5.5's qvariant.cpp
#ifndef DBL_MANT_DIG
#define DBL_MANT_DIG 53
#endif
Expand Down
36 changes: 24 additions & 12 deletions test/polar/v2/testtrainingsession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,31 @@ void compare(const QDomNodeList &a, const QDomNodeList &b)
}
}

void fuzzyCompare(const QString &a, const QString &b, bool &compared)
void fuzzyCompare(const QString &a, const QString &b)
{
bool aOK, bOK;
const double aDouble = a.toDouble(&aOK);
const double bDouble = b.toDouble(&bOK);
if (aOK && bOK) {
compared = true;
QCOMPARE(aDouble, bDouble);
// Only consider fuzzy comparisons for floating point numbers.
if ((a.contains(QLatin1Char('.'))) || (a.contains(QLatin1Char('.')))) {
if ((a.length() <= 10) && (b.length() <= 10)) { // float precision.
bool aOK, bOK;
const float aFloat = a.toFloat(&aOK);
const float bFloat = b.toFloat(&bOK);
if (aOK && bOK) {
QCOMPARE(aFloat, bFloat);
return;
}
} else { // double precision.
bool aOK, bOK;
const double aDouble = a.toDouble(&aOK);
const double bDouble = b.toDouble(&bOK);
if (aOK && bOK) {
QCOMPARE(aDouble, bDouble);
return;
}
}
}

// Either value was not a floating point number, so compare literal strings.
QCOMPARE(a, b);
}

void compare(const QDomNode &a, const QDomNode &b)
Expand All @@ -85,11 +101,7 @@ void compare(const QDomNode &a, const QDomNode &b)
QCOMPARE(a.namespaceURI(), b.namespaceURI());
QCOMPARE(a.nodeName(), b.nodeName());
QCOMPARE(a.nodeType(), b.nodeType());
bool compared = false;
fuzzyCompare(a.nodeValue(), b.nodeValue(), compared);
if (!compared) {
QCOMPARE(a.nodeValue(), b.nodeValue());
}
fuzzyCompare(a.nodeValue(), b.nodeValue());
QCOMPARE(a.prefix(), b.prefix());
}

Expand Down

0 comments on commit 8a09bfb

Please sign in to comment.