From 992c43fec900e204deffc026318b97ab0f83eff6 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 27 Mar 2015 17:12:45 +0100 Subject: [PATCH] Issue #22117: Fix rounding in _PyTime_FromSecondsObject() * Rename _PyTime_FromObject() to _PyTime_FromSecondsObject() * Add _PyTime_AsNanosecondsObject() and _testcapi.pytime_fromsecondsobject() * Add unit tests --- Include/pytime.h | 9 +- Lib/test/test_time.py | 252 ++++++++++++++++++++++++++------------ Modules/_testcapimodule.c | 17 +++ Modules/timemodule.c | 2 +- Python/pytime.c | 17 ++- 5 files changed, 211 insertions(+), 86 deletions(-) diff --git a/Include/pytime.h b/Include/pytime.h index 9cf1170d70b..78e4ae900fe 100644 --- a/Include/pytime.h +++ b/Include/pytime.h @@ -121,15 +121,18 @@ typedef PY_INT64_T _PyTime_t; /* Convert a Python float or int to a timetamp. Raise an exception and return -1 on error, return 0 on success. */ -PyAPI_FUNC(int) _PyTime_FromObject(_PyTime_t *t, +PyAPI_FUNC(int) _PyTime_FromSecondsObject(_PyTime_t *t, PyObject *obj, _PyTime_round_t round); /* Convert timestamp to a number of milliseconds (10^-3 seconds). */ -PyAPI_FUNC(_PyTime_t) -_PyTime_AsMilliseconds(_PyTime_t t, +PyAPI_FUNC(_PyTime_t) _PyTime_AsMilliseconds(_PyTime_t t, _PyTime_round_t round); +/* Convert timestamp to a number of nanoseconds (10^-9 seconds) as a Python int + object. */ +PyAPI_FUNC(PyObject *) _PyTime_AsNanosecondsObject(_PyTime_t t); + /* Convert a timestamp to a timeval structure. */ PyAPI_FUNC(int) _PyTime_AsTimeval(_PyTime_t t, struct timeval *tv, diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py index be7ddcc34d6..1bf0d09bd85 100644 --- a/Lib/test/test_time.py +++ b/Lib/test/test_time.py @@ -1,10 +1,11 @@ from test import support +import enum +import locale +import platform +import sys +import sysconfig import time import unittest -import locale -import sysconfig -import sys -import platform try: import threading except ImportError: @@ -14,8 +15,15 @@ except ImportError: SIZEOF_INT = sysconfig.get_config_var('SIZEOF_INT') or 4 TIME_MAXYEAR = (1 << 8 * SIZEOF_INT - 1) - 1 TIME_MINYEAR = -TIME_MAXYEAR - 1 -_PyTime_ROUND_DOWN = 0 -_PyTime_ROUND_UP = 1 + + +class _PyTime(enum.IntEnum): + # Round towards zero + ROUND_DOWN = 0 + # Round away from zero + ROUND_UP = 1 + +ALL_ROUNDING_METHODS = (_PyTime.ROUND_UP, _PyTime.ROUND_DOWN) class TimeTestCase(unittest.TestCase): @@ -596,23 +604,23 @@ class TestPytime(unittest.TestCase): from _testcapi import pytime_object_to_time_t for obj, time_t, rnd in ( # Round towards zero - (0, 0, _PyTime_ROUND_DOWN), - (-1, -1, _PyTime_ROUND_DOWN), - (-1.0, -1, _PyTime_ROUND_DOWN), - (-1.9, -1, _PyTime_ROUND_DOWN), - (1.0, 1, _PyTime_ROUND_DOWN), - (1.9, 1, _PyTime_ROUND_DOWN), + (0, 0, _PyTime.ROUND_DOWN), + (-1, -1, _PyTime.ROUND_DOWN), + (-1.0, -1, _PyTime.ROUND_DOWN), + (-1.9, -1, _PyTime.ROUND_DOWN), + (1.0, 1, _PyTime.ROUND_DOWN), + (1.9, 1, _PyTime.ROUND_DOWN), # Round away from zero - (0, 0, _PyTime_ROUND_UP), - (-1, -1, _PyTime_ROUND_UP), - (-1.0, -1, _PyTime_ROUND_UP), - (-1.9, -2, _PyTime_ROUND_UP), - (1.0, 1, _PyTime_ROUND_UP), - (1.9, 2, _PyTime_ROUND_UP), + (0, 0, _PyTime.ROUND_UP), + (-1, -1, _PyTime.ROUND_UP), + (-1.0, -1, _PyTime.ROUND_UP), + (-1.9, -2, _PyTime.ROUND_UP), + (1.0, 1, _PyTime.ROUND_UP), + (1.9, 2, _PyTime.ROUND_UP), ): self.assertEqual(pytime_object_to_time_t(obj, rnd), time_t) - rnd = _PyTime_ROUND_DOWN + rnd = _PyTime.ROUND_DOWN for invalid in self.invalid_values: self.assertRaises(OverflowError, pytime_object_to_time_t, invalid, rnd) @@ -622,44 +630,44 @@ class TestPytime(unittest.TestCase): from _testcapi import pytime_object_to_timeval for obj, timeval, rnd in ( # Round towards zero - (0, (0, 0), _PyTime_ROUND_DOWN), - (-1, (-1, 0), _PyTime_ROUND_DOWN), - (-1.0, (-1, 0), _PyTime_ROUND_DOWN), - (1e-6, (0, 1), _PyTime_ROUND_DOWN), - (1e-7, (0, 0), _PyTime_ROUND_DOWN), - (-1e-6, (-1, 999999), _PyTime_ROUND_DOWN), - (-1e-7, (-1, 999999), _PyTime_ROUND_DOWN), - (-1.2, (-2, 800000), _PyTime_ROUND_DOWN), - (0.9999999, (0, 999999), _PyTime_ROUND_DOWN), - (0.0000041, (0, 4), _PyTime_ROUND_DOWN), - (1.1234560, (1, 123456), _PyTime_ROUND_DOWN), - (1.1234569, (1, 123456), _PyTime_ROUND_DOWN), - (-0.0000040, (-1, 999996), _PyTime_ROUND_DOWN), - (-0.0000041, (-1, 999995), _PyTime_ROUND_DOWN), - (-1.1234560, (-2, 876544), _PyTime_ROUND_DOWN), - (-1.1234561, (-2, 876543), _PyTime_ROUND_DOWN), + (0, (0, 0), _PyTime.ROUND_DOWN), + (-1, (-1, 0), _PyTime.ROUND_DOWN), + (-1.0, (-1, 0), _PyTime.ROUND_DOWN), + (1e-6, (0, 1), _PyTime.ROUND_DOWN), + (1e-7, (0, 0), _PyTime.ROUND_DOWN), + (-1e-6, (-1, 999999), _PyTime.ROUND_DOWN), + (-1e-7, (-1, 999999), _PyTime.ROUND_DOWN), + (-1.2, (-2, 800000), _PyTime.ROUND_DOWN), + (0.9999999, (0, 999999), _PyTime.ROUND_DOWN), + (0.0000041, (0, 4), _PyTime.ROUND_DOWN), + (1.1234560, (1, 123456), _PyTime.ROUND_DOWN), + (1.1234569, (1, 123456), _PyTime.ROUND_DOWN), + (-0.0000040, (-1, 999996), _PyTime.ROUND_DOWN), + (-0.0000041, (-1, 999995), _PyTime.ROUND_DOWN), + (-1.1234560, (-2, 876544), _PyTime.ROUND_DOWN), + (-1.1234561, (-2, 876543), _PyTime.ROUND_DOWN), # Round away from zero - (0, (0, 0), _PyTime_ROUND_UP), - (-1, (-1, 0), _PyTime_ROUND_UP), - (-1.0, (-1, 0), _PyTime_ROUND_UP), - (1e-6, (0, 1), _PyTime_ROUND_UP), - (1e-7, (0, 1), _PyTime_ROUND_UP), - (-1e-6, (-1, 999999), _PyTime_ROUND_UP), - (-1e-7, (-1, 999999), _PyTime_ROUND_UP), - (-1.2, (-2, 800000), _PyTime_ROUND_UP), - (0.9999999, (1, 0), _PyTime_ROUND_UP), - (0.0000041, (0, 5), _PyTime_ROUND_UP), - (1.1234560, (1, 123457), _PyTime_ROUND_UP), - (1.1234569, (1, 123457), _PyTime_ROUND_UP), - (-0.0000040, (-1, 999996), _PyTime_ROUND_UP), - (-0.0000041, (-1, 999995), _PyTime_ROUND_UP), - (-1.1234560, (-2, 876544), _PyTime_ROUND_UP), - (-1.1234561, (-2, 876543), _PyTime_ROUND_UP), + (0, (0, 0), _PyTime.ROUND_UP), + (-1, (-1, 0), _PyTime.ROUND_UP), + (-1.0, (-1, 0), _PyTime.ROUND_UP), + (1e-6, (0, 1), _PyTime.ROUND_UP), + (1e-7, (0, 1), _PyTime.ROUND_UP), + (-1e-6, (-1, 999999), _PyTime.ROUND_UP), + (-1e-7, (-1, 999999), _PyTime.ROUND_UP), + (-1.2, (-2, 800000), _PyTime.ROUND_UP), + (0.9999999, (1, 0), _PyTime.ROUND_UP), + (0.0000041, (0, 5), _PyTime.ROUND_UP), + (1.1234560, (1, 123457), _PyTime.ROUND_UP), + (1.1234569, (1, 123457), _PyTime.ROUND_UP), + (-0.0000040, (-1, 999996), _PyTime.ROUND_UP), + (-0.0000041, (-1, 999995), _PyTime.ROUND_UP), + (-1.1234560, (-2, 876544), _PyTime.ROUND_UP), + (-1.1234561, (-2, 876543), _PyTime.ROUND_UP), ): with self.subTest(obj=obj, round=rnd, timeval=timeval): self.assertEqual(pytime_object_to_timeval(obj, rnd), timeval) - rnd = _PyTime_ROUND_DOWN + rnd = _PyTime.ROUND_DOWN for invalid in self.invalid_values: self.assertRaises(OverflowError, pytime_object_to_timeval, invalid, rnd) @@ -669,38 +677,38 @@ class TestPytime(unittest.TestCase): from _testcapi import pytime_object_to_timespec for obj, timespec, rnd in ( # Round towards zero - (0, (0, 0), _PyTime_ROUND_DOWN), - (-1, (-1, 0), _PyTime_ROUND_DOWN), - (-1.0, (-1, 0), _PyTime_ROUND_DOWN), - (1e-9, (0, 1), _PyTime_ROUND_DOWN), - (1e-10, (0, 0), _PyTime_ROUND_DOWN), - (-1e-9, (-1, 999999999), _PyTime_ROUND_DOWN), - (-1e-10, (-1, 999999999), _PyTime_ROUND_DOWN), - (-1.2, (-2, 800000000), _PyTime_ROUND_DOWN), - (0.9999999999, (0, 999999999), _PyTime_ROUND_DOWN), - (1.1234567890, (1, 123456789), _PyTime_ROUND_DOWN), - (1.1234567899, (1, 123456789), _PyTime_ROUND_DOWN), - (-1.1234567890, (-2, 876543211), _PyTime_ROUND_DOWN), - (-1.1234567891, (-2, 876543210), _PyTime_ROUND_DOWN), + (0, (0, 0), _PyTime.ROUND_DOWN), + (-1, (-1, 0), _PyTime.ROUND_DOWN), + (-1.0, (-1, 0), _PyTime.ROUND_DOWN), + (1e-9, (0, 1), _PyTime.ROUND_DOWN), + (1e-10, (0, 0), _PyTime.ROUND_DOWN), + (-1e-9, (-1, 999999999), _PyTime.ROUND_DOWN), + (-1e-10, (-1, 999999999), _PyTime.ROUND_DOWN), + (-1.2, (-2, 800000000), _PyTime.ROUND_DOWN), + (0.9999999999, (0, 999999999), _PyTime.ROUND_DOWN), + (1.1234567890, (1, 123456789), _PyTime.ROUND_DOWN), + (1.1234567899, (1, 123456789), _PyTime.ROUND_DOWN), + (-1.1234567890, (-2, 876543211), _PyTime.ROUND_DOWN), + (-1.1234567891, (-2, 876543210), _PyTime.ROUND_DOWN), # Round away from zero - (0, (0, 0), _PyTime_ROUND_UP), - (-1, (-1, 0), _PyTime_ROUND_UP), - (-1.0, (-1, 0), _PyTime_ROUND_UP), - (1e-9, (0, 1), _PyTime_ROUND_UP), - (1e-10, (0, 1), _PyTime_ROUND_UP), - (-1e-9, (-1, 999999999), _PyTime_ROUND_UP), - (-1e-10, (-1, 999999999), _PyTime_ROUND_UP), - (-1.2, (-2, 800000000), _PyTime_ROUND_UP), - (0.9999999999, (1, 0), _PyTime_ROUND_UP), - (1.1234567890, (1, 123456790), _PyTime_ROUND_UP), - (1.1234567899, (1, 123456790), _PyTime_ROUND_UP), - (-1.1234567890, (-2, 876543211), _PyTime_ROUND_UP), - (-1.1234567891, (-2, 876543210), _PyTime_ROUND_UP), + (0, (0, 0), _PyTime.ROUND_UP), + (-1, (-1, 0), _PyTime.ROUND_UP), + (-1.0, (-1, 0), _PyTime.ROUND_UP), + (1e-9, (0, 1), _PyTime.ROUND_UP), + (1e-10, (0, 1), _PyTime.ROUND_UP), + (-1e-9, (-1, 999999999), _PyTime.ROUND_UP), + (-1e-10, (-1, 999999999), _PyTime.ROUND_UP), + (-1.2, (-2, 800000000), _PyTime.ROUND_UP), + (0.9999999999, (1, 0), _PyTime.ROUND_UP), + (1.1234567890, (1, 123456790), _PyTime.ROUND_UP), + (1.1234567899, (1, 123456790), _PyTime.ROUND_UP), + (-1.1234567890, (-2, 876543211), _PyTime.ROUND_UP), + (-1.1234567891, (-2, 876543210), _PyTime.ROUND_UP), ): with self.subTest(obj=obj, round=rnd, timespec=timespec): self.assertEqual(pytime_object_to_timespec(obj, rnd), timespec) - rnd = _PyTime_ROUND_DOWN + rnd = _PyTime.ROUND_DOWN for invalid in self.invalid_values: self.assertRaises(OverflowError, pytime_object_to_timespec, invalid, rnd) @@ -759,5 +767,91 @@ class TestPytime(unittest.TestCase): self.assertIs(lt.tm_zone, None) +@support.cpython_only +class TestPyTime_t(unittest.TestCase): + def test_FromSecondsObject(self): + from _testcapi import pytime_fromsecondsobject + SEC_TO_NS = 10 ** 9 + MAX_SEC = 2 ** 63 // 10 ** 9 + + # Conversion giving the same result for all rounding methods + for rnd in ALL_ROUNDING_METHODS: + for obj, ts in ( + # integers + (0, 0), + (1, SEC_TO_NS), + (-3, -3 * SEC_TO_NS), + + # float: subseconds + (0.0, 0), + (1e-9, 1), + (1e-6, 10 ** 3), + (1e-3, 10 ** 6), + + # float: seconds + (2.0, 2 * SEC_TO_NS), + (123.0, 123 * SEC_TO_NS), + (-7.0, -7 * SEC_TO_NS), + + # nanosecond are kept for value <= 2^23 seconds + (2**22 - 1e-9, 4194303999999999), + (2**22, 4194304000000000), + (2**22 + 1e-9, 4194304000000001), + (2**23 - 1e-9, 8388607999999999), + (2**23, 8388608000000000), + + # start loosing precision for value > 2^23 seconds + (2**23 + 1e-9, 8388608000000002), + + # nanoseconds are lost for value > 2^23 seconds + (2**24 - 1e-9, 16777215999999998), + (2**24, 16777216000000000), + (2**24 + 1e-9, 16777216000000000), + (2**25 - 1e-9, 33554432000000000), + (2**25 , 33554432000000000), + (2**25 + 1e-9, 33554432000000000), + + # close to 2^63 nanoseconds + (9223372036, 9223372036 * SEC_TO_NS), + (9223372036.0, 9223372036 * SEC_TO_NS), + (-9223372036, -9223372036 * SEC_TO_NS), + (-9223372036.0, -9223372036 * SEC_TO_NS), + ): + with self.subTest(obj=obj, round=rnd, timestamp=ts): + self.assertEqual(pytime_fromsecondsobject(obj, rnd), ts) + + with self.subTest(round=rnd): + with self.assertRaises(OverflowError): + pytime_fromsecondsobject(9223372037, rnd) + pytime_fromsecondsobject(9223372037.0, rnd) + pytime_fromsecondsobject(-9223372037, rnd) + pytime_fromsecondsobject(-9223372037.0, rnd) + + # Conversion giving different results depending on the rounding method + UP = _PyTime.ROUND_UP + DOWN = _PyTime.ROUND_DOWN + for obj, ts, rnd in ( + # close to zero + ( 1e-10, 1, UP), + ( 1e-10, 0, DOWN), + (-1e-10, 0, DOWN), + (-1e-10, -1, UP), + + # test rounding of the last nanosecond + ( 1.1234567899, 1123456790, UP), + ( 1.1234567899, 1123456789, DOWN), + (-1.1234567899, -1123456789, DOWN), + (-1.1234567899, -1123456790, UP), + + # close to 1 second + ( 0.9999999999, 1000000000, UP), + ( 0.9999999999, 999999999, DOWN), + (-0.9999999999, -999999999, DOWN), + (-0.9999999999, -1000000000, UP), + ): + with self.subTest(obj=obj, round=rnd, timestamp=ts): + self.assertEqual(pytime_fromsecondsobject(obj, rnd), ts) + + if __name__ == "__main__": unittest.main() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index b8e1dbc4f9d..ec513bca359 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3378,6 +3378,22 @@ return_result_with_error(PyObject *self, PyObject *args) Py_RETURN_NONE; } +static PyObject * +test_pytime_fromsecondsobject(PyObject *self, PyObject *args) +{ + PyObject *obj; + int round; + _PyTime_t ts; + + if (!PyArg_ParseTuple(args, "Oi", &obj, &round)) + return NULL; + if (check_time_rounding(round) < 0) + return NULL; + if (_PyTime_FromSecondsObject(&ts, obj, round) == -1) + return NULL; + return _PyTime_AsNanosecondsObject(ts); +} + static PyMethodDef TestMethods[] = { {"raise_exception", raise_exception, METH_VARARGS}, @@ -3541,6 +3557,7 @@ static PyMethodDef TestMethods[] = { return_null_without_error, METH_NOARGS}, {"return_result_with_error", return_result_with_error, METH_NOARGS}, + {"pytime_fromsecondsobject", test_pytime_fromsecondsobject, METH_VARARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Modules/timemodule.c b/Modules/timemodule.c index e44e082c262..1ce2f6a0edb 100644 --- a/Modules/timemodule.c +++ b/Modules/timemodule.c @@ -221,7 +221,7 @@ static PyObject * time_sleep(PyObject *self, PyObject *obj) { _PyTime_t secs; - if (_PyTime_FromObject(&secs, obj, _PyTime_ROUND_UP)) + if (_PyTime_FromSecondsObject(&secs, obj, _PyTime_ROUND_UP)) return NULL; if (secs < 0) { PyErr_SetString(PyExc_ValueError, diff --git a/Python/pytime.c b/Python/pytime.c index 6bf7030cbaf..2aeeddc9436 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -424,7 +424,7 @@ _PyTime_FromTimespec(_PyTime_t *tp, struct timespec *ts) #endif int -_PyTime_FromObject(_PyTime_t *t, PyObject *obj, _PyTime_round_t round) +_PyTime_FromSecondsObject(_PyTime_t *t, PyObject *obj, _PyTime_round_t round) { if (PyFloat_Check(obj)) { double d, err; @@ -433,8 +433,7 @@ _PyTime_FromObject(_PyTime_t *t, PyObject *obj, _PyTime_round_t round) d = PyFloat_AsDouble(obj); d *= 1e9; - /* FIXME: use sign */ - if (round == _PyTime_ROUND_UP) + if ((round == _PyTime_ROUND_UP) ^ (d < 0)) d = ceil(d); else d = floor(d); @@ -471,6 +470,18 @@ _PyTime_FromObject(_PyTime_t *t, PyObject *obj, _PyTime_round_t round) } } +PyObject * +_PyTime_AsNanosecondsObject(_PyTime_t t) +{ +#ifdef HAVE_LONG_LONG + assert(sizeof(PY_LONG_LONG) >= sizeof(_PyTime_t)); + return PyLong_FromLongLong((PY_LONG_LONG)t); +#else + assert(sizeof(long) >= sizeof(_PyTime_t)); + return PyLong_FromLong((long)t); +#endif +} + static _PyTime_t _PyTime_Multiply(_PyTime_t t, unsigned int multiply, _PyTime_round_t round) {