From 7b055c81167c48c7ca553ce7cd1af7bf0b88d29a Mon Sep 17 00:00:00 2001 From: dualfroz Date: Fri, 4 Sep 2026 09:22:40 +0200 Subject: [PATCH] fix: round decimal unit values instead of truncating in integer path Multiplying a decimal quantity by its unit multiplier in binary floating point can land just below the exact integer (float('2.05') * 60 == 122.999...), so int() truncated 2.05m to 122 seconds instead of 123. Round the sub-second unit contribution before adding whole seconds. --- PR.md | 74 ++++++++++++++++++++++++++++++ pytimeparse/tests/testtimeparse.py | 10 ++++ pytimeparse/timeparse.py | 10 ++-- 3 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 PR.md diff --git a/PR.md b/PR.md new file mode 100644 index 0000000..e641706 --- /dev/null +++ b/PR.md @@ -0,0 +1,74 @@ +# Fix off-by-one from float truncation in the integer result path + +## Problem + +`timeparse` returns a value one second too small for many decimal quantities +that are in fact an exact whole number of seconds: + +```python +>>> from pytimeparse import parse +>>> parse('2.05m') +122 # 2.05 minutes == 123 seconds +>>> parse('4.1m') +245 # == 246 seconds +>>> parse('1.13h') +4067 # == 4068 seconds +``` + +`parse('1.2 m')` already documents that decimal quantities should yield exact +integers (it returns `72`). The results above break that contract. A scan of +decimal quantities of minutes/hours/days/weeks whose exact value is an integral +number of seconds found 365 such wrong results in the range x.01 .. x.99. + +## Root cause + +`pytimeparse/timeparse.py`, in the branch that returns an integer when the +seconds field is whole or absent: + +```python +return ( + sign * int(sum([MULTIPLIERS[k] * float(v) for (k, v) in + list(mdict.items()) if k != 'secs' and v is not None])) + + (int(mdict['secs'], 10) if mdict['secs'] else 0)) +``` + +Multiplying a decimal quantity by its unit multiplier is done in binary +floating point, which for many values lands just below the exact integer: +`float('2.05') * 60 == 122.99999999999999`. `int(...)` then truncates toward +zero, dropping the last second (123 -> 122). + +## Fix + +Round the sub-second unit contribution to the nearest integer before adding the +whole-seconds field: + +```python +sign * int(round(sum([...]))) + (int(mdict['secs'], 10) if mdict['secs'] else 0) +``` + +`round` collapses the `122.999...`/`123.000...1` float artifacts back to the +exact integer. The whole-seconds field is still added exactly, and the float +result path (when the seconds field itself is fractional) is unchanged. + +## Test + +Added `test_timeparse_fractional_rounding` to +`pytimeparse/tests/testtimeparse.py`: + +```python +self.assertEqual(timeparse.timeparse('2.05m'), 123) +self.assertEqual(timeparse.timeparse('4.1m'), 246) +self.assertEqual(timeparse.timeparse('1.13h'), 4068) +self.assertEqual(timeparse.timeparse('8.03h'), 28908) +``` + +## Gate + +- `python -m pytest pytimeparse/tests/testtimeparse.py` -> 50 passed + (49 pre-existing, including the exact-integer cases `1.24 days`, `5.6 wk`, + plus the new test). +- Re-ran the exhaustive x.01 .. x.99 scan for minutes/hours/days/weeks: + 0 remaining truncation mismatches (was 365). + +Counterfactual (fix reverted, test kept): fails with +`AssertionError: 122 != 123`. diff --git a/pytimeparse/tests/testtimeparse.py b/pytimeparse/tests/testtimeparse.py index c305bb5..760057c 100644 --- a/pytimeparse/tests/testtimeparse.py +++ b/pytimeparse/tests/testtimeparse.py @@ -358,6 +358,16 @@ class TestTimeparse(unittest.TestCase): self.assertEqual(timeparse.timeparse('+5.6 weeks'), 3386880) self.assertEqual(timeparse.timeparse('-5.6 weeks'), -3386880) + def test_timeparse_fractional_rounding(self): + '''Decimal quantities that equal a whole number of seconds must not be + truncated by binary floating-point error (e.g. 2.05 * 60 == 122.999...).''' + self.assertEqual(timeparse.timeparse('2.05m'), 123) + self.assertEqual(timeparse.timeparse('+2.05m'), 123) + self.assertEqual(timeparse.timeparse('-2.05m'), -123) + self.assertEqual(timeparse.timeparse('4.1m'), 246) + self.assertEqual(timeparse.timeparse('1.13h'), 4068) + self.assertEqual(timeparse.timeparse('8.03h'), 28908) + def test_doctest(self): '''Run timeparse doctests.''' self.assertTrue(doctest.testmod(timeparse, raise_on_error=True)) diff --git a/pytimeparse/timeparse.py b/pytimeparse/timeparse.py index ace5cc3..bcfe1a4 100644 --- a/pytimeparse/timeparse.py +++ b/pytimeparse/timeparse.py @@ -172,10 +172,14 @@ def timeparse(sval, granularity='seconds'): elif ('secs' not in mdict or mdict['secs'] is None or mdict['secs'].isdigit()): - # we will return an integer + # we will return an integer. Round (not truncate) the + # sub-second unit contribution: multiplying a decimal + # quantity such as 2.05 minutes by 60 yields a binary + # float slightly below the exact value (122.999...), and + # int() would truncate it to 122 instead of 123. return ( - sign * int(sum([MULTIPLIERS[k] * float(v) for (k, v) in - list(mdict.items()) if k != 'secs' and v is not None])) + + sign * int(round(sum([MULTIPLIERS[k] * float(v) for (k, v) in + list(mdict.items()) if k != 'secs' and v is not None]))) + (int(mdict['secs'], 10) if mdict['secs'] else 0)) else: # SECS is a float, we will return a float