[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Berker Peksag]

Berker Peksag  added the comment:

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

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Berker Peksag]

Berker Peksag  added the comment:


New changeset 843ea47a034307c7b1ca642dd70f0269255b289a by Berker Peksag 
(Utkarsh Upadhyay) in branch 'master':
bpo-31545: Update documentation containing timedelta repr. (GH-3687)
https://github.com/python/cpython/commit/843ea47a034307c7b1ca642dd70f0269255b289a


--
nosy: +berker.peksag

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Changes by Utkarsh Upadhyay :


--
keywords: +patch
pull_requests: +3677
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Roundup Robot]

Changes by Roundup Robot :


--
pull_requests: +2992

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:


New changeset 8e45318b0d8df9340ac41b1d0447ffc83c7f5102 by Victor Stinner 
(Utkarsh Upadhyay) in branch 'master':
bpo-30302: Update WhatsNew and documentation. (#2929)
https://github.com/python/cpython/commit/8e45318b0d8df9340ac41b1d0447ffc83c7f5102


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Changes by Utkarsh Upadhyay :


--
pull_requests: +2981

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:


New changeset cc5a65cd9025280ea67ef4bbc2a8bfe31ced6c30 by Victor Stinner 
(Utkarsh Upadhyay) in branch 'master':
bpo-30302 Make timedelta.__repr__ more informative. (#1493)
https://github.com/python/cpython/commit/cc5a65cd9025280ea67ef4bbc2a8bfe31ced6c30


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

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, 10)

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

~
ut

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

> [...] 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=145066335824146=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=145073617109654=2

~
ut

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:

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!)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:

> 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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Alexander Belopolsky]

Changes by Alexander Belopolsky :


--
nosy: +tim.peters

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Alexander Belopolsky]

Alexander Belopolsky added the comment:

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=10) 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=10)
datetime.timedelta(3114, 28747, 10)
>>> print(_)
3114 days, 7:59:07.10

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> Isn’t there a “Unicode writer” API that could be used?

It could be used if accumulate only Python strings, C strings and separate 
characters. But in any case we need to convert C integers to strings, and add 
up to 3 items per every attribute (separator, name, value), the result of every 
addition should be checked. PyUnicode_FromFormat() is an easy way to 
concatenate several items. It needs only one check.

> The annoying part is to handle the ", " separator. I also had bad experiences 
> with handling char* strings. It's so easy to make mistakes :-(

It could be simpler if use the trick with the sep variable. As for mistakes, 
actually that version of Utkarsh's patch was less buggy than the few following 
versions using the Python C API. But the final C code is correct and LGTM.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Martin Panter]

Martin Panter added the comment:

Don’t let my minus sign suggestion stop this, especially since a couple other 
people said they don’t like it. The current pull request proposal is beneficial 
without it.

Isn’t there a “Unicode writer” API that could be used? Maybe that’s another 
alternative to the string accumulation or list building, but I never used it so 
it may not be appropriate.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

So, as a compromise, I'll stick to PyObjects (avoiding char* and the buffer 
size calculations) but will use PyUnicode all the way instead using PyList_* 
functions (thus reducing the size of the code by ~20 lines).

I've pushed this change, ready for review! :)

(haypo has already approved the change while I am still writing this.)

~
ut

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:

Serhiy: "The C code looks cumbersome. It could be made a little simpler if 
accumulate arguments into a string rather than a list."

Utkarsh's first version used C code. I proposed to use a PyList and 
PyUnicodeObject strings to not have to compute the size of the char[] buffers.

The annoying part is to handle the ", " separator. I also had bad experiences 
with handling char* strings. It's so easy to make mistakes :-(

IMHO the current C code using Python objects is not that complex, but I don't 
really care of the implementation as soon as it works :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Don't wait me. I have no preferences and just remind about the Martin's 
suggestion.

The C code looks cumbersome. It could be made a little simpler if accumulate 
arguments into a string rather than a list.

if (GET_TD_SECONDS(self) != 0) {
Py_SETREF(args, PyUnicode_FromFormat("%U%sseconds=%d",
 args, sep, GET_TD_SECONDS(self)));
if (args == NULL)
return NULL;
sep = ", ";
}

args is initialized by an empty Python string, sep is initialized by "". 
PyUnicode_Join() at the end is not needed. This would save more than 20 lines.

The code could be simplified even more if use the char buffer(s) and 
PyOS_snprintf instead of PyUnicode_FromFormat().

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

