naturaltime and precisedelta: return non-finite floats unchanged instead of raising OverflowError - #344
Closed
HrachShah wants to merge 2 commits into
Closed
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
naturaltime and precisedelta both routed their float input through
_date_and_delta, which fed it straight to round() (or skipped round
in precise mode) and then dt.timedelta(seconds=...). For a non-finite
float like inf or -inf, round(float("inf")) raises OverflowError
and dt.timedelta(seconds=float("inf")) raises OverflowError too.
NaN happened to work because round(float("nan")) raises ValueError
which the except clause caught, but inf/-inf slipped past.
The naturaldelta path caught inf/-inf via a different probe (it
calls int(value) first, which raises OverflowError for inf) and
that case is fixed in the still-open PR python-humanize#342 -- this commit leaves
that probe alone and adds the equivalent isfinite guard inside
_date_and_delta, which is what both naturaltime and precisedelta
go through. Adding OverflowError to the except tuple covers the
last remaining case where a finite but oversized number
(> ~3e9 days in seconds) would otherwise crash.
Three new parametrized test functions in tests/test_time.py cover
naturaltime, precisedelta, and the internal _date_and_delta
contract for inf/-inf/nan. The inf and -inf cases fail on the
pre-fix code with OverflowError; the nan case already worked and
is included as a regression guard. Full suite: 724 passed,
69 skipped.
for more information, see https://pre-commit.ci
Copy link
Copy Markdown
Member
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both naturaltime() and precisedelta() feed a float input through _date_and_delta(), which passes it straight to round() (or skips rounding when precise=True). For float('inf') and float('-inf'), round() raises OverflowError: cannot convert float infinity to integer and dt.timedelta(seconds=float('inf')) raises the same. The except clause in the function catches ValueError, TypeError, but not OverflowError, so the user gets a traceback rather than the documented "return str(value) unchanged" behavior.
naturaldelta() already had the same bug, fixed in #342 by widening the except to include OverflowError. naturaltime() and precisedelta() still crash because they take a different path (round() instead of int()). This PR mirrors #342's fix for the two other public functions, by guarding the float-input branch with an explicit math.isfinite check before round()/timedelta() and by widening the except to (ValueError, TypeError, OverflowError) so a huge but finite value (e.g. 1e100) falls through the same way.
precise=True (used by precisedelta()) skips the round() call but still constructs dt.timedelta(seconds=value), which raises OverflowError for non-finite floats. The isfinite guard handles both paths.
Verified: