dualfroz
7b055c8116
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.
75 lines
2.4 KiB
Markdown
75 lines
2.4 KiB
Markdown
# 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`.
|