The docstring changes ought to be easier to review since I'll only be making 
the Python and C docstrings mostly identical (with some minor changes here and 
there). I'll also (hopefully) need fewer pointers now that I've been through 
one review process (thanks @haypo!).

To answer your question, the changes in this PR are completely orthogonal to 
the changes in the docstring needed. 

Hence, I'm counting one more vote towards making the docstring changes in a 
separate PR.

Thanks! :-)

~
ut

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[R. David Murray]

R. David Murray added the comment:

If the docstring changes incorporate changes from this PR, I'd keep them in a 
single PR myself.  If not, two PRs would make review easier.  On the other 
hand, if haypo is volunteering to do the review, do whatever he wants :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

@r.david.murray: I'm primarily waiting for Serhiy (and, optionally Martin) to 
"Okay" the pull request. 

The code seems fine (@haypo has had a through look at it), but we still were 
mildly divided over whether we want to factor out the negative sign or not.

re: thoughts and opinions; I wanted to have them on:

  - Whether to factor out the negative sign.
  - Whether to throw in the doc-string/documentation changes into this PR or in 
a new one.


Current position on these questions:

- In favor of factoring out the -ve sign: Martin
  Not in favor of factoring out the -ve sign: Victor, R. David


- DocString changes: (@haypo's opinion) in a separate PR.


~
ut

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[R. David Murray]

R. David Murray added the comment:

I'm not entirely sure what you are asking for opinions on, but for what it is 
worth I'm with Haypo: the repr should show the *actual* value of the attributes.

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

I went through the e-mail chain discussing the changes in repr again and I have 
come around to seeing that though Guido was somewhat opposed to the idea of 
factoring out the minus sign because it would have meant changing the 
attributes [1], what he really didn't want was perhaps adding attributes in 
repr which don't exist on the object [2]. Changing to keyword arguments was a 
'go', though [3].

Esp. after it has been pointed out how easy it would be do implement, my 
opposition to the idea of factoring out the negative sign has somewhat come 
down. Personally, I prefer the current version.

Further, this PR has also come to fix a couple of issues in the testing 
framework and it is becoming rather unwieldy to throw in doc-string changes 
into it as well. Hence, after some discussion with @haypo, I think I'll make a 
separate PR for fixing the docstrings after this PR is merged; there was 
general consensus that the docstrings should be fixed [4,5]. @haypo has, 
thankfully, showed me how and where to do that using Argument Clinic, which was 
the show-stopper the last time [6].

Thoughts and opinions?

~
ut

[1]: https://marc.info/?l=python-dev=145066335824146=2
[2]: https://marc.info/?l=python-dev=145066934224955=2
[3]: https://marc.info/?l=python-dev=145073617109654=2
[4]: https://marc.info/?l=python-dev=145073579109548=2
[5]: https://marc.info/?l=python-dev=145073612409626=2
[6]: https://marc.info/?l=python-dev=145075173412963=2

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:

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

I read your latest PR. Now I don't like seconds=0 anymore. I would prefer a 
datetime.timedelta(0) special case.

In the Python implementation, add:

if not args:
   args.append('0')

and remove the "not days and not microseconds" special case.

datetime.timedelta(0) was always valid, and IMHO it's explicitly enough.

I'm also ok with "datetime.timedelta()" alone, since it was also always 
accepted and is also well defined.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

@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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:

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 ;-))

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[STINNER Victor]

STINNER Victor added the comment:

> 2. 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?

--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

So we are okay with

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

and

datetime.timedelta(minutes=-1).seconds
# 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=145066934224955=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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Martin Panter]

Martin Panter added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Utkarsh Upadhyay]

Utkarsh Upadhyay added the comment:

> This was discussed fairly recently: 
> 

That thread went deep and culminated here, as far as I can tell: 
https://marc.info/?l=python-dev=145077422417470=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=145065347022557=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).


> 1. 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.

> 2. 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=145066934224955=2

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30302] Improve .__repr__ implementation for datetime.timedelta

[Martin Panter]

Martin Panter added the comment:

This was discussed fairly recently: 
.
 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:

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

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

--
nosy: +martin.panter
title: Improve .__repr__ implementation for datetime.datetime.timedelta -> 
Improve .__repr__ implementation for datetime.timedelta
versions:  -Python 2.7, Python 3.3, Python 3.4, Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com