Skip to content
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

datetime: define division timedelta/timedelta #46958

Closed
webograph mannequin opened this issue Apr 27, 2008 · 39 comments
Closed

datetime: define division timedelta/timedelta #46958

webograph mannequin opened this issue Apr 27, 2008 · 39 comments
Assignees
Labels
extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@webograph
Copy link
Mannequin

webograph mannequin commented Apr 27, 2008

BPO 2706
Nosy @jribbens, @amauryfa, @mdickinson, @vstinner, @merwok, @bitdancer
Files
  • datetime_datetime_division.patch: patch as described
  • datetime_datetime_division_dupcode.patch: patch without function pointers
  • timedeltadiv.parch: Patch with tests against revision 67223
  • timedelta_true_divide_divmod.patch
  • unnamed
  • issue2706a.diff: patch against py3k revision 80210
  • 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:

    assignee = 'https://github.com/mdickinson'
    closed_at = <Date 2010-04-20.22:44:53.246>
    created_at = <Date 2008-04-27.21:04:03.427>
    labels = ['extension-modules', 'type-feature', 'library']
    title = 'datetime: define division timedelta/timedelta'
    updated_at = <Date 2010-04-21.17:54:08.177>
    user = 'https://bugs.python.org/webograph'

    bugs.python.org fields:

    activity = <Date 2010-04-21.17:54:08.177>
    actor = 'mark.dickinson'
    assignee = 'mark.dickinson'
    closed = True
    closed_date = <Date 2010-04-20.22:44:53.246>
    closer = 'mark.dickinson'
    components = ['Extension Modules', 'Library (Lib)']
    creation = <Date 2008-04-27.21:04:03.427>
    creator = 'webograph'
    dependencies = []
    files = ['10126', '10946', '12010', '12011', '16999', '17003']
    hgrepos = []
    issue_num = 2706
    keywords = ['patch']
    message_count = 39.0
    messages = ['65902', '70018', '75876', '75877', '75878', '75880', '75881', '75882', '75883', '75884', '75892', '75894', '75895', '75908', '75909', '75913', '75917', '77646', '83416', '83447', '83453', '83936', '84131', '84151', '102956', '102962', '103615', '103631', '103634', '103635', '103636', '103637', '103638', '103653', '103664', '103787', '103788', '103867', '103872']
    nosy_count = 11.0
    nosy_names = ['jribbens', 'amaury.forgeotdarc', 'mark.dickinson', 'vstinner', 'jess.austin', 'fredrikj', 'webograph', 'eric.araujo', 'r.david.murray', 'tleeuwenburg@gmail.com', 'Alexander.Belopolsky']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue2706'
    versions = ['Python 3.2']

    @webograph
    Copy link
    Mannequin Author

    webograph mannequin commented Apr 27, 2008

    i suggest that division be defined for timedelta1/timedelta2, in that
    sense that it gives how many times timedelta2 fits in timedelta1 (ie the
    usual meaning of division), using integer arithmetics for floor division
    (//) and returning float for truediv (/ after from __future__ import division)

    use case
    --------

    aside from the obvious how-many-times-does-a-fit-into-b, this solves the
    issue of having individual methods for conversion to a number of
    seconds, hours, days or nanocenturies (as described in bpo-1673409).
    example:

    from datetime import timedelta
    duration = timedelta(hours=1.5, seconds=20)
    print "Until the time is up, you can listen to 'We will rock you' %d
    times."%(duration//timedelta(minutes=5, seconds=3))
    import time
    time.sleep(duration/timedelta(seconds=1))

    history
    -------

    this issue follows a discussion on python-list, re-initiated by [1].

    there have previously been similar feature requests on datetime, most of
    which have been rejected due to ambiguities (e.g. [2]), conflicts with
    time_t or issues with time zones.

    the only issue i've seen that can be relevant here is the
    integer-vs-float discussion, which is here handled by floordiv (//) and
    truediv.

    patch
    -----

    i've written a patch against svn trunk revision 62520.

    it uses function pointers to reduce code duplication; in case this
    inappropriate here, i also have a pointerless version.

    i familiar with c but not experienced, especially with the python ways
    of writing c. most of the code is just adapted from other functions in
    the same files, so it is probably, but should nevertheless checked with
    special care.

    i've also added test, but am not sure what has to be tested and what not.

    compatibility
    -------------

    only cases in which division would fail without the patch are changed.
    this will be a problem if (and only if) someone divides unknown objects
    and waits for TypeError to be raised.
    such behavior is probably rare.

    [1] <mid:4813CD56.40800@eml.cc>,
    http://mail.python.org/pipermail/python-list/2008-April/488406.html
    [2] http://mail.python.org/pipermail/python-dev/2002-March/020604.html

    @webograph webograph mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Apr 27, 2008
    @webograph
    Copy link
    Mannequin Author

    webograph mannequin commented Jul 19, 2008

    this is the mentioned patch without the function pointers, in case it
    better fits the python coding style.

    @abalkin
    Copy link
    Member

    abalkin commented Nov 14, 2008

    I attaching webograph's patch updated to revision 67223 where I added a
    few tests.

    I am +1 on the floor divide changes (allowing timedelta // timedelta),
    but I am not sure how true division should work if at all. For the sake
    of argument, let's assume from __future__ import division or py3k.
    Currently:

    >>> timedelta(1)/2
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: unsupported operand type(s) for /: 'datetime.timedelta' and 
    'int'
    >>> timedelta(1)/timedelta(2)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: unsupported operand type(s) for /: 'datetime.timedelta' and 
    'datetime.timedelta'

    With this patch, timedelta(1)/2 is still unsupported, but

    >>> timedelta(1)/timedelta(2)
    0.5

    Note that this is probably correct behavior because timedelta/int true
    division does not make much sense, but there is a proposal (see bpo-1083) to make timedelta/int equivalent to timedelta//int. (I am
    against bpo-1083 proposal and for webograph's approach, but this needs
    to be further discussed.)

    Also, I've added a test that demonstrates the following behavior:

    >>> int(timedelta.min/(timedelta.min//3))
    2

    This is not a bug in webograph's patch, but rather a bug in long true
    division, but it shows that true division may not be as useful as it
    seems.

    @vstinner
    Copy link
    Member

    Why not also implementing divmod()? It's useful to split a timedelta
    into, for example, (hours, minutes, seconds):

    def formatTimedelta(delta):
        """
        >>> formatTimedelta(timedelta(hours=1, minutes=24, seconds=19))
        '1h 24min 19sec'
        """
        hours, minutes = divmodTimedelta(delta, timedelta(hours=1))
        minutes, seconds = divmodTimedelta(minutes, timedelta(minutes=1))
        seconds, fraction = divmodTimedelta(seconds, timedelta(seconds=1))
        return "{0}h {1}min {2}sec".format(hours, minutes, seconds)

    My implementation gives divmod(timedelta, timedelta) -> (long,
    timedelta). It's a little bit strange to get two different types in
    the result. The second return value is the remainder. My example works
    in the reverse order of the classical code:

    def formatSeconds(seconds):
        """
        >>> formatTimedelta(1*3600 + 24*60 + 19)
        '1h 24min 19sec'
        """
        minutes, seconds = divmod(seconds, 60)
        hours, minutes = divmod(minutes, 60)
        return "{0}h {1}min {2}sec".format(hours, minutes, seconds)
        
    About my new patch:
     - based on datetime_datetime_division_dupcode.patch
     - create divmod() operation on (timedelta, timedelta)
     - add unit tests for the division (floor and true division) and 
    divmod
     - update the documentation for the true division and divmod

    @abalkin
    Copy link
    Member

    abalkin commented Nov 14, 2008

    On Fri, Nov 14, 2008 at 12:51 PM, STINNER Victor <report@bugs.python.org> wrote:

    STINNER Victor <victor.stinner@haypocalc.com> added the comment:

    Why not also implementing divmod()? It's useful to split a timedelta
    into, for example, (hours, minutes, seconds):

    I agree and in this case mod should probably be implemented too.

    With your patch:

    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: unsupported operand type(s) for %: 'datetime.timedelta' and
    'datetime.timedelta'

    @abalkin
    Copy link
    Member

    abalkin commented Nov 14, 2008

    Also, why not

    >>> divmod(timedelta(3), 2)
    (datetime.timedelta(1), datetime.timedelta(1))

    ?

    And where do we stop? :-)

    On Fri, Nov 14, 2008 at 1:02 PM, Alexander Belopolsky
    <belopolsky@users.sourceforge.net> wrote:
    > On Fri, Nov 14, 2008 at 12:51 PM, STINNER Victor 
    <report@bugs.python.org> wrote:
    >>
    >> STINNER Victor <victor.stinner@haypocalc.com> added the comment:
    >>
    >> Why not also implementing divmod()? It's useful to split a timedelta
    >> into, for example, (hours, minutes, seconds):
    >
    > I agree and in this case mod should probably be implemented too.
    >
    > With your patch:
    >
    >>>> timedelta(3)%timedelta(2)
    > Traceback (most recent call last):
    >  File "<stdin>", line 1, in <module>
    > TypeError: unsupported operand type(s) for %: 'datetime.timedelta' and
    > 'datetime.timedelta'
    >

    @vstinner
    Copy link
    Member

    Since timedelta(3) // 2 is already accepted, divmod should also accept
    integers (but not float).

    With the last patch and "from __future__ import division", we support:
    timedelta // <timedelta or int>
    timedelta / timedelta
    divmod(timedelta, timedelta)

    What do you think about:
    timedelta / <timedelta or int or float> # only with __future__.divison
    timedelta // <timedelta or int>
    timedelta % <timedelta or int>
    divmod(timedelta, <timedelta or int>)
    with:
    timedelta // int -> timedelta
    timedelta // timedelta -> int
    timedelta % int -> timedelta
    timedelta % timedelta -> int
    divmod(timedelta, int) -> (timedelta, timedelta)
    divmod(timedelta, timedelta) -> (int, timedelta)
    timedelta / <anything> -> float # __future__.divison

    @abalkin
    Copy link
    Member

    abalkin commented Nov 14, 2008

    While I agree that divmod may be useful, your particular use case is
    not convincing. The same can be done easier without divmod:

    def formatTimedelta(delta):
       return "{0}h {1}min {2}sec".format(*str(delta).split(':'))

    or you can convert delta to time using an arbitrary anchor date and
    extract hms that way:

    (1, 24, 19)

    (depending on your needs you may want to add delta.days*24 to the hours)

    On Fri, Nov 14, 2008 at 12:51 PM, STINNER Victor <report@bugs.python.org> wrote:
    >
    > STINNER Victor <victor.stinner@haypocalc.com> added the comment:
    >
    > Why not also implementing divmod()? It's useful to split a timedelta
    > into, for example, (hours, minutes, seconds):
    >
    > def formatTimedelta(delta):
    >    """
    >    >>> formatTimedelta(timedelta(hours=1, minutes=24, seconds=19))
    >    '1h 24min 19sec'
    >    """
    >    hours, minutes = divmodTimedelta(delta, timedelta(hours=1))
    >    minutes, seconds = divmodTimedelta(minutes, timedelta(minutes=1))
    >    seconds, fraction = divmodTimedelta(seconds, timedelta(seconds=1))
    >    return "{0}h {1}min {2}sec".format(hours, minutes, seconds)
    >

    @abalkin
    Copy link
    Member

    abalkin commented Nov 14, 2008

    On Fri, Nov 14, 2008 at 1:28 PM, STINNER Victor <report@bugs.python.org> wrote:
    ..

    What do you think about:
    timedelta / <timedelta or int or float> # only with __future__.divison
    timedelta // <timedelta or int>
    timedelta % <timedelta or int>
    divmod(timedelta, <timedelta or int>)
    with:
    timedelta // int -> timedelta
    already there

    +1

    +1

    +1

    timedelta % float -> timedelta (because int % float -> int works) ?

    +1

    +1

    divmod(timedelta, float) -> (timedelta, timedelta) ?

    -1

    Only timedelta / timedelta should produce dimensionless numbers.
    timedelta / <float or int> should be disallowed in true division mode.
    I am +0 on timedelta / timedelta -> float in true division mode.

    @abalkin
    Copy link
    Member

    abalkin commented Nov 14, 2008

    Oops, the tracker ate some lines from e-mail. Reposting through the
    web:

    On Fri, Nov 14, 2008 at 1:28 PM, STINNER Victor <report@bugs.python.org>
    wrote:
    ..

    What do you think about:
    timedelta / <timedelta or int or float> # only with
    __future__.divison
    timedelta // <timedelta or int>
    timedelta % <timedelta or int>
    divmod(timedelta, <timedelta or int>)
    with:
    timedelta // int -> timedelta
    already there

    timedelta // timedelta -> int
    +1

    timedelta % int -> timedelta
    +1

    timedelta % timedelta -> int
    +1

    timedelta % float -> timedelta (because int % float -> int works) ?

    divmod(timedelta, int) -> (timedelta, timedelta)
    +1

    divmod(timedelta, timedelta) -> (int, timedelta)
    +1

    divmod(timedelta, float) -> (timedelta, timedelta) ?

    timedelta / <anything> -> float # __future__.divison
    -1

    Only timedelta / timedelta should produce dimensionless numbers.
    timedelta / <float or int> should be disallowed in true division mode.
    I am +0 on timedelta / timedelta -> float in true division mode.
    Reply

    Forward

    @vstinner
    Copy link
    Member

    def formatTimedelta(delta):
    return "{0}h {1}min {2}sec".format(*str(delta).split(':'))

    OMG, this is ugly! Conversion to string and reparse the formatted text :-/
    Your code doesn't work with different units than hours, minutes or seconds:

    ['4 days, 1', '32', '01']
    >>> str(timedelta(hours=1, minutes=32, seconds=1, microseconds=2)).split(":")
    ['1', '32', '01.000002']

    or you can convert delta to time using an arbitrary anchor date
    and extract hms that way:

    How? I don't understand your suggestion.

    (depending on your needs you may want to add delta.days*24 to the hours)

    The goal of the new operators (timedelta / timedelta, divmod(timedelta,
    timedelta), etc.) is to avoid the use of the timedelta "internals" (days,
    seconds and microseconds attributes) and give a new "natural" way to process
    time deltas.

    @abalkin
    Copy link
    Member

    abalkin commented Nov 14, 2008

    haypo> How? I don't understand your suggestion.

    Sorry, another case of mail to tracker bug. Here is what I wrote:

    """
    .. you can convert delta to time using an arbitrary anchor date and
    extract hms that way:

    >>> x = datetime(1,1,1) + timedelta(hours=1, minutes=24, seconds=19)
    >>> x.hour,x.minute,x.second
    (1, 24, 19)

    (depending on your needs you may want to add delta.days*24 to the hours)
    """

    but tracker ate the '>>>' lines :-(

    @vstinner
    Copy link
    Member

    @WeboGraph: time_gmtime() and time_localtime() already use function
    pointer. I prefer function pointer than code duplication!

    @mdickinson
    Copy link
    Member

    timedelta / <float or int> should be disallowed in true division mode.

    I don't understand this; why should the division mode affect
    division operations involving timedeltas at all? The meaning
    of "/" is unaffected by the division mode for float/float or
    float/int; why should timedeltas be any different?

    I vote +1 for timedelta/timedelta and timedelta/float (regardless
    of division mode). timedelta / timedelta is the one obvious way
    to find out 'how many A's in B', and the one that it's natural
    to try first, before looking for (timedelta -> float) conversion
    methods.

    @mdickinson
    Copy link
    Member

    By the way, I assume that any plan to add this division would also include
    adding the inverse operation:

    timedelta * float -> timedelta.

    It wouldn't make a whole lot of sense to have one without the other.

    @vstinner
    Copy link
    Member

    Some examples to help the choice (using the last patch).

    int
    ---

    2L
    >>> print dt2 * 2
    3:08:38
    >>> print dt1 - dt2 * 2
    0:51:22
    >>> divmod(dt1, dt2)
    (2L, datetime.timedelta(0, 3082))
    >>> print timedelta(0, 3082)
    0:51:22

    In 4 hours, you can watch the movie twice, and then your have 51 minutes left.

    Operations used:

    • timedelta // timedelta
    • timedelta * int
    • divmod(timedelta, timedelta)

    float
    -----

    0.21258172822053367
    >>> "Progress: %.1f%%" % ((dt1 / dt2) * 100.0)
    'Progress: 21.3%'
    >>> dt2 * 0.75
    ...
    TypeError: unsupported operand type(s) for *: 'datetime.timedelta' and 'float'
    >>> print (dt2 * 3) // 4
    1:10:44.250000

    If you are seen this movie since 20 minutes, you're at 21% of the total. If
    you want to jump to 75%, it will be at 1:10:44.

    Note: timedelta * float is not implemented yet.

    Operations used:

    • timedelta / timedelta
    • timedelta * int and timedelta // int (because timdelta / float is
      not implemented yet)

    @abalkin
    Copy link
    Member

    abalkin commented Nov 15, 2008

    On Sat, Nov 15, 2008 at 5:08 AM, Mark Dickinson <report@bugs.python.org> wrote:

    Mark Dickinson <dickinsm@gmail.com> added the comment:

    > timedelta / <float or int> should be disallowed in true division mode.

    I don't understand this; why should the division mode affect
    division operations involving timedeltas at all?

    Here is how I think about this: timedeltas are integers in units of
    time. For simplicity, let's assume we always express timedeltas in
    microseconds (us), so timedeltas are things like 1us, 2us, etc. As
    with integers, we can define true division (/) and floor division
    (//) so that 3us/2us = 1.5 and 3us//2us = 1. Note that the result is
    dimensionless. Similarly, you can divide timedeltas by dimensionless
    integers: 3us/2 = 1.5us and 3us//2 = 1us. However in python you
    cannot have a timedelta representing 1.5us, so timedelta(0, 0, 3)/2
    should be en error. In order to have a timedelta/int true division,
    we would need to have another type floattimedelta which would be a
    floating point number in units of time.

    The meaning of "/" is unaffected by the division mode for float/float or
    float/int; why should timedeltas be any different?

    Because they are integers. If we had a floattimedelta type that would
    store timestamp as a float, its division would rightfully not be
    affected by the division mode.

    I vote +1 for timedelta/timedelta and timedelta/float (regardless
    of division mode).

    What do you vote timedelta/timedelta should produce in floor division
    mode: an int or a float? and what should timedelta/float produce: a
    timedelta or a float?

    @vstinner
    Copy link
    Member

    I'm finally opposed to datetime.totimedelta() => float, I
    prefer .totimedelta() => (second, microsecond) which means (int,int).
    But I like timedelta/timedelta => float, eg. to compute a progression
    percent. Anyone interested by my last patch (implement
    timedelta/timedelta and divmod(timedelta, timedelta)?

    @fredrikj
    Copy link
    Mannequin

    fredrikj mannequin commented Mar 10, 2009

    I think datetime division would be a fine application for Fractions.

    @tleeuwenburggmailcom
    Copy link
    Mannequin

    tleeuwenburggmailcom mannequin commented Mar 10, 2009

    Hi all,

    I'm trying to help out by reviewing issues in the tracker... so this is
    just a first attempt and I hope it is somewhat useful. This issue covers
    a number of discrete functional changes. I here review each in turn:

    1. Allow truediv, the operator '/', to be applied to two timedeltas
      (e.g. td1 / td2). The return value is a float representing the number of
      times that td2 goes into td1.

    Evaluation: Since both time deltas are quantitative values of the same
    unit, it makes sense they should be able to support basic math
    operations. In this case, operation to be added is '/' or truediv. I
    would personally find this functionality useful, and believe it is a
    natural addition to the code.

    Recommendation: That this functionality be recommended for development

    1. Allow truediv to be applied to a timedelta and an int or float. The
      return value is a timedelta representing the fractional proportion of
      the original timedelta.

    Evaluation: This makes total sense.
    Recommendation: That this functionality be recommended for development

    1. Allow divmod, the operator '%', to be applied to two timedeltas (e.g.
      td1 % td2). I think there is some debate here about whether the return
      value be an integer in microsends, or a timedelta. I personally believe
      that a timedelta should be returned, representing the amount of time
      remaining after (td1 // td2) * td2 has been subtracted.

    The alternative is an integer, but due to a lack of immediate clarity
    over what time quanta this integer represents, I would suggest returning
    a unit amount. There is also some discussion of returning (long,
    timedelta), but I personally fail to see the merits of returning the
    long without some unit attached.

    1. Allow divmod, the operator '%', to be applied to a timedelta and an
      integer or float. (e.g. <timedelta> % 3). I'm not quite as sold on this.
      I suggest that more discussion is required to flesh this out further.

    A patch has been attached which implements some of this behaviour. I
    would suggest that it's valuable to start doing this work in discrete
    chunks so that progress can be begun before the issues under debate are
    resolved. I suggest that it would be appropriate to create three smaller
    issues, each tackling just one piece of functionality. This should make
    the changes more atomic and the code patch simpler.

    -T

    @abalkin
    Copy link
    Member

    abalkin commented Mar 11, 2009

    On Tue, Mar 10, 2009 at 5:15 PM, Tennessee Leeuwenburg
    <report@bugs.python.org> wrote:
    ..

    1. Allow divmod, the operator '%', to be applied to two timedeltas (e.g.
      td1 % td2). I think there is some debate here about whether the return
      value be an integer in microsends, or a timedelta. I personally believe
      that a timedelta should be returned, representing the amount of time
      remaining after (td1 // td2)  * td2 has been subtracted.

    The alternative is an integer, but due to a lack of immediate clarity
    over what time quanta this integer represents, I would suggest returning
    a unit amount. There is also some discussion of returning (long,
    timedelta), but I personally fail to see the merits of returning the
    long without some unit attached.

    I don't think this alternative was ever seriously considered and no
    patch was ever proposed to do it that way. Victor's latest patch
    implements divmod(timedelta, timedelta) -> (int, timedelta) and
    therefore timedelta % timedelta -> timedelta.

    @webograph
    Copy link
    Mannequin Author

    webograph mannequin commented Mar 21, 2009

    i don't think this can be solved in a way that is independent of the
    chosen unit, as it requires a concept of "whole time-units" (as in
    "whole numbers"); whether these be seconds or minutes would be
    completely arbitrary.

    (5 minutes % 3 = 0 minutes would be true if based on seconds because
    5 minutes = 3 * 100 seconds + 0 minutes but 5 minutes % 3 = 2 minutes based on minutes because 5 minutes = 3 * 1 minute + 2 minutes.)

    @jessaustin
    Copy link
    Mannequin

    jessaustin mannequin commented Mar 25, 2009

    A comment on the two most recent patches... For both of these, we can
    do the following:

    >>> from datetime import timedelta
    >>> td = timedelta(12)
    >>> td
    datetime.timedelta(12)
    >>> td //= 3
    >>> td
    datetime.timedelta(4)
    >>> td //= timedelta(2)
    >>> td
    2          # CHANGED VARIABLE TYPE!

    I think the last operation will trap unsuspecting programmers, and
    provide no benefit for the savvy. There really is no reason to allow an
    in-place operation like this to change the type of the variable so
    drastically. (That is, I realize a similar thing could happen with ints
    and floats, but it seems worse with timedeltas and ints.) I feel the
    last operation should raise a TypeError, even though it would be quite
    valid for a non-in-place operation.

    @amauryfa
    Copy link
    Member

    Well, this already happen with other types:

    >>> a   = 100
    >>> a //= 2.0
    >>> a
    50.0
    
    >>> d  = datetime.datetime.now()
    >>> d -= datetime.datetime.now()
    >>> d
    datetime.timedelta(-1, 86391, 609000)

    See http://docs.python.org/reference/datamodel.html#object.\_\_iadd__
    "... return the result (which could be, but does not have to be, self) ..."

    @tleeuwenburggmailcom tleeuwenburggmailcom mannequin self-assigned this Apr 8, 2009
    @abalkin
    Copy link
    Member

    abalkin commented Apr 12, 2010

    Is there a good reason why this issue is languishing? The requested functionality seems to be well motivated, simple to implement with few objections resolved in the discussion.

    I wonder if it would be helpful to limit this patch to 3.x series. That way some of the controversies about true vs. floor division would disappear and it will be easier to accept new features.

    @bitdancer
    Copy link
    Member

    It's too late for 2.7 anyway.

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 19, 2010

    I am attaching a forward port of Victor's timedelta_true_divide_divmod.patch to py3k.

    @mdickinson
    Copy link
    Member

    Why is divmod(timedelta, timedelta) supported but not timedelta % timedelta? I think if one is implemented, the other should be too.

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 19, 2010

    On Mon, Apr 19, 2010 at 4:09 PM, Mark Dickinson <report@bugs.python.org>wrote:

    I noticed that as I was porting Victor's patch. I did not add timedelta %
    timedelta because I found out that timedelta % int is not supported in the
    released versions while timedelta // int is.

    I was contemplating opening a separate RFE for divmod(timedelta, int)
    and timedelta % int because I feel that increasing the number of added
    features will reduce the likelihood of patch being accepted. Personally, I
    only had need for timedelta // timedelta in my code and not any other
    proposed features.

    @mdickinson
    Copy link
    Member

    By the way, the patch looks good to me, as far as it goes, and I'm +1 on adding all this. I only have the tiniest of nits:

    • the patch deletes a line at the top of Lib/test/test_datetime.py, for no apparent reason

    • in delta_add, I think the added 'if' block should be an 'else if' (yes, I know this is excessively picky ;)

    • the brace style for if blocks is inconsistent: please put the opening brace on the same line as the if.

    One other thought: with this division operation added, it might be nice to add constants like td_hour, td_minute, etc. to the module. Then the perennial 'how can I convert my timedelta x to minutes' questions could be answered with 'just do x/td_minute'. I would personally find x/td_second to be a more obvious and natural way to find the total number of seconds in a timedelta than x.total_seconds. I also quite like the idea of being able to create a 2.5-hour timedelta with something like

    2*td_hour + 30*td_minute

    On the other hand, maybe such constants would just be added clutter, since it's easy to create them when needed.

    @mdickinson
    Copy link
    Member

    Tennessee, are you still tracking this issue? If not, can I steal it from you. :)

    I found out that timedelta % int is not supported in the
    released versions while timedelta // int is.

    Mmm. Interesting. :)

    I think it would be fine to add timedelta % timedelta in this patch, and then open another feature request for timedelta % int and friends as you suggest. I don't think divmod(timedelta, timedelta) should go in if timedelta % timedelta doesn't also go in.

    I feel that increasing the number of added
    features will reduce the likelihood of patch being accepted.

    Perhaps; I haven't seen much opposition to these ideas anyway---I think the only reason they haven't been implemented yet is lack of round-tuits.

    I'd be +1 on accepting the current patch if timedelta % timedelta were added to it.

    @mdickinson
    Copy link
    Member

    Hmm. Having timedelta // int work is *really* peculiar, since it can only be made sense of with reference to some implicit particular chosen unit of time; in this case, that unit of time is apparently microseconds, as far as I can tell.

    Surely there aren't any applications for timedelta // int?! The operation just doesn't make dimensional sense, since it involves taking floor of a timedelta.

    I vote -3.2 minutes on extending this craziness by adding timedelta % int or divmod(timedelta, int).

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 19, 2010

    I should be able to add timedelta % timedelta and fix the nits that Mark mentioned tonight.

    @tleeuwenburggmailcom
    Copy link
    Mannequin

    tleeuwenburggmailcom mannequin commented Apr 19, 2010

    On Tue, Apr 20, 2010 at 6:46 AM, Mark Dickinson <report@bugs.python.org>wrote:

    Mark Dickinson <dickinsm@gmail.com> added the comment:

    Tennessee, are you still tracking this issue? If not, can I steal it from
    you. :)

    Hi Mark,

    Please feel free to steal it from me! Wonderful to see some movement on this
    issue...

    > I found out that timedelta % int is not supported in the
    > released versions while timedelta // int is.

    Mmm. Interesting. :)

    I think it would be fine to add timedelta % timedelta in this patch, and
    then open another feature request for timedelta % int and friends as you
    suggest. I don't think divmod(timedelta, timedelta) should go in if
    timedelta % timedelta doesn't also go in.

    I'm an incrementalist -- I'm happy with one step at a time, so for what it's
    worth I'd be happy to see divmod go in alone with % to be added later.
    However, seeing as I'm really contributing, I'd say feel free to disregard
    my vote of +1... :)

    Regards,
    -Tennessee

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 20, 2010

    New patch, issue2706a.diff, implements timedelta % timedelta and addresses Mark's code review comments.

    With respect to Mark's
    """
    One other thought: with this division operation added, it might be nice to add constants like td_hour, td_minute, etc. to the module. Then the perennial 'how can I convert my timedelta x to minutes' questions could be answered with 'just do x/td_minute'. I would personally find x/td_second to be a more obvious and natural way to find the total number of seconds in a timedelta than x.total_seconds. I also quite like the idea of being able to create a 2.5-hour timedelta with something like

    2*td_hour + 30*td_minute

    On the other hand, maybe such constants would just be added clutter, since it's easy to create them when needed.
    """

    I dislike this proposal for the same reason as Mark likes it: 2*td_hour + 30*td_minute == timedelta(hours=2, minutes=30) is a violation of TOOWTDI.

    @mdickinson
    Copy link
    Member

    Stealing from Tennessee...

    Patch committed to py3k in r80290, r80291, with some minor tweaks and fixes:

    • rename delta_remailder to delta_remainder
    • fix memory leak in delta_remainder
    • add whatsnew entry for docs
    • note that timedelta // timedelta returns an integer in docs
    • add tests for raising ZeroDivisionError, and remove uses of
      lambda in favour of using the operator module

    Thanks, Alexander!

    @mdickinson mdickinson added the extension-modules C modules in the Modules dir label Apr 20, 2010
    @mdickinson
    Copy link
    Member

    Grr. s/whatsnew/versionadded/

    @AlexanderBelopolsky
    Copy link
    Mannequin

    AlexanderBelopolsky mannequin commented Apr 21, 2010

    Can someone (Mark?) add an acknowledgment for Victor Stinner to the NEWS file? My py3k patch was 90% code from Victor's trunk patch.

    @mdickinson
    Copy link
    Member

    Ah, yes. Sorry, Victor! There's actually no acknowledgement in Misc/NEWS: it's not *that* common to put acknowledgements there, and I'm not sure it's necessary here, but I've fixed the commit message.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants