[issue30302] Improve .__repr__ implementation for datetime.timedelta

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 ___

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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)

[issue30302] Improve .__repr__ implementation for datetime.timedelta

Changes by Utkarsh Upadhyay : -- keywords: +patch pull_requests: +3677 stage: -> patch review ___ Python tracker ___

[issue30302] Improve .__repr__ implementation for datetime.timedelta

Changes by Roundup Robot : -- pull_requests: +2992 ___ Python tracker ___

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

Changes by Utkarsh Upadhyay : -- pull_requests: +2981 ___ Python tracker ___ ___

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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,

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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,

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

Changes by Alexander Belopolsky : -- nosy: +tim.peters ___ Python tracker ___

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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)

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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,

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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!

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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.

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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.

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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,

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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.

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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

[issue30302] Improve .__repr__ implementation for datetime.timedelta

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