Issue #23517: Fix rounding in fromtimestamp() and utcfromtimestamp() methods

of datetime.datetime: microseconds are now rounded to nearest with ties going
to nearest even integer (ROUND_HALF_EVEN), instead of being rounding towards
zero (ROUND_DOWN). It's important that these methods use the same rounding
mode than datetime.timedelta to keep the property:

   (datetime(1970,1,1) + timedelta(seconds=t)) == datetime.utcfromtimestamp(t)

It also the rounding mode used by round(float) for example.

Add more unit tests on the rounding mode in test_datetime.

4 files changed, 113 insertions, 41 deletions

diff --git a/Lib/datetime.py b/Lib/datetime.py
index 34e5d387cf..3af12e7716 100644
--- a/Lib/datetime.py
+++ b/Lib/datetime.py
@@ -1362,49 +1362,42 @@ class datetime(date):
         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.
         """
+        frac, t = _math.modf(t)
+        us = round(frac * 1e6)
+        if us >= 1000000:
+            t += 1
+            us -= 1000000
+        elif us < 0:
+            t -= 1
+            us += 1000000
 
-        _check_tzinfo_arg(tz)
+        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
+        return cls(y, m, d, hh, mm, ss, us, tz)
 
-        converter = _time.localtime if tz is None else _time.gmtime
+    @classmethod
+    def fromtimestamp(cls, t, tz=None):
+        """Construct a datetime from a POSIX timestamp (like time.time()).
 
