Merge pull request #83 from zopefoundation/issue41

Make the C and Python TimeStamp round the same way

CHANGES.rst

@@ -37,6 +37,15 @@
 - Remove some internal compatibility shims that are no longer
   necessary. See `PR 82 <https://github.com/zopefoundation/persistent/pull/82>`_.
 
+- Make the return value of ``TimeStamp.second()`` consistent across C
+  and Python implementations when the ``TimeStamp`` was created from 6
+  arguments with floating point seconds. Also make it match 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)
 ------------------
 

persistent/tests/test_timestamp.py

@@ -17,6 +17,8 @@
 MAX_32_BITS = 2 ** 31 - 1
 MAX_64_BITS = 2 ** 63 - 1
 
+import persistent.timestamp
+
 class Test__UTC(unittest.TestCase):
 
     def _getTargetClass(self):
@@ -202,7 +204,8 @@ def _getTargetClass(self):
         from persistent.timestamp import TimeStamp
         return TimeStamp
 
-
+@unittest.skipIf(persistent.timestamp.CTimeStamp is None,
+                 "CTimeStamp not available")
 class PyAndCComparisonTests(unittest.TestCase):
     """
     Compares C and Python implementations.
@@ -254,7 +257,6 @@ def test_raw_equal(self):
 
     def test_equal(self):
         c, py = self._make_C_and_Py(*self.now_ts_args)
-
         self.assertEqual(c, py)
 
     def test_hash_equal(self):
@@ -396,22 +398,32 @@ def test_ordering(self):
         self.assertTrue(big_c != small_py)
         self.assertTrue(small_py != big_c)
 
+    def test_seconds_precision(self, seconds=6.123456789):
+        # https://github.com/zopefoundation/persistent/issues/41
+        args = (2001, 2, 3, 4, 5, seconds)
+        c = self._makeC(*args)
+        py = self._makePy(*args)
 
-def test_suite():
-    suite = [
-        unittest.makeSuite(Test__UTC),
-        unittest.makeSuite(pyTimeStampTests),
-        unittest.makeSuite(TimeStampTests),
-    ]
+        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)
 
-    try:
-        from persistent.timestamp import pyTimeStamp
-        from persistent.timestamp import TimeStamp
-    except ImportError: # pragma: no cover
-        pass
-    else:
-        if pyTimeStamp != TimeStamp:
-            # We have both implementations available
-            suite.append(unittest.makeSuite(PyAndCComparisonTests))
-
-    return unittest.TestSuite(suite)
+def test_suite():
+    return unittest.defaultTestLoader.loadTestsFromName(__name__)

persistent/timestamp.py

@@ -53,6 +53,7 @@ def fromutc(self, dt):
         return dt
 
 def _makeUTC(y, mo, d, h, mi, s):
+    s = round(s, 6) # microsecond precision, to match the C implementation
     usec, sec = math.modf(s)
     sec = int(sec)
     usec = int(usec * 1e6)
@@ -75,29 +76,34 @@ def _parseRaw(octets):
     day = a // (60 * 24) % 31 + 1
     month = a // (60 * 24 * 31) % 12 + 1
     year = a // (60 * 24 * 31 * 12) + 1900
-    second = round(b * _SCONV, 6) #microsecond precision
+    second = b * _SCONV
     return (year, month, day, hour, minute, second)
 
 
 class pyTimeStamp(object):
     __slots__ = ('_raw', '_elements')
 
     def __init__(self, *args):
+        self._elements = None
         if len(args) == 1:
             raw = args[0]
             if not isinstance(raw, _RAWTYPE):
                 raise TypeError('Raw octets must be of type: %s' % _RAWTYPE)
             if len(raw) != 8:
                 raise TypeError('Raw must be 8 octets')
             self._raw = raw
-            self._elements = _parseRaw(raw)
         elif len(args) == 6:
             self._raw = _makeRaw(*args)
-            self._elements = args
+            # Note that we don't preserve the incoming arguments in self._elements,
+            # we derive them from the raw value. This is because the incoming
+            # seconds value could have more precision than would survive
+            # in the raw data, so we must be consistent.
         else:
             raise TypeError('Pass either a single 8-octet arg '
                             'or 5 integers and a float')
 
+        self._elements = _parseRaw(self._raw)
+
     def raw(self):
         return self._raw