From 90f8bf2916a872ecdb046642949db6f8fa30d35a Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 23 Sep 2025 21:53:26 +0300 Subject: [PATCH 1/2] gh-67795: Improve precision of non-float timestamp and timeout arguments Use the as_integer_ratio() or the numerator and denominator attributes to represent the number as an integer ratio and perform scaling and rounding exactly. This allows to avoid the precision loss and double rounding error due to conversion to float. --- Doc/whatsnew/3.15.rst | 3 +- .../pycore_global_objects_fini_generated.h | 2 + Include/internal/pycore_global_strings.h | 2 + .../internal/pycore_runtime_init_generated.h | 2 + .../internal/pycore_unicodeobject_generated.h | 8 + Lib/_pydatetime.py | 24 +- Lib/test/datetimetester.py | 42 ++- Lib/test/test_os.py | 8 +- ...5-09-22-11-30-45.gh-issue-67795.fROoZt.rst | 3 +- Python/pytime.c | 251 +++++++++++++++++- 10 files changed, 322 insertions(+), 23 deletions(-) diff --git a/Doc/whatsnew/3.15.rst b/Doc/whatsnew/3.15.rst index 7b146621dddcfa..78b69af29962b9 100644 --- a/Doc/whatsnew/3.15.rst +++ b/Doc/whatsnew/3.15.rst @@ -281,7 +281,8 @@ Other language changes * Functions that take timestamp or timeout arguments now accept any real numbers (such as :class:`~decimal.Decimal` and :class:`~fractions.Fraction`), - not only integers or floats, although this does not improve precision. + not only integers or floats. + This allows to avoid the precision loss caused by the :class:`float` type. (Contributed by Serhiy Storchaka in :gh:`67795`.) diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index a598af4f37c123..7aaac2687717ca 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -910,6 +910,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(default)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(defaultaction)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(delete)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(denominator)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(depth)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(desired_access)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(detect_types)); @@ -1152,6 +1153,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(nt)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(null)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(number)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(numerator)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(obj)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(object)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(offset)); diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index 6959343947c1f4..647a9e1833cba7 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -401,6 +401,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(default) STRUCT_FOR_ID(defaultaction) STRUCT_FOR_ID(delete) + STRUCT_FOR_ID(denominator) STRUCT_FOR_ID(depth) STRUCT_FOR_ID(desired_access) STRUCT_FOR_ID(detect_types) @@ -643,6 +644,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(nt) STRUCT_FOR_ID(null) STRUCT_FOR_ID(number) + STRUCT_FOR_ID(numerator) STRUCT_FOR_ID(obj) STRUCT_FOR_ID(object) STRUCT_FOR_ID(offset) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index 314837c5b3f288..987ab09a99386b 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -908,6 +908,7 @@ extern "C" { INIT_ID(default), \ INIT_ID(defaultaction), \ INIT_ID(delete), \ + INIT_ID(denominator), \ INIT_ID(depth), \ INIT_ID(desired_access), \ INIT_ID(detect_types), \ @@ -1150,6 +1151,7 @@ extern "C" { INIT_ID(nt), \ INIT_ID(null), \ INIT_ID(number), \ + INIT_ID(numerator), \ INIT_ID(obj), \ INIT_ID(object), \ INIT_ID(offset), \ diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h index 45b00a20a07dda..d84974a7ff8404 100644 --- a/Include/internal/pycore_unicodeobject_generated.h +++ b/Include/internal/pycore_unicodeobject_generated.h @@ -1392,6 +1392,10 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); assert(PyUnicode_GET_LENGTH(string) != 1); + string = &_Py_ID(denominator); + _PyUnicode_InternStatic(interp, &string); + assert(_PyUnicode_CheckConsistency(string, 1)); + assert(PyUnicode_GET_LENGTH(string) != 1); string = &_Py_ID(depth); _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); @@ -2360,6 +2364,10 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); assert(PyUnicode_GET_LENGTH(string) != 1); + string = &_Py_ID(numerator); + _PyUnicode_InternStatic(interp, &string); + assert(_PyUnicode_CheckConsistency(string, 1)); + assert(PyUnicode_GET_LENGTH(string) != 1); string = &_Py_ID(obj); _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); diff --git a/Lib/_pydatetime.py b/Lib/_pydatetime.py index b6d68f2372850a..51ab83948c2a3e 100644 --- a/Lib/_pydatetime.py +++ b/Lib/_pydatetime.py @@ -1857,14 +1857,28 @@ def _fromtimestamp(cls, t, utc, tz): A timezone info object may be passed in as well. """ - frac, t = _math.modf(t) - us = round(frac * 1e6) - if us >= 1000000: + if isinstance(t, float): + frac, t = _math.modf(t) + us = round(frac * 1e6) + else: + try: + try: + n, d = t.as_integer_ratio() + except AttributeError: + n = t.numerator + d = t.denumerator + except AttributeError: + frac, t = _math.modf(t) + us = round(frac * 1e6) + else: + t, n = divmod(n, d) + us = _divide_and_round(n * 1_000_000, d) + if us >= 1_000_000: t += 1 - us -= 1000000 + us -= 1_000_000 elif us < 0: t -= 1 - us += 1000000 + us += 1_000_000 converter = _time.gmtime if utc else _time.localtime y, m, d, hh, mm, ss, weekday, jday, dst = converter(t) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 7df27206206268..f96cade9475807 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -2773,11 +2773,11 @@ def utcfromtimestamp(*args, **kwargs): self.assertEqual(t, zero) t = fts(D('0.000_000_5')) self.assertEqual(t, zero) - t = fts(D('0.000_000_500_000_000_000_000_1')) + t = fts(D('0.000_000_500_000_000_000_000_000_000_000_000_000_001')) self.assertEqual(t, one) t = fts(D('0.000_000_9')) self.assertEqual(t, one) - t = fts(D('0.999_999_499_999_999_9')) + t = fts(D('0.999_999_499_999_999_999_999_999_999_999_999_999_999')) self.assertEqual(t.second, 0) self.assertEqual(t.microsecond, 999_999) t = fts(D('0.999_999_5')) @@ -2790,6 +2790,21 @@ def utcfromtimestamp(*args, **kwargs): self.assertEqual(t.second, 0) self.assertEqual(t.microsecond, 7812) + t = fts(D('2_147_475_000.000_000_5')) + self.assertEqual(t.second, 0) + self.assertEqual(t.microsecond, 0) + t = fts(D('2_147_475_000' + '.000_000_500_000_000_000_000_000_000_000_000_000_001')) + self.assertEqual(t.second, 0) + self.assertEqual(t.microsecond, 1) + t = fts(D('2_147_475_000' + '.999_999_499_999_999_999_999_999_999_999_999_999_999')) + self.assertEqual(t.second, 0) + self.assertEqual(t.microsecond, 999_999) + t = fts(D('2_147_475_000.999_999_5')) + self.assertEqual(t.second, 1) + self.assertEqual(t.microsecond, 0) + @support.run_with_tz('MSK-03') # Something east of Greenwich def test_microsecond_rounding_fraction(self): F = fractions.Fraction @@ -2824,11 +2839,13 @@ def utcfromtimestamp(*args, **kwargs): self.assertEqual(t, zero) t = fts(F(5, 10_000_000)) self.assertEqual(t, zero) - t = fts(F(5_000_000_000, 9_999_999_999_999_999)) + t = fts(F( 5_000_000_000_000_000_000_000_000_000_000_000, + 9_999_999_999_999_999_999_999_999_999_999_999_999_999)) self.assertEqual(t, one) t = fts(F(9, 10_000_000)) self.assertEqual(t, one) - t = fts(F(9_999_995_000_000_000, 10_000_000_000_000_001)) + t = fts(F( 9_999_995_000_000_000_000_000_000_000_000_000_000_000, + 10_000_000_000_000_000_000_000_000_000_000_000_000_001)) self.assertEqual(t.second, 0) self.assertEqual(t.microsecond, 999_999) t = fts(F(9_999_995, 10_000_000)) @@ -2841,6 +2858,23 @@ def utcfromtimestamp(*args, **kwargs): self.assertEqual(t.second, 0) self.assertEqual(t.microsecond, 7812) + t = fts(2_147_475_000 + F(5, 10_000_000)) + self.assertEqual(t.second, 0) + self.assertEqual(t.microsecond, 0) + t = fts(2_147_475_000 + + F( 5_000_000_000_000_000_000_000_000_000_000_000, + 9_999_999_999_999_999_999_999_999_999_999_999_999_999)) + self.assertEqual(t.second, 0) + self.assertEqual(t.microsecond, 1) + t = fts(2_147_475_000 + + F( 9_999_995_000_000_000_000_000_000_000_000_000_000_000, + 10_000_000_000_000_000_000_000_000_000_000_000_000_001)) + self.assertEqual(t.second, 0) + self.assertEqual(t.microsecond, 999_999) + t = fts(2_147_475_000 + F(9_999_995, 10_000_000)) + self.assertEqual(t.second, 1) + self.assertEqual(t.microsecond, 0) + def test_timestamp_limits(self): with self.subTest("minimum UTC"): min_dt = self.theclass.min.replace(tzinfo=timezone.utc) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 1180e27a7a5310..3abe3626014c43 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -951,16 +951,12 @@ def ns_to_sec(ns): @staticmethod def ns_to_sec_decimal(ns): # Convert a number of nanosecond (int) to a number of seconds (Decimal). - # Round towards infinity by adding 0.5 nanosecond to avoid rounding - # issue, os.utime() rounds towards minus infinity. - return decimal.Decimal('1e-9') * ns + decimal.Decimal('0.5e-9') + return decimal.Decimal('1e-9') * ns @staticmethod def ns_to_sec_fraction(ns): # Convert a number of nanosecond (int) to a number of seconds (Fraction). - # Round towards infinity by adding 0.5 nanosecond to avoid rounding - # issue, os.utime() rounds towards minus infinity. - return fractions.Fraction(ns, 10**9) + fractions.Fraction(1, 2*10**9) + return fractions.Fraction(ns, 10**9) def test_utime_by_indexed(self): # pass times as floating-point seconds as the second indexed parameter diff --git a/Misc/NEWS.d/next/Library/2025-09-22-11-30-45.gh-issue-67795.fROoZt.rst b/Misc/NEWS.d/next/Library/2025-09-22-11-30-45.gh-issue-67795.fROoZt.rst index 6c11c93bc1170f..dfbc7ee6654d93 100644 --- a/Misc/NEWS.d/next/Library/2025-09-22-11-30-45.gh-issue-67795.fROoZt.rst +++ b/Misc/NEWS.d/next/Library/2025-09-22-11-30-45.gh-issue-67795.fROoZt.rst @@ -1,3 +1,4 @@ Functions that take timestamp or timeout arguments now accept any real numbers (such as :class:`~decimal.Decimal` and :class:`~fractions.Fraction`), -not only integers or floats, although this does not improve precision. +not only integers or floats. +This allows to avoid the precision loss caused by the :class:`float` type. diff --git a/Python/pytime.c b/Python/pytime.c index 0206467364f894..1c802a229d2f90 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1,5 +1,6 @@ #include "Python.h" #include "pycore_initconfig.h" // _PyStatus_ERR +#include "pycore_long.h" // _PyLong_DivmodNear() #include "pycore_pystate.h" // _Py_AssertHoldsTstate() #include "pycore_runtime.h" // _PyRuntime #include "pycore_time.h" // PyTime_t @@ -362,34 +363,228 @@ pytime_double_to_denominator(double d, time_t *sec, long *numerator, } +static int +maybe_as_integer_ratio(PyObject *number, PyObject **ratio) +{ + *ratio = NULL; + if (PyType_Check(number)) { + PyErr_SetString(PyExc_TypeError, + "required a number, not type"); + return -1; + } + PyObject *meth; + if (PyObject_GetOptionalAttr(number, &_Py_ID(as_integer_ratio), &meth) < 0) { + return -1; + } + if (meth) { + *ratio = PyObject_CallNoArgs(meth); + Py_DECREF(meth); + if (*ratio == NULL) { + return -1; + } + if (!PyTuple_Check(*ratio)) { + PyErr_Format(PyExc_TypeError, + "unexpected return type from %T.as_integer_ratio(): " + "expected tuple, not '%T'", + number, *ratio); + Py_CLEAR(*ratio); + return -1; + } + if (PyTuple_GET_SIZE(*ratio) != 2) { + PyErr_Format(PyExc_ValueError, + "%T.as_integer_ratio() must return a 2-tuple", + number); + Py_CLEAR(*ratio); + return -1; + } + return 1; + } + + PyObject *numerator, *denominator; + int rc = PyObject_GetOptionalAttr(number, &_Py_ID(numerator), &numerator); + if (rc <= 0) { + return rc; + } + rc = PyObject_GetOptionalAttr(number, &_Py_ID(denominator), &denominator); + if (rc <= 0) { + Py_DECREF(numerator); + return rc; + } + *ratio = PyTuple_Pack(2, numerator, denominator); + Py_DECREF(numerator); + Py_DECREF(denominator); + return *ratio ? 1 : -1; +} + +static PyObject * +checked_divmod(PyObject *a, PyObject *b) +{ + PyObject *result = PyNumber_Divmod(a, b); + if (result != NULL) { + if (!PyTuple_Check(result)) { + PyErr_Format(PyExc_TypeError, + "divmod() returned non-tuple (type %T)", + result); + Py_DECREF(result); + return NULL; + } + if (PyTuple_GET_SIZE(result) != 2) { + PyErr_Format(PyExc_TypeError, + "divmod() returned a tuple of size %zd", + PyTuple_GET_SIZE(result)); + Py_DECREF(result); + return NULL; + } + } + return result; +} + +static PyObject * +divide_and_round(PyObject *numerator, PyObject *denominator, _PyTime_round_t round) +{ + PyObject *divmod, *result; + if (round == _PyTime_ROUND_UP) { + int isneg = PyObject_RichCompareBool(numerator, _PyLong_GetZero(), Py_LT); + if (isneg < 0) { + return NULL; + } + round = isneg ? _PyTime_ROUND_FLOOR : _PyTime_ROUND_CEILING; + } + if (round == _PyTime_ROUND_FLOOR) { + return PyNumber_FloorDivide(numerator, denominator); + } + else if (round == _PyTime_ROUND_CEILING) { + divmod = checked_divmod(numerator, denominator); + if (divmod == NULL) { + return NULL; + } + int nonzero = PyObject_IsTrue(PyTuple_GET_ITEM(divmod, 1)); + if (nonzero < 0) { + result = NULL; + } + else if (nonzero) { + result = PyNumber_Add(PyTuple_GET_ITEM(divmod, 0), _PyLong_GetOne()); + } + else { + result = Py_NewRef(PyTuple_GET_ITEM(divmod, 0)); + } + } + else { + assert(round == _PyTime_ROUND_HALF_EVEN); + divmod = _PyLong_DivmodNear(numerator, denominator); + if (divmod == NULL) { + return NULL; + } + result = Py_NewRef(PyTuple_GET_ITEM(divmod, 0)); + } + Py_DECREF(divmod); + return result; +} + +static PyObject * +multiply_divide_and_round(long scale, PyObject *numerator, PyObject *denominator, + _PyTime_round_t round) +{ + PyObject *scaleobj = PyLong_FromLong(scale); + if (scaleobj == NULL) { + return NULL; + } + numerator = PyNumber_Multiply(numerator, scaleobj); + Py_DECREF(scaleobj); + if (numerator == NULL) { + return NULL; + } + PyObject *result = divide_and_round(numerator, denominator, round); + Py_DECREF(numerator); + return result; +} + +static int +pytime_ratio_to_denominator(PyObject *numerator, PyObject *denominator, + time_t *sec, long *subsec, + long scale, _PyTime_round_t round) +{ + PyObject *divmod = checked_divmod(numerator, denominator); + if (divmod == NULL) { + return -1; + } + *sec = _PyLong_AsTime_t(PyTuple_GET_ITEM(divmod, 0)); + if (*sec == (time_t)-1 && PyErr_Occurred()) { + Py_DECREF(divmod); + return -1; + } + PyObject *tmp = multiply_divide_and_round(scale, + PyTuple_GET_ITEM(divmod, 1), + denominator, + _PyTime_ROUND_HALF_EVEN); + Py_DECREF(divmod); + *subsec = PyLong_AsLong(tmp); + Py_DECREF(tmp); + if (*subsec == -1 && PyErr_Occurred()) { + return -1; + } + if (*subsec < 0) { + *subsec += scale; + if (*sec <= PY_TIME_T_MIN) { + pytime_time_t_overflow(); + return -1; + } + *sec -= 1; + } + else if (*subsec >= scale) { + *subsec -= scale; + if (*sec >= PY_TIME_T_MAX) { + pytime_time_t_overflow(); + return -1; + } + *sec += 1; + } + return 0; +} + + static int pytime_object_to_denominator(PyObject *obj, time_t *sec, long *numerator, long denominator, _PyTime_round_t round) { assert(denominator >= 1); + *numerator = 0; if (PyIndex_Check(obj)) { *sec = _PyLong_AsTime_t(obj); - *numerator = 0; if (*sec == (time_t)-1 && PyErr_Occurred()) { return -1; } return 0; } - else { + else if (PyFloat_Check(obj)) { +fromfloat:; double d = PyFloat_AsDouble(obj); if (d == -1 && PyErr_Occurred()) { - *numerator = 0; return -1; } if (isnan(d)) { - *numerator = 0; PyErr_SetString(PyExc_ValueError, "Invalid value NaN (not a number)"); return -1; } return pytime_double_to_denominator(d, sec, numerator, denominator, round); } + else { + PyObject *ratio; + if (maybe_as_integer_ratio(obj, &ratio) < 0) { + return -1; + } + if (ratio == NULL) { + goto fromfloat; + } + int rc = pytime_ratio_to_denominator(PyTuple_GET_ITEM(ratio, 0), + PyTuple_GET_ITEM(ratio, 1), + sec, numerator, + denominator, round); + Py_DECREF(ratio); + return rc; + } } @@ -403,7 +598,8 @@ _PyTime_ObjectToTime_t(PyObject *obj, time_t *sec, _PyTime_round_t round) } return 0; } - else { + else if (PyFloat_Check(obj)) { +fromfloat:; double intpart; /* volatile avoids optimization changing how numbers are rounded */ volatile double d; @@ -428,6 +624,28 @@ _PyTime_ObjectToTime_t(PyObject *obj, time_t *sec, _PyTime_round_t round) *sec = (time_t)intpart; return 0; } + else { + PyObject *ratio; + if (maybe_as_integer_ratio(obj, &ratio) < 0) { + return -1; + } + if (ratio == NULL) { + goto fromfloat; + } + PyObject *secobj = divide_and_round(PyTuple_GET_ITEM(ratio, 0), + PyTuple_GET_ITEM(ratio, 1), + round); + Py_DECREF(ratio); + if (secobj == NULL) { + return -1; + } + *sec = _PyLong_AsTime_t(secobj); + Py_DECREF(secobj); + if (*sec == (time_t)-1 && PyErr_Occurred()) { + return -1; + } + return 0; + } } @@ -609,7 +827,8 @@ pytime_from_object(PyTime_t *tp, PyObject *obj, _PyTime_round_t round, *tp = ns; return 0; } - else { + else if (PyFloat_Check(obj)) { +fromfloat:; double d; d = PyFloat_AsDouble(obj); if (d == -1 && PyErr_Occurred()) { @@ -621,6 +840,26 @@ pytime_from_object(PyTime_t *tp, PyObject *obj, _PyTime_round_t round, } return pytime_from_double(tp, d, round, unit_to_ns); } + else { + PyObject *ratio; + if (maybe_as_integer_ratio(obj, &ratio) < 0) { + return -1; + } + if (ratio == NULL) { + goto fromfloat; + } + PyObject *nsobj = multiply_divide_and_round(unit_to_ns, + PyTuple_GET_ITEM(ratio, 0), + PyTuple_GET_ITEM(ratio, 1), + round); + Py_DECREF(ratio); + if (nsobj == NULL) { + return -1; + } + int rc = _PyTime_FromLong(tp, nsobj); + Py_DECREF(nsobj); + return rc; + } } From 68433af3a29b75d6ecef5c1cf1ce838fe6250f14 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 3 Oct 2025 17:58:41 +0300 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Victor Stinner --- Python/pytime.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Python/pytime.c b/Python/pytime.c index 1c802a229d2f90..eb9a17716a171e 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -442,7 +442,6 @@ checked_divmod(PyObject *a, PyObject *b) static PyObject * divide_and_round(PyObject *numerator, PyObject *denominator, _PyTime_round_t round) { - PyObject *divmod, *result; if (round == _PyTime_ROUND_UP) { int isneg = PyObject_RichCompareBool(numerator, _PyLong_GetZero(), Py_LT); if (isneg < 0) { @@ -450,10 +449,13 @@ divide_and_round(PyObject *numerator, PyObject *denominator, _PyTime_round_t rou } round = isneg ? _PyTime_ROUND_FLOOR : _PyTime_ROUND_CEILING; } + if (round == _PyTime_ROUND_FLOOR) { return PyNumber_FloorDivide(numerator, denominator); } - else if (round == _PyTime_ROUND_CEILING) { + + PyObject *divmod, *result; + if (round == _PyTime_ROUND_CEILING) { divmod = checked_divmod(numerator, denominator); if (divmod == NULL) { return NULL;