-        t, frac = divmod(t, 1.0)
-        us = int(frac * 1e6)
+        A timezone info object may be passed in as well.
+        """
+        _check_tzinfo_arg(tz)
 
-        # 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 = 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)
+        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 UTC datetime from a POSIX timestamp (like time.time())."
-        t, frac = divmod(t, 1.0)
-        us = int(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)
+        """Construct a naive UTC datetime from a POSIX timestamp."""
+        return cls._fromtimestamp(t, True, None)
 
     # XXX This is supposed to do better than we *can* do by using time.time(),
     # XXX if the platform supports a more accurate way.  The C implementation
diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py
index 8e48b9fd53..a942d4de81 100644
--- a/Lib/test/datetimetester.py
+++ b/Lib/test/datetimetester.py
@@ -650,8 +650,16 @@ class TestTimeDelta(HarmlessMixedComparison, unittest.TestCase):
         # Single-field rounding.
         eq(td(milliseconds=0.4/1000), td(0))    # rounds to 0
         eq(td(milliseconds=-0.4/1000), td(0))    # rounds to 0
+        eq(td(milliseconds=0.5/1000), td(microseconds=0))
+        eq(td(milliseconds=-0.5/1000), td(microseconds=-0))
         eq(td(milliseconds=0.6/1000), td(microseconds=1))
         eq(td(milliseconds=-0.6/1000), td(microseconds=-1))
+        eq(td(milliseconds=1.5/1000), td(microseconds=2))
+        eq(td(milliseconds=-1.5/1000), td(microseconds=-2))
+        eq(td(seconds=0.5/10**6), td(microseconds=0))
+        eq(td(seconds=-0.5/10**6), td(microseconds=-0))
+        eq(td(seconds=1/2**7), td(microseconds=7812))
+        eq(td(seconds=-1/2**7), td(microseconds=-7812))
 
         # Rounding due to contributions from more than one field.
         us_per_hour = 3600e6
@@ -1824,12 +1832,14 @@ class TestDateTime(TestDate):
                           tzinfo=timezone(timedelta(hours=-5), 'EST'))
         self.assertEqual(t.timestamp(),
                          18000 + 3600 + 2*60 + 3 + 4*1e-6)
+
     def test_microsecond_rounding(self):
         for fts in [self.theclass.fromtimestamp,
                     self.theclass.utcfromtimestamp]:
             zero = fts(0)
             self.assertEqual(zero.second, 0)
             self.assertEqual(zero.microsecond, 0)
+            one = fts(1e-6)
             try:
                 minus_one = fts(-1e-6)
             except OSError:
@@ -1840,22 +1850,28 @@ class TestDateTime(TestDate):
                 self.assertEqual(minus_one.microsecond, 999999)
 
                 t = fts(-1e-8)
-                self.assertEqual(t, minus_one)
+                self.assertEqual(t, zero)
                 t = fts(-9e-7)
                 self.assertEqual(t, minus_one)
                 t = fts(-1e-7)
-                self.assertEqual(t, minus_one)
+                self.assertEqual(t, zero)
+                t = fts(-1/2**7)
+                self.assertEqual(t.second, 59)
+                self.assertEqual(t.microsecond, 992188)
 
             t = fts(1e-7)
             self.assertEqual(t, zero)
             t = fts(9e-7)
-            self.assertEqual(t, zero)
+            self.assertEqual(t, one)
             t = fts(0.99999949)
             self.assertEqual(t.second, 0)
             self.assertEqual(t.microsecond, 999999)
             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, 999999)
+            self.assertEqual(t.microsecond, 7812)
 
     def test_insane_fromtimestamp(self):
         # It's possible that some platform maps time_t to double,
diff --git a/Misc/NEWS b/Misc/NEWS
index e9fe6c7c98..d570bd49dc 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -81,6 +81,14 @@ Core and Builtins
 Library
 -------
 
+- Issue #23517: Fix rounding in fromtimestamp() and utcfromtimestamp() methods
+  of datetime.datetime: microseconds are now rounded to nearest with ties
+  going to nearest even integer (ROUND_HALF_EVEN), instead of being rounding
+  towards zero (ROUND_DOWN). It's important that these methods use the same
+  rounding mode than datetime.timedelta to keep the property:
+  (datetime(1970,1,1) + timedelta(seconds=t)) == datetime.utcfromtimestamp(t).
+  It also the rounding mode used by round(float) for example.
+
 - Issue #24684: socket.socket.getaddrinfo() now calls
   PyUnicode_AsEncodedString() instead of calling the encode() method of the
   host, to handle correctly custom string with an encode() method which doesn't
diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c
index d8225bae49..cabe4edb6a 100644
--- a/Modules/_datetimemodule.c
+++ b/Modules/_datetimemodule.c
@@ -4113,6 +4113,44 @@ datetime_from_timet_and_us(PyObject *cls, TM_FUNC f, time_t timet, int us,
                                  tzinfo);
 }
 
+static time_t
+_PyTime_DoubleToTimet(double x)
+{
+    time_t result;
+    double diff;
+
+    result = (time_t)x;
+    /* How much info did we lose?  time_t may be an integral or
+     * floating type, and we don't know which.  If it's integral,
+     * we don't know whether C truncates, rounds, returns the floor,
+     * etc.  If we lost a second or more, the C rounding is
+     * unreasonable, or the input just doesn't fit in a time_t;
+     * call it an error regardless.  Note that the original cast to
+     * time_t can cause a C error too, but nothing we can do to
+     * worm around that.
+     */
+    diff = x - (double)result;
+    if (diff <= -1.0 || diff >= 1.0) {
+        PyErr_SetString(PyExc_OverflowError,
+                        "timestamp out of range for platform time_t");
+        result = (time_t)-1;
+    }
+    return result;
+}
+
+/* Round a double to the nearest long.  |x| must be small enough to fit
+ * in a C long; this is not checked.
+ */
+static double
+_PyTime_RoundHalfEven(double x)
+{
+    double rounded = round(x);
+    if (fabs(x-rounded) == 0.5)
+        /* halfway case: round to even */
+        rounded = 2.0*round(x/2.0);
+    return rounded;
+}
+
 /* Internal helper.
  * Build datetime from a Python timestamp.  Pass localtime or gmtime for f,
  * to control the interpretation of the timestamp.  Since a double doesn't
@@ -4121,15 +4159,32 @@ datetime_from_timet_and_us(PyObject *cls, TM_FUNC f, time_t timet, int us,
  * to get that much precision (e.g., C time() isn't good enough).
  */
 static PyObject *
-datetime_from_timestamp(PyObject *cls, TM_FUNC f, PyObject *timestamp,
+datetime_from_timestamp(PyObject *cls, TM_FUNC f, double timestamp,
                         PyObject *tzinfo)
 {
     time_t timet;
-    long us;
+    double fraction;
+    int us;
 
-    if (_PyTime_ObjectToTimeval(timestamp, &timet, &us, _PyTime_ROUND_DOWN) == -1)
+    timet = _PyTime_DoubleToTimet(timestamp);
+    if (timet == (time_t)-1 && PyErr_Occurred())
         return NULL;
-    return datetime_from_timet_and_us(cls, f, timet, (int)us, tzinfo);
+    fraction = timestamp - (double)timet;
+    us = (int)_PyTime_RoundHalfEven(fraction * 1e6);
+    if (us < 0) {
+        /* Truncation towards zero is not what we wanted
+           for negative numbers (Python's mod semantics) */
+        timet -= 1;
+        us += 1000000;
+    }
+    /* If timestamp is less than one microsecond smaller than a
+     * full second, round up. Otherwise, ValueErrors are raised
+     * for some floats. */
+    if (us == 1000000) {
+        timet += 1;
+        us = 0;
+    }
+    return datetime_from_timet_and_us(cls, f, timet, us, tzinfo);
 }
 
 /* Internal helper.
@@ -4231,11 +4286,11 @@ static PyObject *
 datetime_fromtimestamp(PyObject *cls, PyObject *args, PyObject *kw)
 {
     PyObject *self;
-    PyObject *timestamp;
+    double timestamp;
     PyObject *tzinfo = Py_None;
     static char *keywords[] = {"timestamp", "tz", NULL};
 
-    if (! PyArg_ParseTupleAndKeywords(args, kw, "O|O:fromtimestamp",
+    if (! PyArg_ParseTupleAndKeywords(args, kw, "d|O:fromtimestamp",
                                       keywords, &timestamp, &tzinfo))
         return NULL;
     if (check_tzinfo_subclass(tzinfo) < 0)
@@ -4259,10 +4314,10 @@ datetime_fromtimestamp(PyObject *cls, PyObject *args, PyObject *kw)
 static PyObject *
 datetime_utcfromtimestamp(PyObject *cls, PyObject *args)
 {
-    PyObject *timestamp;
+    double timestamp;
     PyObject *result = NULL;
 
-    if (PyArg_ParseTuple(args, "O:utcfromtimestamp", &timestamp))
+    if (PyArg_ParseTuple(args, "d:utcfromtimestamp", &timestamp))
         result = datetime_from_timestamp(cls, gmtime, timestamp,
                                          Py_None);
     return result;