naturaltime and precisedelta: return non-finite floats unchanged#348
Open
HrachShah wants to merge 2 commits into
Open
naturaltime and precisedelta: return non-finite floats unchanged#348HrachShah wants to merge 2 commits into
HrachShah wants to merge 2 commits into
Conversation
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
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
1 participant
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.
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 catches 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 #342 -- this PR 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.