Roundup Robot added the comment:
New changeset 8fe15bf68522 by Alexander Belopolsky in branch '3.4':
Fixes #23521: Corrected pure python implementation of timedelta division.
https://hg.python.org/cpython/rev/8fe15bf68522
New changeset d783132d72bc by Alexander Belopolsky in branch 'default':
Changes by Alexander Belopolsky alexander.belopol...@gmail.com:
--
resolution: - fixed
stage: commit review - resolved
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
Serhiy Storchaka added the comment:
LGTM.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
___
___
Python-bugs-list mailing list
Changes by Alexander Belopolsky alexander.belopol...@gmail.com:
--
stage: needs patch - commit review
versions: +Python 3.4
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
___
Alexander Belopolsky added the comment:
You saw the demo python implementation of divmod_near in Objects/longobject.c?
I was looking for it, but could not find. Thanks for the pointer. (See also
#8817.)
In issue23521-6.patch, I've used a slightly optimized variant of divmod_near
code and
Alexander Belopolsky added the comment:
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.
--
Added file:
Brian Kearns added the comment:
You saw the demo python implementation of divmod_near in Objects/longobject.c?
Maybe it's worth using a common implementation?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
Alexander Belopolsky added the comment:
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.)
--
Changes by Alexander Belopolsky alexander.belopol...@gmail.com:
--
nosy: +bdkearns, benjamin.peterson
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
___
New submission from Alexander Belopolsky:
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
Alexander Belopolsky added the comment:
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
Serhiy Storchaka added the comment:
LGTM. But the bug in C implementation should be fixed.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
___
Alexander Belopolsky added the comment:
You can use self._to_microseconds().
Right. Did that and added a simple test.
--
Added file: http://bugs.python.org/file38235/issue23521-2.patch
___
Python tracker rep...@bugs.python.org
Serhiy Storchaka added the comment:
You can use self._to_microseconds().
--
nosy: +serhiy.storchaka
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
___
Alexander Belopolsky added the comment:
the bug in C implementation should be fixed.
In the past (see #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)
Alexander Belopolsky added the comment:
Maybe we should also use divide_and_round for __truediv__
You mean in timedelta / float case, right? You are probably right, but can you
provide a test case in which it matters?
--
___
Python tracker
Alexander Belopolsky added the comment:
Fixed truediv in issue23521-4.patch.
--
Added file: http://bugs.python.org/file38237/issue23521-4.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
Brian Kearns added the comment:
Maybe we should also use divide_and_round for __truediv__ to match the C
implementation calling divide_nearest there as well?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
Alexander Belopolsky added the comment:
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
--
___
Python tracker rep...@bugs.python.org
Brian Kearns added the comment:
td = timedelta(seconds=1)
print(td / (1/0.6112295))
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
___
___
Alexander Belopolsky added the comment:
I eliminated floating point arithmetics from the calculation completely and it
now matches C.
--
Added file: http://bugs.python.org/file38236/issue23521-3.patch
___
Python tracker rep...@bugs.python.org
Alexander Belopolsky added the comment:
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
--
___
Python tracker rep...@bugs.python.org
Serhiy Storchaka added the comment:
Added comments on Rietveld.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue23521
___
___
Python-bugs-list
23 matches
Mail list logo