classification
Title: datetime.time support for '+' and '-'
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: belopolsky, eric.smith, francismb, jbatista, joar, maker, petri.lehtinen, r.david.murray, ronaldoussoren, skip.montanaro, thezulk
Priority: normal Keywords: needs review, patch

Created on 2013-02-21 12:47 by ronaldoussoren, last changed 2014-03-07 19:46 by belopolsky.

Files
File name Uploaded Description Edit
issue17267.patch joar, 2013-02-23 19:37 review
issue17267.patch joar, 2013-02-23 20:21 patch for time() + timedelta() (C speedups & pure) review
issue17267-3.4.diff thezulk, 2013-02-23 20:35 time_add and time_subtract review
issue17267-v2.patch joar, 2013-02-24 22:36 review
issue17267-v3.patch joar, 2013-02-26 21:28 review
Messages (19)
msg182592 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2013-02-21 12:47
It would be nice if datetime.time would be possible to add a delta to a datetime.time object, and if datetime.time had a method for returning the current time (just like datetime.date and date time.datetime have 'today' and 'now' methods).

Rationale for the '+' operator: calculating the wall clock time some time after an time on an unspecified date. The easy solution would be:

   tm = datetime.time(13, 20)
   later = tm + datetime.timedelta(hours=5, minutes=44)

That's is currently not possible, but requires more complicated code.

Getting the current time without date information currently requires getting done with 'datetime.datetime.now().time()', a class method of 'datetime.time' would IMHO be nicer. I must admit that I don't have a good suggestion for the name of that method.
msg182594 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2013-02-21 14:00
What would this give:

   tm = datetime.time(13, 20)
   later = tm + datetime.timedelta(hours=47, minutes=44)

datetime.time(13, 4)? Or raise an exception?

I've thought about this before, but it's always a problem when going over date boundaries. If you define "+" to be modulo 24 hours, then it's not very useful for cases I've looked at. Every time I've used time by itself, I end up going back to datetime. But I'll admit that might be a shortcoming of mine, not the concept.
msg182595 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2013-02-21 14:31
IMHO this would to module 24 arithmetic, just like a normal clock.

When I do calculations with plain time that is what I want, if the date is also important I use datetime.datetime.  That a time value silently truncates when going past midnight is IMHO also the obvious behavior.

The biggest argument against adding this functionality I could come up with is that this can give wrong answers when daylight savings time transitions happen, which could lead to subtle bugs.
msg182801 - (view) Author: Joar Wandborg (joar) * Date: 2013-02-23 19:37
Patch submitted, please review.
msg182807 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2013-02-23 19:58
Added some comments to Rietveld.
msg182814 - (view) Author: Joar Wandborg (joar) * Date: 2013-02-23 20:21
New patch adressing the review comments.
msg182818 - (view) Author: Andreas Åkerlund (thezulk) * Date: 2013-02-23 20:35
Well I have also made a patch for this, using the datetime operator code as much as possible.. this is for version 3.4..
msg182902 - (view) Author: Joar Wandborg (joar) * Date: 2013-02-24 22:36
I am adding yet another patch. This time it has

- TZ-aware tests (datetimetester.TestTimeTZ).
- C time_subtract method.
- Pure time.__sub__ method.

in addition to the previous

- Tests for time + timedelta
- C time_add
- Pure time.__add__

There's one issue though, and that is that I have not quite figured out TZ-aware cross-TZ `time - timedelta` i.e.

    time(0, tzinfo=est) - timedelta(hours=1) * 5 == time(0, tzinfo=utc)

fails.

I have included a failing test for it.
msg183090 - (view) Author: Joar Wandborg (joar) * Date: 2013-02-26 21:28
New patch, removed whitespace change and unnecessary test.

add_time_timedelta arg `factor` will not be changed, in order to preserve uniformity.
msg183114 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2013-02-27 06:15
LGTM.
msg183132 - (view) Author: jbatista (jbatista) Date: 2013-02-27 10:41
IMHO this should be "safe" when the timezone is UTC for example, where there is no problems with daylight savings. What should be the behavior when adding a certain timedelta() and it crosses a date where there is an hour switch due to daylight savings? The unadvised would get incorrect results.
msg183137 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2013-02-27 11:06
datetime.time arithmetic cannot be timezone aware, as there is no associated date and hence you cannot possibly know if there it a DST transition.

I don't think this is a problem. Adding/removing time to a clock value has clear real-world semantics. Using the (naive) real world semantics is the best we can do and should generally give the expected answer.

As to cross-timezone comparisons: 

   time(0, tzinfo=est) - timedelta(hours=1) * 5 == time(0, tzinfo=utc)

fails because the LHS of '==' is a time in a different timezone than the value on the RHS. That's expected and correct.
msg183138 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2013-02-27 11:10
A time object isn't associated with any date, so I don't really see a
problem here. The fact that you can shoot yourself in the leg can be
documented, noting that you should use datetime instead.

