[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Mark Dickinson added the comment: Alexander: applying this is fine by me. As a user-visible change, yes, I think it should have a Misc/NEWS entry. (It's too small a change to be worth mentioning in the 'whatsnew' documents though.) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Roundup Robot added the comment: New changeset f7c84ef35b00 by Alexander Belopolsky in branch 'default': Fixes #8860: Round half-microseconds to even in the timedelta constructor. http://hg.python.org/cpython/rev/f7c84ef35b00 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Changes by Alexander Belopolsky alexander.belopol...@gmail.com: -- resolution: - fixed stage: commit review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Alexander Belopolsky added the comment: I cleaned up the patch a little: 1. Removed now unused static round_to_long() function. 2. Removed commented out code. Mark, Any reason not to apply this? Do we need a NEWS entry for something like this? -- priority: low - normal stage: test needed - commit review versions: +Python 3.4 -Python 3.2 Added file: http://bugs.python.org/file31143/issue8860a.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Alexander Belopolsky added the comment: With the current patch we still have the following quirks: timedelta(seconds=0.6112295) == timedelta(seconds=1)*0.6112295 False timedelta(seconds=0.6112295) == timedelta(seconds=round(0.6112295, 6)) False This is not a reason to hold the patch, though. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Yu Tomita added the comment: I updated the unittest for the patch by Mark. The test fails without the patch and passes with it. Also, I updated the patch to work in Python 3.4 (_datetime.c instead of datetime.c). -- nosy: +nekobon Added file: http://bugs.python.org/file29458/issue8860.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: The timedelta(seconds=0.6112295) example is handled correctly No, it's not! It's being rounded *up* where it should be being rounded *down*. Let me try to reformulate the issue. When use is entering 0.6112295, she means 0.6112295, not 0x1.38f312b1b36bdp-1 or 0.611229498116351207499974407255649566650390625 which are exact values of the underlying binary representation of 0.6112295. Modern Python is able to round 0.6112295 to 6 decimal places correctly: round(0.6112295, 6) 0.611229 and timedelta should do the same, but it does not: timedelta(seconds=0.6112295) datetime.timedelta(0, 0, 611230) With respect to accumulation, I believe the invariant that we want to have should be timedelta(x=a, y=b, ...) == timedelta(x=a) + timedelta(y=b) while the alternative (using higher precision addition with rounding at the end) may be more accurate in some cases, the above looks least surprising. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: Similar problem affects fromtimestamp() constructors: datetime.fromtimestamp(0.0078125)-datetime.fromtimestamp(0) datetime.timedelta(0, 0, 7813) datetime.utcfromtimestamp(0.0078125)-datetime.utcfromtimestamp(0) datetime.timedelta(0, 0, 7813) both rounded to odd, but 0.0078125*timedelta(seconds=1) datetime.timedelta(0, 0, 7812) is correctly rounded to even. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: The timedelta(seconds=0.6112295) example is handled correctly because 0.6112295 sec is not half way between two nearest microseconds: abs(0.6112295 - 0.6112290) == abs(0.6112295 - 0.6112300) False The fact that it displays as if it is does not make timedelta rounding wrong. I am still not sure that it is possible to accumulate rounding error by adding seven doubles, each 1 to affect the rounded result. While proving that the rounding is always correct or finding a counter-example is an interesting puzzle, I think it has little practical value. I will add unit tests and get this patch ready for for commit review, but setting the priority to low. -- components: +Extension Modules priority: normal - low versions: +Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: The half-way check should be done with decimal arihmetics, but the result is the same: x = Decimal(0.6112295) abs(x - Decimal('0.6112290')) == abs(x - Decimal('0.6112300')) False -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Mark Dickinson dicki...@gmail.com added the comment: The timedelta(seconds=0.6112295) example is handled correctly No, it's not! It's being rounded *up* where it should be being rounded *down*. because 0.6112295 sec is not half way between two nearest microseconds Exactly. The actual value stored by the C double is a little closer to 0.611229 than to 0.611230. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Mark Dickinson dicki...@gmail.com added the comment: The accumulated error doesn't need to exceed 1; it just needs to be enough to make (e.g.) leftover appear to be = 0.5 when it's actually just less than 0.5. It shouldn't be too hard to find examples where this happens, but I haven't thought about it. I wonder if it would be justified to expose something like int _PyLong_IsOdd(PyObject *self) {...} Yes, that could work. I'd be happier with this if there are other uses (even within longobject.c); there might well be, but I haven't looked. I also have a mild preference for _PyLong_IsEven over _PyLong_IsOdd. :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Mark Dickinson dicki...@gmail.com added the comment: An example where getting correct rounding is awkward: the following rounds up, with or without the patch: datetime.timedelta(seconds=0.6112295) datetime.timedelta(0, 0, 611230) But in this case, the result should actually be rounded down (with either rounding mode), since the exact value of float('0.6112295') is a little less than 0.6112295: print(Decimal(0.6112295)) 0.611229498116351207499974407255649566650390625 The problem is that leftover_us ends up being exactly 0.5 in this case. Again, this could be worked around if we really care, but I'm not convinced it's worth it. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
New submission from Alexander Belopolsky belopol...@users.sourceforge.net: From issue1289118, msg106389: from datetime import timedelta as d [d(microseconds=i + .5)//d.resolution for i in range(-10,10)] [-10, -9, -8, -7, -6, -5, -4, -3, -2, -1, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10] Should this be considered a bug? For comparison, [d.resolution*(i+0.5)//d.resolution for i in range(-10,10)] [-10, -8, -8, -6, -6, -4, -4, -2, -2, 0, 0, 2, 2, 4, 4, 6, 6, 8, 8, 10] and [round(i+0.5) for i in range(-10,10)] [-10, -8, -8, -6, -6, -4, -4, -2, -2, 0, 0, 2, 2, 4, 4, 6, 6, 8, 8, 10] I checked the documentation and while it says: If any argument is a float and there are fractional microseconds, the fractional microseconds left over from all arguments are combined and their sum is rounded to the nearest microsecond. it does not specify how half-integers should be handled. While it may not be a bug in strict sense, it looks like the code in question can be improved. -- assignee: belopolsky messages: 106793 nosy: belopolsky, haypo, mark.dickinson, mcherm, rhettinger, stutzbach, tim_one priority: normal severity: normal stage: unit test needed status: open title: Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics type: feature request ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: Here is a shorter example of inconsistent behavior: 0.5 * timedelta(microseconds=1) datetime.timedelta(0) timedelta(microseconds=0.5) datetime.timedelta(0, 0, 1) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Mark Dickinson dicki...@gmail.com added the comment: I agree it would be nice to fix this. We could either (1) alter delta_new so that the final round uses round-to-nearest; this would give the desired behaviour in most cases, but there would still be a small possibility of rounding going in the wrong direction as a result of accumulated errors in 'leftover_us'. Or (2) rewrite delta_new to do correct rounding in all cases, by using integer arithmetic where necessary. (2) seems like overkill to me. For (1), it looks like it would be enough just to replace the round_to_long function with: static long round_to_long(double x) { return (long)round(x); } Actually, at this point, one might as well just inline round_to_long. Note that round_to_long is also called from datetime_from_timestamp. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Mark Dickinson dicki...@gmail.com added the comment: Aargh! No, I take that back. round() also does round-half-away-from-zero, of course. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Mark Dickinson dicki...@gmail.com added the comment: Here's a first stab at a patch. It still needs tests. -- keywords: +patch Added file: http://bugs.python.org/file17508/issue8860.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: Mark (2) seems like overkill to me. I agree, however it would be interesting to figure out when accumulated errors can produce an inaccurate result. ISTM that leftover is the sum of up to 7 doubles each between 0 and 1 which is then rounded to the nearest integer. I don't see how accumulated error can exceed 1 and affect the result. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue8860] Rounding in timedelta constructor is inconsistent with that in timedelta arithmetics
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: I wonder if it would be justified to expose something like int _PyLong_IsOdd(PyObject *self) { PyLongObject *lo = (PyLongObject *)self; return Py_SIZE(lo) != 0 ((lo-ob_digit[0] 1) != 0); } in longobject.h? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue8860 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com