gh-152849: Fix OverflowError message for out-of-range float timestamps#152850
gh-152849: Fix OverflowError message for out-of-range float timestamps#152850tonghuaroot wants to merge 4 commits into
Conversation
…estamps
pytime_from_double() converts a float number of seconds to a PyTime_t
(nanoseconds) but reported overflow with pytime_time_t_overflow()
("out of range for platform time_t"), naming a type it never uses. The
integer path already uses pytime_overflow(); switch the float path to it
so both conversion paths report the same PyTime_t overflow.
| def test_FromSecondsObject_float_overflow_message(self): | ||
| # Float path must report a PyTime_t overflow, like the integer path. | ||
| from _testinternalcapi import _PyTime_FromSecondsObject | ||
| for value in (2.0 ** 63, -(2.0 ** 63)): |
There was a problem hiding this comment.
Can you instead use _testcapi.PyTime_MIN and _testcapi.PyTime_MAX please?
There was a problem hiding this comment.
Done, the test now uses float(_testcapi.PyTime_MAX) and float(_testcapi.PyTime_MIN).
There was a problem hiding this comment.
This should be in Library.
There was a problem hiding this comment.
Moved to Library.
| from _testinternalcapi import _PyTime_FromSecondsObject | ||
| for value in (2.0 ** 63, -(2.0 ** 63)): | ||
| for time_rnd, _ in ROUNDING_MODES: | ||
| with self.assertRaisesRegex(OverflowError, "to C PyTime_t"): |
There was a problem hiding this comment.
Please assert the full message.
There was a problem hiding this comment.
Done, it now asserts the full message.
| @@ -636,7 +636,7 @@ pytime_from_double(PyTime_t *tp, double value, _PyTime_round_t round, | |||
|
|
|||
| /* See comments in pytime_double_to_denominator */ | |||
| if (!((double)PyTime_MIN <= d && d < -(double)PyTime_MIN)) { | |||
There was a problem hiding this comment.
The error message of pytime_overflow currently says "too large," but it can also be too small, let's update to:
timestamp out of range for C PyTime_t
There was a problem hiding this comment.
Good point, updated to timestamp out of range for C PyTime_t. This also covers the too-small case.
3 participants
pytime_from_double()inPython/pytime.cconverts a float number ofseconds to a
PyTime_t(nanoseconds). On overflow it calledpytime_time_t_overflow(), which reports"timestamp out of range for platform time_t"— but this path never converts totime_t. The integerpath,
pytime_from_object(), already reports the same conceptual overflowwith
pytime_overflow()("timestamp too large to convert to C PyTime_t").This swaps the float path to
pytime_overflow()so both paths agree and themessage names the type actually involved. The old wording was wrong in fact,
not only in type:
PyTime_tsaturates at roughly 292 years (int64nanoseconds), while 64-bit
time_tspans far longer, so e.g.time.sleep(1e10)(~317 years) overflowsPyTime_tyet is well withintime_trange — the reportedtime_tlimit was never the actual constraint.The correct helper already exists in the same file, so this is a one-line
change that only affects the message text; it targets
mainonly.User-visible via any float-seconds API, e.g.:
Verified in a build of
main: the_PyTime_FromSecondsObject()float path(and
time.sleep()) now raises thePyTime_tmessage for both positive andnegative out-of-range values, matching the integer path; the full
test_timesuite passes. A regression test asserting the float-path overflowmessage is added to
TestCPyTime(fails before the fix, passes after).time_toverflow message for out-of-range float timestamps inpytime_from_double()#152849