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
timedelta multiply and divide by floating point #42364
Comments
In python 2.4.1, the datetime.timedelta type allows for For example: >>> import datetime
>>> datetime.timedelta(minutes=5)/2
datetime.timedelta(0, 150)
>>> datetime.timedelta(minutes=5)*0.5
Traceback (most recent call last):
File "<stdin>", line 1, in ?
TypeError: unsupported operand type(s) for *:
'datetime.timedelta' and 'float' |
Logged In: YES Tim, do you prefer the current behavior? |
Logged In: YES I, too, would like to know what Tim thinks, but for what it's |
Logged In: YES timedelta arithmetic is 100% portable now, and wholly That said, I don't have a strong objection to complicating the If someone wants to work on it, note that a timedelta can |
Logged In: YES Let me elaborate on the use-case where I originally ran into I'm conducting a series of observation experiments where I Basically, I want timedelta objects to look and act like For implementation, why not multiply the float times .day, I agree it'd be possible to lose information with the wrong |
Logged In: YES
How about adding tolong() that returns the number of microseconds |
Attached is a diff to the datetime module that |
Ummm... make that: "I'll implement multiplication." |
I like this idea, it's the opposite of the issue bpo-2706. |
This is in a way more similar to bpo-1083 than to bpo-2706. I am -1 on this RFE for the same reason as I am opposing allowing true division of timedelta by an int. The timedelta type is fundamentally an integer type. A type delta is just a certain number of microseconds. A timedelta divided by a number or multiplied by a float is logically a fractional number of microseconds and python does not have a type to represent it. Daniel's use case of passing timedeltas to a statistical packages is neatly addressed by bpo-2706's timedelta / timedelta (true) division. Just strip the dimensionality from your data by dividing each time delta by a chosen unit interval (depending on the problem, a second, a microsecond or even a day may be appropriate). The result will be a set of floats that your number crunching package will be happy to process. Another advantage of this approach is that floats can be processed more efficiently than timedeltas with FP arithmetics and intermediate results will be more accurate in most cases. I recommend accepting bpo-2706 and rejecting this issue together with bpo-2706. |
I meant rejecting bpo-1083, of course. |
I disagree strongly with this, and find this a bizarre point of view. Regardless of how the timedelta is stored internally, it's used to represent physical times. I doubt there are many applications that care about the fact that each timedelta is an integral number of microseconds. Multiplication or division of a time by a float or int makes perfect sense physically, and I think it should be a legal operation here. |
Not sure why this is marked for 3.3. |
I'll take a look at Skip's patch. |
Whoops. I meant to assign this to me, not Skip. |
Sorry, dropping this again. I've got caught up with too many non-datetime related issues. |
dt.diff does not apply to current SVN version anymore. I am attaching a quick update that does not change the actual calculation performed. See bpo-1289118-py3k.diff. I am still -1 for the reason I stated before, but I would like to review a working patch first before proposing a resolution. |
Alexander, I still don't understand your objection. What's the downside of allowing the multiplication or division of a timedelta by a float? Perhaps it's true that there are applications where timedeltas are best viewed as integers (with an implicitt 'microsecond' unit), but I think it's also true that there are plenty of applications where they're just regarded as a representation of a physical quantity, and there this proposal seems entirely appropriate. I *would* want the timedelta * float and timedelta / float operations to be correctly rounded, so that behaviour is entirely predictable; the current patch doesn't do that. But it wouldn't be hard to implement: there are functions available to express a float as a quotient of two integers, and after that the computation can be performed in integer arithmetic. |
Python reference implementation showing how to do correct rounding. |
N.B. There's already logic for doing div_nearest (i.e., divide one integer by another, returning the closest integer to the result) in the long_round function in Objects/longobject.c. It might be worth pulling that logic out and making it available in a _Py function so that it can be reused in other modules. |
I am attaching a patch that implements Mark's timedelta_arith.py algorithms in C. With rounding well defined, I am close to withdrawing my opposition to supporting mixed timedelta with float operations. There is still one issue that I believe is worth discussing before this feature is accepted. Time, unlike most other physical quantities has a non-arbitrary notion of direction. Therefore, in many applications, rounding towards past or towards future may be preferable to rounding to nearest. For example, one of the likely applications of floating point division would be to construct time series from continuous or differently sampled functions. If such series are used to measure correlations between cause and effect, it is important that effect is measured at a time following the cause and not at an early or the same moment. As Mark noted in private correspondence, this issue is mitigated by the fact that "with correct rounding, for timedeltas t and s, and a positive float x, it is guaranteed that t <= s implies t op x <= s op x" (where op is either * or /). It is still possible however, that even the case of t < s and t op x == s op x present a problem in some applications. Despite this issue, I would support round to nearest even choice over round to past or to future mainly because it is less likely to lead to surprises where (d1/d2) * d2 != d1. This choice also conforms with the round() builtin definition and is somewhat more difficult to implement right using existing means. Daniel, would you like to chime in on the questions of how the results of these operations should be rounded? If I don't hear principle objections from the "nosy" list, I'll add a documentation patch. |
I don't have a strong feeling about the method of rounding. My thinking is: If my application is sensitive to how the last microsecond is rounded, then I shouldn't be using a type that only gives me 1-microsecond precision. (Likewise, if my application is sensitive to how the last binary digital of the floating point mantissa is rounded ... I'm in trouble) That said, round-to-nearest strikes me as the least-surprising approach. |
Re rounding: I'll just note that timedelta / timedelta -> float currently does round to nearest; I'd find it quite surprising if float * timedelta -> timedelta didn't round to nearest. |
It looks like we have a consensus on the rounding mode. Note, however [-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, [-10, -8, -8, -6, -6, -4, -4, -2, -2, 0, 0, 2, 2, 4, 4, 6, 6, 8, 8, 10] [-10, -8, -8, -6, -6, -4, -4, -2, -2, 0, 0, 2, 2, 4, 4, 6, 6, 8, 8, 10] |
Alexander, it looks like Roundup ate some of your message there. :) Yes, ideally I'd say that the constructor should be doing round-half-to-even. Though last time I looked, the constructor looked quite complicated (especially for float inputs); it may not be feasible to fix this easily. At any rate, we should open a separate issue for this. |
Indeed. Here is what I intended: """
>>> 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. I'll open a separate issue for this. |
By the way, does your patch do the right thing for timedelta(microseconds=1) / -4.0 ? Because my Python code doesn't. :) [If n is negative, then the 2*r > n condition in div_nearest should be 2*r < n instead.] |
There's a patch in bpo-8817 that exposes a round-to-nearest form of divmod in a function called _PyLong_Divmod_Near; this would save on duplication of code. |
No. >>> timedelta(microseconds=1) / -4.0
datetime.timedelta(-1, 86399, 999999) (I just copied your python algorithm ...) I will merge with bpo-8817 patch and that should fix the problem. |
Attaching a combined bpo-1289118 + bpo-8817 patch. Datetime code now uses bpo-8817's _PyLong_Divmod_Near. |
Attaching a new patch with documentation changes, additional tests, updated bpo-8817 patch and a reference leak fix. |
The patch looks good to me. Please replace the tab characters in datetimemodule.c with spaces, though. :) |
Committed in r81625. Fixed white space and added a note to "new in 3.2" section of the RST doc. |
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: