Issue #23517: Fix implementation of the ROUND_HALF_UP rounding mode in

datetime.datetime.fromtimestamp() and datetime.datetime.utcfromtimestamp().
microseconds sign should be kept before rounding.

Author: vstinner

Lib/datetime.py

@@ -1374,48 +1374,42 @@ def tzinfo(self):
         return self._tzinfo
 
     @classmethod
-    def fromtimestamp(cls, t, tz=None):
+    def _fromtimestamp(cls, t, utc, tz):
         """Construct a datetime from a POSIX timestamp (like time.time()).
 
         A timezone info object may be passed in as well.
         """
-        _check_tzinfo_arg(tz)
-
-        converter = _time.localtime if tz is None else _time.gmtime
-
-        t, frac = divmod(t, 1.0)
+        frac, t = _math.modf(t)
         us = _round_half_up(frac * 1e6)
-
-        # If timestamp is less than one microsecond smaller than a
-        # full second, us can be rounded up to 1000000.  In this case,
-        # roll over to seconds, otherwise, ValueError is raised
-        # by the constructor.
-        if us == 1000000:
+        if us >= 1000000:
             t += 1
-            us = 0
+            us -= 1000000
+        elif us < 0:
+            t -= 1
+            us += 1000000
+
+        converter = _time.gmtime if utc else _time.localtime
         y, m, d, hh, mm, ss, weekday, jday, dst = converter(t)
         ss = min(ss, 59)    # clamp out leap seconds if the platform has them
-        result = cls(y, m, d, hh, mm, ss, us, tz)
+        return cls(y, m, d, hh, mm, ss, us, tz)
+
+    @classmethod
+    def fromtimestamp(cls, t, tz=None):
+        """Construct a datetime from a POSIX timestamp (like time.time()).
+
+        A timezone info object may be passed in as well.
+        """
+        _check_tzinfo_arg(tz)
+
+        result = cls._fromtimestamp(t, tz is not None, tz)
         if tz is not None:
             result = tz.fromutc(result)
         return result
 
     @classmethod
     def utcfromtimestamp(cls, t):
         """Construct a naive UTC datetime from a POSIX timestamp."""
-        t, frac = divmod(t, 1.0)
-        us = _round_half_up(frac * 1e6)
-
-        # If timestamp is less than one microsecond smaller than a
-        # full second, us can be rounded up to 1000000.  In this case,
-        # roll over to seconds, otherwise, ValueError is raised
-        # by the constructor.
-        if us == 1000000:
-            t += 1
-            us = 0
-        y, m, d, hh, mm, ss, weekday, jday, dst = _time.gmtime(t)
-        ss = min(ss, 59)    # clamp out leap seconds if the platform has them
-        return cls(y, m, d, hh, mm, ss, us)
+        return cls._fromtimestamp(t, True, None)
 
     @classmethod
     def now(cls, tz=None):

Lib/test/datetimetester.py

@@ -668,6 +668,8 @@ def test_microsecond_rounding(self):
         eq(td(milliseconds=-0.6/1000), td(microseconds=-1))
         eq(td(seconds=0.5/10**6), td(microseconds=1))
         eq(td(seconds=-0.5/10**6), td(microseconds=-1))
+        eq(td(seconds=1/2**7), td(microseconds=7813))
+        eq(td(seconds=-1/2**7), td(microseconds=-7813))
 
         # Rounding due to contributions from more than one field.
         us_per_hour = 3600e6
@@ -1842,8 +1844,8 @@ def test_timestamp_aware(self):
                          18000 + 3600 + 2*60 + 3 + 4*1e-6)
 
     def test_microsecond_rounding(self):
-        for fts in [self.theclass.fromtimestamp,
-                    self.theclass.utcfromtimestamp]:
+        for fts in (datetime.fromtimestamp,
+                    self.theclass.utcfromtimestamp):
             zero = fts(0)
             self.assertEqual(zero.second, 0)
             self.assertEqual(zero.microsecond, 0)
