New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OverflowError from timedelta * float in datetime.py #67709
Comments
>>> import sys
>>> sys.modules['_datetime'] = None
>>> from datetime import timedelta
>>> timedelta(seconds=1)*0.6112295
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/a/Work/cpython/Lib/datetime.py", line 519, in __mul__
return self * a / b
File "/Users/a/Work/cpython/Lib/datetime.py", line 516, in __mul__
self._microseconds * other)
File "/Users/a/Work/cpython/Lib/datetime.py", line 411, in __new__
raise OverflowError("timedelta # of days is too large: %d" % d)
OverflowError: timedelta # of days is too large: 63720670102 C implementation is unaffected: >>> from datetime import timedelta
>>> timedelta(seconds=1)*0.6112295
datetime.timedelta(0, 0, 611229) |
Attached patch fixes the issue, but produces a slightly different result: >>> timedelta(seconds=1)*0.6112295
datetime.timedelta(0, 0, 611230) Note that C implementation is probably buggy: >>> from datetime import *
>>> timedelta(seconds=1)*0.6112295
datetime.timedelta(0, 0, 611229)
>>> timedelta(seconds=0.6112295)
datetime.timedelta(0, 0, 611230) |
You can use self._to_microseconds(). |
Right. Did that and added a simple test. |
LGTM. But the bug in C implementation should be fixed. |
In the past (see bpo-8860) we were not able to reach a consensus on which behavior is correct and which has a bug: >>> timedelta(seconds=1)*0.6112295
datetime.timedelta(0, 0, 611229)
>>> timedelta(seconds=0.6112295)
datetime.timedelta(0, 0, 611230) It all boils down to the the fact that >>> round(0.6112295*10**6)
611230 but >>> round(0.6112295, 6) * 10**6
611229.0 In my view, timedelta(seconds=1) is 10**6 microseconds and timedelta(seconds=1) * 0.6112295 is 611229.5 microseconds which is correctly rounded to 611230 in timedelta(seconds=0.6112295), but timedelta(seconds=1)*0.6112295 returning 611229 is incorrect. If I understand Mark's argument correctly (see msg107097), he views timedeltas as fractional number of seconds rather than integer number of microseconds and in his view timedelta(seconds=0.6112295) is timedelta(seconds=0.6112295) is 0.611229499999999981 seconds because >>> Decimal(0.6112295)
Decimal('0.61122949999999998116351207499974407255649566650390625') Thus timedelta(seconds=0.6112295), 0.6112295 should be rounded down to 6 decimal places and result in 611229 microseconds. Neither of us thought resolving this was important enough to hold other fixes. In the same spirit, I suggest that we apply my current patch that fixes a crash and pass the rounding curiosity down to the future generations. |
I eliminated floating point arithmetics from the calculation completely and it now matches C. |
Maybe we should also use divide_and_round for __truediv__ to match the C implementation calling divide_nearest there as well? |
You mean in timedelta / float case, right? You are probably right, but can you provide a test case in which it matters? |
td = timedelta(seconds=1)
print(td / (1/0.6112295)) |
>>> td = timedelta(seconds=1)
>>> print(td / (1/0.6112295))
0:00:00.611229 What is wrong with this answer? It is the same as in >>> print(td * 0.6112295)
0:00:00.611229 |
I've got it: >>> import sys
>>> sys.modules['_datetime'] = None
>>> from datetime import *
>>> td = timedelta(seconds=1)
>>> print(td / (1/0.6112295))
0:00:00.611230 |
Fixed truediv in bpo-23521-4.patch. |
Added comments on Rietveld. |
I addressed review comments and fixed the case of negative divisor. There may be a better solution than calling the function recursively, but I wanted an easy to understand code. |
You saw the demo python implementation of divmod_near in Objects/longobject.c? Maybe it's worth using a common implementation? |
I was looking for it, but could not find. Thanks for the pointer. (See also bpo-8817.) In bpo-23521-6.patch, I've used a slightly optimized variant of divmod_near code and beefed up the tests some more because the code |
We can further optimize _divide_and_round by changing r*=2 to r<<=1 and q % 2 == 1 to (q & 1), but this will obfuscate the algorithm and limit arguments to ints. (As written, _divide_and_round will work with any numeric types.) |
LGTM. |
New changeset 8fe15bf68522 by Alexander Belopolsky in branch '3.4': New changeset d783132d72bc by Alexander Belopolsky in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: