Improve .__repr__ implementation for datetime.timedelta

@tim-one

BPO	30302
Nosy	@tim-one, @abalkin, @vstinner, @bitdancer, @berkerpeksag, @vadmium, @serhiy-storchaka, @musically-ut
PRs	bpo-30302 Make timedelta.__repr__ more informative. #1493 bpo-30302: Update WhatsNew and documentation. #2929 Merge pull request #1 from python/master #2943 bpo-31545 Update documentation containing timedeltas repr. #3687

^{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 = None
closed_at = <Date 2017-10-27.11:28:20.761>
created_at = <Date 2017-05-07.19:57:26.207>
labels = ['3.7', 'type-feature', 'library']
title = 'Improve .__repr__ implementation for datetime.timedelta'
updated_at = <Date 2017-10-27.11:28:20.759>
user = 'https://github.com/musically-ut'

bugs.python.org fields:

activity = <Date 2017-10-27.11:28:20.759>
actor = 'berker.peksag'
assignee = 'none'
closed = True
closed_date = <Date 2017-10-27.11:28:20.761>
closer = 'berker.peksag'
components = ['Library (Lib)']
creation = <Date 2017-05-07.19:57:26.207>
creator = 'musically_ut'
dependencies = []
files = []
hgrepos = []
issue_num = 30302
keywords = ['patch']
message_count = 35.0
messages = ['293212', '293213', '293215', '293216', '293219', '294062', '294106', '297029', '297048', '297049', '297050', '297269', '297293', '297424', '297429', '297438', '297440', '297441', '297444', '297447', '297457', '297471', '297521', '297531', '297560', '297561', '297570', '297865', '299148', '299149', '299152', '299381', '299384', '305111', '305113']
nosy_count = 8.0
nosy_names = ['tim.peters', 'belopolsky', 'vstinner', 'r.david.murray', 'berker.peksag', 'martin.panter', 'serhiy.storchaka', 'musically_ut']
pr_nums = ['1493', '2929', '2943', '3687']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue30302'
versions = ['Python 3.7']

Currently, the default implementation of datetime.datetime.__repr__ (the default output string produced at the console/IPython) gives a rather cryptic output:

from datetime import datetime as D
D.fromtimestamp(1390953543.1) - D.fromtimestamp(1121871596)
# datetime.timedelta(3114, 28747, 100000)

For the uninitiated, it is not obvious that the numeric values here are days, seconds and microsecond respectively.

Would there be any pushback against changing this to:

# datetime.timedelta(days=3114, seconds=28747, microseconds=100000)

?

The drawback is that this change increases the length of the repr. If you output few values in a row (for example output the repr of a list of timedeltas), this makes the output less readable.

Users of datetime.timedelta know what arguments mean. If they don't know they always can look in the documentation or builtin help.

datetime.datetime has more arguments, and its repr doesn't use keywords.

The drawback is that this change increases the length of the repr.

I would argue that it is a reasonable trade-off given the increase in ease of understanding.

I know that this is a weak argument, but, keywords are not without precedent. Consider the comically more verbose example:

import time
time.gmtime(1121871596)
# time.struct_time(tm_year=2005, tm_mon=7, tm_mday=20, tm_hour=14, tm_min=59, tm_sec=56, tm_wday=2, tm_yday=201, tm_isdst=0)

datetime.datetime has more arguments, and its repr doesn't use keywords.

I think that guessing the meaning of values is much harder when it comes to timedelta.

Users of datetime.timedelta know what arguments mean. If they don't know they always can look in the documentation or builtin help.

I created the issue after ... a friend ... spent an embarrassing amount of time debugging because he thought that the third argument represented milliseconds and not microseconds. <_<

I could, of course, tell him:

In the face of ambiguity, resist the temptation to guess.

But he could retort:

Explicit is better than implicit.

and

Readability counts.

I think he has a point.

This was discussed fairly recently: <https://marc.info/?i=CAPTjJmrBxpvYQUYXshBC1J13M_h5or67CNBKrkySw4ef6RqDoQ@mail.gmail.com\>. There seems to be a bit of support for changing this. It is not a bug fix, so has to go into the next release, now 3.7.

I disagree that the positional timedelta parameters are well-known.

The built-in help was also discussed in that thread. I don’t think it got fixed yet, did it?

The datetime class represents absolute dates. It is nonsense to specify a date without a year. Timedelta is different, because time is measured with different units. The first parameter (days) is surprising considering most other Python APIs (sleep etc) deal with seconds.

The size of the repr could be reduced a bit by dropping the module name: datetime.timedelta vs just timedelta. Although that would be inconsistent with the other classes; I’m not sure about this.

Other improvements that I would like to see:

Drop leading zeros: timedelta(seconds=60) rather than timedelta(days=0, seconds=60). This would also help reduce the size.
Factor out the negative sign: -timedelta(seconds=60) rather than timedelta(days=-1, seconds=86340).

This was discussed fairly recently: <https://marc.info/?i=CAPTjJmrBxpvYQUYXshBC1J13M_h5or67CNBKrkySw4ef6RqDoQ@mail.gmail.com\>

That thread went deep and culminated here, as far as I can tell: https://marc.info/?l=python-dev&m=145077422417470&w=2 (I may not have explored all the branches, though.)

So there indeed seems to be general agreement about changing this. It was heartening to know that I wasn't the only one to stumble. \o/

The built-in help was also discussed in that thread. I don’t think it got fixed yet, did it?

No, the doc-string is as uninformative as then, as far as I can tell:

In [178]: datetime.timedelta?
Init signature: datetime.timedelta(self, /, *args, **kwargs)
Docstring: Difference between two datetime values.
File: ~/miniconda3/lib/python3.5/datetime.py
Type: type

I'll investigate what documentation for other functions looks like and see if I can come up with something better. The exact documentation would be best discussed over diffs on Github.

Then there is the issue of repr being explicitly documented, as you had pointed out on the Github issue. Guido thinks that any breakage is _unlikely_ but "asked around" here: https://marc.info/?l=python-dev&m=145065347022557&w=2

As far as I can tell, he didn't see an explicit response.

The size of the repr could be reduced a bit by dropping the module name: datetime.timedelta vs just timedelta. Although that would be inconsistent with the other classes; I’m not sure about this.

Personally, I don't see a big problem either way but having datetime.timedelta in the repr feels reassuring to me that I have the 'right' type instead of some other 'timedelta' from a non-stdlib module (e.g. moments or pandas).

Drop leading zeros: timedelta(seconds=60) rather than timedelta(days=0, seconds=60). This would also help reduce the size.

This sounds reasonable. I'll make this change and add corresponding tests.

Factor out the negative sign: -timedelta(seconds=60) rather than timedelta(days=-1, seconds=86340).

I'm not sure there was consensus about this; if I understand correctly, Guido thinks that it is important that the repr return what the attributes hold: https://marc.info/?l=python-dev&m=145066934224955&w=2

I think the benefit of the repr being easier to understand outweighs the pain of breaking the old format. If the change is a problem, that might be mitigated by adding an entry to the “Porting to Python 3.7” documentation.

I don’t think my option of factoring the minus sign out to the front of the timedelta constructor was mentioned on Python-dev. The advantage is that it doesn’t mention problematic negative attribute values, and if you negate a timedelta, you still get the right attribute values:

>>> d
-datetime.timedelta(seconds=60)
>>> -d
datetime.timedelta(seconds=60)
>>> (-d).seconds
60

So we are okay with

datetime.timedelta(minutes=-1)
# -datetime.timedelta(seconds=60)

and

datetime.timedelta(minutes=-1).seconds
\bpo-86340

datetime.timedelta(minutes=-1).days
# -1

?

Perhaps I'm reading this incorrectly:

"If the repr() shows different numbers than the attributes things are worse than now." ~ Guido, https://marc.info/?l=python-dev&m=145066934224955&w=2

Maybe he was only talking about:

datetime.timedelta(minutes=-1)
# datetime.timedelta(minutes=-1)

but I would like a second opinion on it. :)

Also, I'll just drop the description of .repr from the .RST documentation because it is not to be relied on anyway. Any objections to that?

~
ut

I've added the following tests to remove the 0 attributes from the repr:

    self.assertEqual(repr(self.theclass(days=1, seconds=0)),
                     "%s(days=1)" % name)
    self.assertEqual(repr(self.theclass(seconds=60)),
                     "%s(seconds=60)" % name)
    self.assertEqual(repr(self.theclass()),
                     "%s(seconds=0)" % name)
    self.assertEqual(repr(self.theclass(microseconds=100)),
                     "%s(microseconds=100)" % name)
    self.assertEqual(repr(self.theclass(days=1, microseconds=100)),
                     "%s(days=1, microseconds=100)" % name)
    self.assertEqual(repr(self.theclass(seconds=1, microseconds=100)),
                     "%s(seconds=1, microseconds=100)" % name)

I am still ambivalent about factoring the minus sign outside, though.

I've also fixed a bug in test_datetime.py which prevented execution of the Python implementation in Lib/datetime.py.

TODO:

Decide about factoring the minus sign.
Drop description of timedelta.__repr__ from the documentation.

~
ut

Factor out the negative sign: -timedelta(seconds=60) rather than timedelta(days=-1, seconds=86340).

Please don't do that. repr() is designed for developers, not end users. Don't make repr() implementation overcomplicated. Just dump raw data, tha's all.

