Skip to content

Commit

Permalink
Make the C and Python TimeStamp round the same way.
Browse files Browse the repository at this point in the history
Fixes #41.

The C version now always rounds its seconds value, even when its decoding from a value with lots of noise, as when initially created from 6.123456789. Previously it would return 6.1234567780047655; now it returns 6.123457 which is what it would return after being round-tripped through raw().

The Python version also does this, by (expensively) unpacking the raw value it just computed.

These could both probably be better optimized.
  • Loading branch information
jamadden committed Aug 1, 2018
1 parent 1fe3387 commit 7ccaad4
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 4 deletions.
8 changes: 8 additions & 0 deletions CHANGES.rst
Expand Up @@ -31,6 +31,14 @@
- Remove some internal compatibility shims that are no longer
necessary. See `PR 82 <https://github.com/zopefoundation/persistent/pull/82>`_.

- The precision of ``TimeStamp`` objects created from 6 arguments with
floating point seconds values matches across C and Python, and
matches across trips through ``TimeStamp.raw()``. Previously, the C
version could initially have erroneous rounding and too much
false precision, while the Python version could have too much
precision. The raw/repr values have not changed. See `issue 41
<https://github.com/zopefoundation/persistent/issues/41>`_.

4.3.0 (2018-07-30)
------------------

Expand Down
7 changes: 6 additions & 1 deletion persistent/_timestamp.c
Expand Up @@ -197,10 +197,15 @@ static double
TimeStamp_sec(TimeStamp *self)
{
unsigned int v;
double sec;

v = (self->data[4] * 16777216 + self->data[5] * 65536
+ self->data[6] * 256 + self->data[7]);
return SCONV * v;
sec = SCONV * (double)v;
/* always round to 6 places for consistency; that's going to
happen after a trip through raw() */
sec = round(sec * 1000000.0) / 1000000.0;
return sec;
}

static PyObject *
Expand Down
22 changes: 20 additions & 2 deletions persistent/tests/test_timestamp.py
Expand Up @@ -398,14 +398,32 @@ def test_ordering(self):
self.assertTrue(big_c != small_py)
self.assertTrue(small_py != big_c)

def test_seconds_precision(self):
def test_seconds_precision(self, seconds=6.123456789):
# https://github.com/zopefoundation/persistent/issues/41
args = (2001, 2, 3, 4, 5, 6.123456789)
args = (2001, 2, 3, 4, 5, seconds)
c = self._makeC(*args)
py = self._makePy(*args)

self.assertEqual(c, py)
self.assertEqual(c.second(), py.second())

py2 = self._makePy(c.raw())
self.assertEqual(py2, c)

c2 = self._makeC(c.raw())
self.assertEqual(c2, c)

def test_seconds_precision_half(self):
# make sure our rounding matches
self.test_seconds_precision(seconds=6.5)
self.test_seconds_precision(seconds=6.55)
self.test_seconds_precision(seconds=6.555)
self.test_seconds_precision(seconds=6.5555)
self.test_seconds_precision(seconds=6.55555)
self.test_seconds_precision(seconds=6.555555)
self.test_seconds_precision(seconds=6.5555555)
self.test_seconds_precision(seconds=6.55555555)
self.test_seconds_precision(seconds=6.555555555)

def test_suite():
return unittest.defaultTestLoader.loadTestsFromName(__name__)
2 changes: 1 addition & 1 deletion persistent/timestamp.py
Expand Up @@ -93,7 +93,7 @@ def __init__(self, *args):
self._elements = _parseRaw(raw)
elif len(args) == 6:
self._raw = _makeRaw(*args)
self._elements = args
self._elements = _parseRaw(self._raw)
else:
raise TypeError('Pass either a single 8-octet arg '
'or 5 integers and a float')
Expand Down

0 comments on commit 7ccaad4

Please sign in to comment.