ISTM the reason why time objects even have an associated timezone is
to support easy calculations between times in different timezones.
msg183347 - (view) Author: Francis MB (francismb) * Date: 2013-03-02 20:16
Hi Joar,
just a detail: is there a reason for the asymmetric check for timedelta isinstance (and raising NotImplemented)? And BTW. isn't a double check for the __sub__ case (or have I missed something)?

+    def __add__(self, other):
+        "Add a time and a timedelta"
+        if not isinstance(other, timedelta):
+            return NotImplemented

vs.
…
+    def __sub__(self, other):
+        "Subtract a time and a timedelta."
+        if isinstance(other, timedelta):
+            return self + -other
+        return NotImplemented

regards,
francis
msg190697 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2013-06-05 22:05
I left a few minor comments on rietveld for the last patch.  I did not see code for time.now() and I don't think adding now() should be combined with time +/- timedelta patch.  Let's do one thing at a time.

I think time + timedelta addition is fairly uncontroversial.  In the past, I argued against using detached time objects, but it is not really a valid reason for rejecting a good feature.

On subtraction, if we add time - timedelta -> time, I think users would expect time - time -> timedelta as well.  This, however, is ambiguous if we stay with mod 24h arithmetic.  The ambiguity can be lifted by requiring days=0 in the result.
msg190698 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2013-06-05 22:09
This was proposed before and rejected in issue1118748, but I think current proposal addresses the ambiguity that was sited as a reason for rejection.
msg190699 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2013-06-05 22:17
See also issue 3250.  If we add mod 24h arithmetics, I would like to see something like time.add_with_carry(timedelta) -> (int, time) method.  With it, users who need a specific overflow behavior will be able to implement it easily:

def check_add(t, td):
    carry, result = t.add_with_carry(tf)
    if carry:
        raise ...
msg190700 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2013-06-05 22:22
I am changing the title to focus this issue on arithmetics. Lack of time.now() is adressed in #8902.
msg212898 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2014-03-07 19:46
I think the timezone related problems are a red herring.  Aware datetime +/- timedelta arithmetics is naive - tzinfo is ignored in calculations and copied to the result:

http://hg.python.org/cpython/file/c83ce2a1841c/Lib/datetime.py#l1711

The utcoffset only will only come into play if we want to implement time - time -> timedelta, but this problem is already there in time comparisons:

http://hg.python.org/cpython/file/c83ce2a1841c/Lib/datetime.py#l1091

It is up to tzinfo subclass implementation writers to handle inability to compute utcoffset without date fields by raising an exception if necessary.  It is perfectly fine for time - time to fail with an error coming from .utcoffset().

I also don't think the fate of #13936 has any bearing on this issue.  As long as we are not trying to implement time + time -> time, we are not introducing any new notion of "zero time".
History
Date User Action Args
2014-03-07 19:46:35belopolskysetmessages: + msg212898
2014-03-07 15:55:59r.david.murraysetnosy: + r.david.murray
2013-06-05 22:22:41belopolskysetmessages: + msg190700
title: datetime.time support for '+' and 'now' -> datetime.time support for '+' and '-'
2013-06-05 22:17:55belopolskysetmessages: + msg190699
2013-06-05 22:09:43belopolskysetnosy: + skip.montanaro
messages: + msg190698
2013-06-05 22:05:48belopolskysetmessages: + msg190697
2013-03-02 20:16:03francismbsetnosy: + francismb
messages: + msg183347
2013-02-27 11:10:23petri.lehtinensetmessages: + msg183138
2013-02-27 11:06:59ronaldoussorensetmessages: + msg183137
2013-02-27 10:41:59jbatistasetnosy: + jbatista
messages: + msg183132
2013-02-27 07:01:43ronaldoussorensetkeywords: + needs review
nosy: + belopolsky
2013-02-27 06:15:27petri.lehtinensetmessages: + msg183114
stage: test needed -> patch review
2013-02-26 21:28:54joarsetfiles: + issue17267-v3.patch

messages: + msg183090
2013-02-24 22:36:15joarsetfiles: + issue17267-v2.patch

messages: + msg182902
2013-02-23 20:35:02thezulksetfiles: + issue17267-3.4.diff
nosy: + thezulk
messages: + msg182818

2013-02-23 20:21:07joarsetfiles: + issue17267.patch

messages: + msg182814
2013-02-23 19:58:01petri.lehtinensetnosy: + petri.lehtinen
messages: + msg182807
2013-02-23 19:37:26joarsetfiles: + issue17267.patch

nosy: + joar
messages: + msg182801

keywords: + patch
2013-02-23 10:12:26makersetnosy: + maker
2013-02-21 14:31:56ronaldoussorensetmessages: + msg182595
2013-02-21 14:00:49eric.smithsetnosy: + eric.smith
messages: + msg182594
2013-02-21 12:47:59ronaldoussorencreate