If you want something more smart, use str() or even write a *new* method?

What is your favorite repr() for datetime.timedelta(seconds=0)?

Since it's hard to decide which unit is the best, I suggest to not write any unit: either "datetime.timedelta(0)" or "datetime.timedelta()" is fine with me.

In fact, "datetime.timedelta(seconds=0)" is also ok, since seconds is the most common unit, no?

(Ok, to be honest, I don't really care. But I wanted to have a discussion on this issue ;-))

@Haypo

@Haypo: thanks for the input!

I will find datetime.timedelta() to be rather confusing because it does not explicitly tell me that the interval indeed is 0.

Between datetime.timedelta(0) and datetime.timedelta(seconds=0), I am ambivalent but I prefer the latter for consistency with the other repr.

Sorry for being late to the discussion, but please allow me to add a -1 vote. The time.struct_time precedent is indeed comically verbose. Whenever I need to inspect a struct_time, I cast it to a plain tuple

Compare

>>> time.gmtime(1121871596)
time.struct_time(tm_year=2005, tm_mon=7, tm_mday=20, tm_hour=14, tm_min=59, tm_sec=56, tm_wday=2, tm_yday=201, tm_isdst=0)

and

>>> time.gmtime(1121871596)[:]
(2005, 7, 20, 14, 59, 56, 2, 201, 0)

Unless you need to know what the last three fields are, the long form gives no advantage.

datetime.timedelta(days=3114, seconds=28747, microseconds=100000) is not as verbose and the extra information may be helpful the first time you see it, but if you deal with timedeltas a lot, it would quickly become annoying. Moreover, unlike in the struct_time case, there will be no easy way to suppress metadata.

Furthermore, "seconds=28747" is not that user-friendly. A friendlier representation would be "hours=7, minutes=59, seconds=7" and similar information is displayed when you print a timedelta:

>>> datetime.timedelta(days=3114, seconds=28747, microseconds=100000)
datetime.timedelta(3114, 28747, 100000)
>>> print(_)
3114 days, 7:59:07.100000

I don't have very strong preferences, but I concur with Alexander. His arguments match my initial opinion about this issue (msg293213).

Furthermore, "seconds=28747" is not that user-friendly. A friendlier representation would be "hours=7, minutes=59, seconds=7" and similar information is displayed when you print a timedelta: (...)

I agree that seconds=28747 is not that user-friendly, *but* maybe it shows a flaw in timedelta design?

Maybe timedelta should only expose properties which would *compute* hours, minutes, etc. from an internal storage? But if we change timedelta.seconds value, it is likely to break the backward compatibility. I wrote my own total_seconds() function which uses days, seconds and microseconds fields.

Or maybe we need a new method to convert a timedelta into a more human friendly (named)tuple?

Alexander: "Sorry for being late to the discussion, but please allow me to add a -1 vote."

While I agree that seconds=28747 is not perfect, IMHO "datetime.timedelta(seconds=28747)" is an enhancement over "datetime.timedelta(0, 28747)".

Can't we make small enhanements step by step, rather than each time try to redesign datetime? ;-)

(Yeah, datetime is now perfect, but well, it's now widely used and part of the stdlib!)

@Haypo

[...] the extra information may be helpful the first time you see it, but if you deal with timedeltas a lot, it would quickly become annoying.

It seems from the discussion on the issue that there are two kinds of developers:

One group which uses timedeltas so often that they will find it annoying if the repr was longer, which takes up more screen space.
The other group which occasionally uses timedelta but may forget what the arguments stood for.

I suspect that the distribution of programmers who use timedeltas binned by their frequency of usage will follow the Pareto principle: only 20% of the developers will account for 80% of uses of timedelta, and the remaining 80% of the developers will use it the remaining 20% of the time. Out of those 80% developers who use it sparingly, some fraction will find the repr with the keywords more informative than the current version and be a tiny bit happier.

I personally belong to the second group of developers. IMHO, Guido too may belong the second group https://marc.info/?l=python-dev&m=145066335824146&w=2 :

Well it would have saved me an embarrassing moment -- I typed datetime.timedelta(seconds=1e6) at the command prompt and when the response came as datetime.timedelta(11, 49600) I mistook that as 11 years (I was in a hurry and trying hard not to have to think :-).

@Haypo: in the same mail, Guido also talks (jokes?) about improving timedeltas from grounds up:

I might still go for it [i.e. changing the attributes], if it wasn't too late by over a decade (as Tim says).

:)

Though I think the discussion there stopped before anyone could raise the objection about verbosity and clarity, Guido, IMO, was optimistic about this change:

I still think the repr change to use keywords has a good chance for 3.6.

https://marc.info/?l=python-dev&m=145073617109654&w=2

~
ut

Bump!

----

>>> time.gmtime(1121871596)[:]
(2005, 7, 20, 14, 59, 56, 2, 201, 0)

I wouldn't mind implementing __getitem__ for timedeltas, such that this becomes possible:

> (D.fromtimestamp(1390953543.1) - D.fromtimestamp(1121871596))[:]
(3114, 28747, 100000)

(Though, TBH, it does feel a little weird that time.struct_time allows indexing).

~
ut

This issue has a long history starting in 2015 with a thread on python-dev. Many messages were written on python-dev and on this issue. I will try to summarize. (I hate doing that, summarizing is a risk of missing important information or misunderstand someone.)

The issue opens multiple questions:

Should we change repr?
** YES, Guido: "I still think the repr change to use keywords has a good chance for 3.6." and "Still, I can't be the only one to ever have been fooled by this, and it is definitely pretty arcane knowledge what the positional arguments to timedelta()."
** YES, Martin Panter: "I disagree that the positional timedelta parameters are well-known."
** YES, Victor Stinner
** NO, Tim Peter: "But I wouldn't change repr() - the internal representation is fully documented, and it's appropriate for repr() to reflect documented internals as directly as possible."
** NO, Serhiy: "The drawback is that this change increases the length of the repr. Users of datetime.timedelta know what arguments mean. If they don't know they always can look in the documentation or builtin help."
** NO, Alexander Belopolsky: msg297521
Can changing repr break code?
** Guido: when we randomized hash, a lot of tests failed
Add a "-" prefix for negative result: -timedelta(seconds=1)?
(Related) Should repr(timedelta(seconds=60)) returns 'timedelta(minutes=1)'?
** NO, Guido: "I'm sure that one often catches people by surprise. However, I don't think we can fix that one without also fixing the values of the attributes -- in that example days is -1 and seconds is 86340 (which will *also* catch people by surprise). And changing that would be much, much harder for backwards compatibility reasons-- we'd have to set days to 0 and seconds to -60, and suddenly we have a much murkier invariant, instead of the crisp"
** NO, Guido: "Then please just trust me. If the repr() shows different numbers than the attributes things are worse than now. People will casually look at the repr() and assume they've seen what the attributes will return, and spend hours debugging code that relies on that incorrect assumption."
** NO, R. David Murray : "I'm with Haypo: the repr should show the *actual* value of the attributes."
** NO, Victor Stinner: "Don't make repr() implementation overcomplicated (...) use str()"

Another remark of Alexander Belopolsky:

"Furthermore, "seconds=28747" is not that user-friendly. A friendlier representation would be "hours=7, minutes=59, seconds=7" and similar information is displayed when you print a timedelta: (...)"
** IHMO you should use str() and not repr() to format a value for humans. repr() is more designed for developers, to debug.

Another interesting remark: the documentation of the constructor is incomplete and should be document parameters.

New changeset cc5a65c by Victor Stinner (Utkarsh Upadhyay) in branch 'master':
bpo-30302 Make timedelta.__repr__ more informative. (bpo-1493)
cc5a65c

Do you think that repr(datetime.timedelta) should be documented in the Porting section of What's New in Python 3.7?

https://docs.python.org/dev/whatsnew/3.7.html#porting-to-python-3-7

It was proposed in the middle of the python-dev thread. I have no opinion on that... or just a little -0 vote, if someone is impacted, it should be easy to notice it anymore, no?

--

While we don't have an obvious consensus, I took the responsability of merging Utkarsh Upadhyay's contribution. He wrote that he is interested to pursue the effort by also enhancing the docstring: please, go ahead, write a new PR, I will help you!

It was discussed to rewrite datetime.timedelta: only accept keywords in its constructor, store seconds=60 as minutes=1, etc. The problem is that it's too late to change the design. So I suggest to not waste time on discussing that, but focus the energy on making tiny changes to enhance the implemenatation without risking of breaking the world.

While developers using regulary timedelta knows what means the 3 numbers, it seems clear that the other developers have to check for the documentation. IMHO having to check the documentation is an issue: adding keywords to repr() reduces the need of documentation, and so makes Python more usable.

Sorry about the longer repr(): use str() or write your own formater?

By the way, if you need a formatter working on Python 2.7-3.7:

>>> def format_td(td): return 'timedelta(%s, %s, %s)' % (td.days, td.seconds, td.microseconds)
... 
>>> format_td(datetime.timedelta(seconds=1))
'timedelta(0, 1, 0)'

If someone disagree with my merge, I suggest to reopen a thread on python-dev.

Thanks for the merge haypo! \o/

I've also created a PR for adding an entry to 'Porting to Python 3.7' in the documentation; I see no harm in including it in the documentation just-in-case.

New changeset 8e45318 by Victor Stinner (Utkarsh Upadhyay) in branch 'master':
bpo-30302: Update WhatsNew and documentation. (bpo-2929)
8e45318

New changeset 843ea47 by Berker Peksag (Utkarsh Upadhyay) in branch 'master':
bpo-31545: Update documentation containing timedelta repr. (GH-3687)
843ea47

All PRs have been merged in both issues (this and bpo-31545) so I guess this issue can be closed now.

Navigation Menu

Repository navigation

Improve .repr implementation for datetime.timedelta #74488

11 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Improve .__repr__ implementation for datetime.timedelta #74488

Description

Activity

musically-ut commented on May 7, 2017

serhiy-storchaka commented on May 7, 2017

musically-ut commented on May 7, 2017

vadmium commented on May 7, 2017

musically-ut commented on May 8, 2017

vadmium commented on May 21, 2017

musically-ut commented on May 21, 2017

musically-ut commented on Jun 27, 2017

vstinner commented on Jun 27, 2017

vstinner commented on Jun 27, 2017

musically-ut commented on Jun 27, 2017

11 remaining items

abalkin commented on Jul 2, 2017

serhiy-storchaka commented on Jul 3, 2017

vstinner commented on Jul 3, 2017

vstinner commented on Jul 3, 2017

musically-ut commented on Jul 3, 2017

musically-ut commented on Jul 7, 2017

vstinner commented on Jul 25, 2017

vstinner commented on Jul 25, 2017

vstinner commented on Jul 25, 2017

musically-ut commented on Jul 28, 2017

vstinner commented on Jul 28, 2017

berkerpeksag commented on Oct 27, 2017

berkerpeksag commented on Oct 27, 2017

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Improve .repr implementation for datetime.timedelta #74488