@@ -1874,6 +1876,12 @@ def test_microsecond_rounding(self):
             t = fts(0.9999999)
             self.assertEqual(t.second, 1)
             self.assertEqual(t.microsecond, 0)
+            t = fts(1/2**7)
+            self.assertEqual(t.second, 0)
+            self.assertEqual(t.microsecond, 7813)
+            t = fts(-1/2**7)
+            self.assertEqual(t.second, 59)
+            self.assertEqual(t.microsecond, 992187)
 
     def test_insane_fromtimestamp(self):
         # It's possible that some platform maps time_t to double,

Lib/test/test_time.py

@@ -655,7 +655,7 @@ def test_time_t(self):
                               pytime_object_to_time_t, invalid, rnd)
 
     @support.cpython_only
-    def test_timespec(self):
+    def test_object_to_timespec(self):
         from _testcapi import pytime_object_to_timespec
 
         # Conversion giving the same result for all rounding methods
@@ -666,7 +666,7 @@ def test_timespec(self):
                 (-1, (-1, 0)),
 
                 # float
-                (-1.2, (-2, 800000000)),
+                (-1/2**7, (-1, 992187500)),
                 (-1.0, (-1, 0)),
                 (-1e-9, (-1, 999999999)),
                 (1e-9, (0, 1)),
@@ -693,7 +693,7 @@ def test_timespec(self):
 
             (1.1234567890, (1, 123456789), FLOOR),
             (1.1234567899, (1, 123456789), FLOOR),
-            (-1.1234567890, (-2, 876543211), FLOOR),
+            (-1.1234567890, (-2, 876543210), FLOOR),
             (-1.1234567891, (-2, 876543210), FLOOR),
             # Round towards infinity (+inf)
             (1.1234567890, (1, 123456790), CEILING),
@@ -1155,7 +1155,7 @@ def test_time_t(self):
             self.assertRaises(OverflowError,
                               pytime_object_to_time_t, invalid, rnd)
 
-    def test_timeval(self):
+    def test_object_to_timeval(self):
         from _testcapi import pytime_object_to_timeval
 
         # Conversion giving the same result for all rounding methods
@@ -1167,7 +1167,8 @@ def test_timeval(self):
 
                 # float
                 (-1.0, (-1, 0)),
-                (-1.2, (-2, 800000)),
+                (1/2**6, (0, 15625)),
+                (-1/2**6, (-1, 984375)),
                 (-1e-6, (-1, 999999)),
                 (1e-6, (0, 1)),
             ):
@@ -1225,7 +1226,7 @@ def test_timespec(self):
                 (-1.0, (-1, 0)),
                 (-1e-9, (-1, 999999999)),
                 (1e-9, (0, 1)),
-                (-1.2, (-2, 800000000)),
+                (-1/2**9, (-1, 998046875)),
             ):
                 with self.subTest(obj=obj, round=rnd, timespec=timespec):
                     self.assertEqual(pytime_object_to_timespec(obj, rnd),

Python/pytime.c

@@ -82,10 +82,6 @@ _PyTime_DoubleToDenominator(double d, time_t *sec, long *numerator,
     volatile double floatpart;
 
     floatpart = modf(d, &intpart);
-    if (floatpart < 0) {
-        floatpart += 1.0;
-        intpart -= 1.0;
-    }
 
     floatpart *= denominator;
     if (round == _PyTime_ROUND_HALF_UP)
@@ -98,6 +94,10 @@ _PyTime_DoubleToDenominator(double d, time_t *sec, long *numerator,
         floatpart -= denominator;
         intpart += 1.0;
     }
+    else if (floatpart < 0) {
+        floatpart += denominator;
+        intpart -= 1.0;
+    }
     assert(0.0 <= floatpart && floatpart < denominator);
 
     *sec = (time_t)intpart;