Upstream wroberts/pytimeparse state of the touched files before 3270e0d981
This commit is contained in:
1 parent
a0b5007d40
commit
81c649efa1
3 files changed
+1
-79
No files matched your search
@@ -1,74 +0,0 @@
|
||||
# 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`.
|
||||
Reference in new issue
Block a user