Skip to content

bpo-30302 Make timedelta.__repr__ more informative. #1493

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

Merged
merged 26 commits into from
Jul 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1cb7b91
Make timedelta.__repr__ more informative.
musically-ut May 7, 2017
84241de
Make C impl of timedelta.__repr__ more informative
musically-ut May 7, 2017
2e61096
Fix tests.
musically-ut May 7, 2017
e6970c8
Add more tests.
musically-ut Jun 25, 2017
f5dd914
Do not show args which are 0.
musically-ut Jun 27, 2017
e1c33ab
Fix bug to run tests on both C and Python code.
musically-ut Jun 27, 2017
345557b
Use append/join instead of concatenating strings.
musically-ut Jun 27, 2017
e79d223
Remove unsed tests.
musically-ut Jun 27, 2017
312b81b
Use append/join in C implementation.
musically-ut Jun 27, 2017
1ca30ee
Remove documentation of `repr`.
musically-ut Jun 27, 2017
f643b40
Do stricter reference management, remove refleaks.
musically-ut Jun 27, 2017
194bfde
Inline simple expression.
musically-ut Jun 27, 2017
a40e0c1
Add checks for C-API failures.
musically-ut Jun 27, 2017
69213f7
Make DECREFs more concise.
musically-ut Jun 27, 2017
c939f81
Add more tests.
musically-ut Jun 29, 2017
9e7bd21
Update documentation for repr.
musically-ut Jun 29, 2017
863485c
timedelta(seconds=0) -> timedelta(0)
musically-ut Jun 29, 2017
5d5a92e
Make the code more DRY.
musically-ut Jun 29, 2017
16516a4
Remove empty lines and use idiomatic methods.
musically-ut Jun 29, 2017
2fa3e20
Revert white-space regression in documentation.
musically-ut Jun 29, 2017
10817a8
Update ACKS and NEWS.d.
musically-ut Jun 30, 2017
e26ab32
Use PyUnicode_* instead of PyList_*.
musically-ut Jun 30, 2017
83ea587
Make the invariant in case of repr explicit.
musically-ut Jun 30, 2017
23eb9ec
Place name at the correct alphabetical position.
musically-ut Jul 6, 2017
ae93f8e
Update documentation.
musically-ut Jul 6, 2017
3d327f4
char * -> const char*
musically-ut Jul 6, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions Doc/library/datetime.rst
Original file line number Diff line number Diff line change
Expand Up @@ -287,11 +287,12 @@ Supported operations:
| | ``[D day[s], ][H]H:MM:SS[.UUUUUU]``, where D |
| | is negative for negative ``t``. (5) |
+--------------------------------+-----------------------------------------------+
| ``repr(t)`` | Returns a string in the form |
| | ``datetime.timedelta(D[, S[, U]])``, where D |
| | is negative for negative ``t``. (5) |
| ``repr(t)`` | Returns a string representation of the |
| | :class:`timedelta` object as a constructor |
| | call with canonical attribute values. |
+--------------------------------+-----------------------------------------------+


Notes:

(1)
Expand Down
22 changes: 10 additions & 12 deletions Lib/datetime.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,20 +454,18 @@ def __new__(cls, days=0, seconds=0, microseconds=0,
return self

def __repr__(self):
if self._microseconds:
return "%s.%s(%d, %d, %d)" % (self.__class__.__module__,
self.__class__.__qualname__,
self._days,
self._seconds,
self._microseconds)
args = []
if self._days:
args.append("days=%d" % self._days)
if self._seconds:
return "%s.%s(%d, %d)" % (self.__class__.__module__,
self.__class__.__qualname__,
self._days,
self._seconds)
return "%s.%s(%d)" % (self.__class__.__module__,
args.append("seconds=%d" % self._seconds)
if self._microseconds:
args.append("microseconds=%d" % self._microseconds)
if not args:
args.append('0')
return "%s.%s(%s)" % (self.__class__.__module__,
self.__class__.__qualname__,
self._days)
', '.join(args))

def __str__(self):
mm, ss = divmod(self._seconds, 60)
Expand Down
16 changes: 13 additions & 3 deletions Lib/test/datetimetester.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,11 +658,21 @@ def test_str(self):
def test_repr(self):
name = 'datetime.' + self.theclass.__name__
self.assertEqual(repr(self.theclass(1)),
"%s(1)" % name)
"%s(days=1)" % name)
self.assertEqual(repr(self.theclass(10, 2)),
"%s(10, 2)" % name)
"%s(days=10, seconds=2)" % name)
self.assertEqual(repr(self.theclass(-10, 2, 400000)),
"%s(-10, 2, 400000)" % name)
"%s(days=-10, seconds=2, microseconds=400000)" % name)
self.assertEqual(repr(self.theclass(seconds=60)),
"%s(seconds=60)" % name)
self.assertEqual(repr(self.theclass()),
"%s(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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that it's worth it to add so many test cases. Just keep 3 examples, 1 negative value, the timedelta(0), and you're done no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the tests were added to differentiate the current implementation from the old one: i.e. no leading days=0 or seconds=0 and to highlight the current handling of negative values (i.e. the normalization of the attributes, as described in the docs). I'll remove the ones which don't test anything unique not covered elsewhere.


def test_roundtrip(self):
for td in (timedelta(days=999999999, hours=23, minutes=59,
Expand Down
2 changes: 2 additions & 0 deletions Lib/test/test_datetime.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ def tearDownClass(cls_):
cls.tearDownClass = tearDownClass
all_test_classes.extend(test_classes)

all_test_classes.extend(test_classes)

def test_main():
run_unittest(*all_test_classes)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use keywords in the ``repr`` of ``datetime.timedelta``.
59 changes: 44 additions & 15 deletions Modules/_datetimemodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2284,21 +2284,50 @@ delta_bool(PyDateTime_Delta *self)
static PyObject *
delta_repr(PyDateTime_Delta *self)
{
if (GET_TD_MICROSECONDS(self) != 0)
return PyUnicode_FromFormat("%s(%d, %d, %d)",
Py_TYPE(self)->tp_name,
GET_TD_DAYS(self),
GET_TD_SECONDS(self),
GET_TD_MICROSECONDS(self));
if (GET_TD_SECONDS(self) != 0)
return PyUnicode_FromFormat("%s(%d, %d)",
Py_TYPE(self)->tp_name,
GET_TD_DAYS(self),
GET_TD_SECONDS(self));

return PyUnicode_FromFormat("%s(%d)",
Py_TYPE(self)->tp_name,
GET_TD_DAYS(self));
PyObject *args = PyUnicode_FromString("");

if (args == NULL) {
return NULL;
}

const char *sep = "";

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

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 = ", ";
}

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

if (PyUnicode_GET_LENGTH(args) == 0) {
Py_SETREF(args, PyUnicode_FromString("0"));
if (args == NULL) {
return NULL;
}
}

PyObject *repr = PyUnicode_FromFormat("%s(%S)", Py_TYPE(self)->tp_name,
args);
Py_DECREF(args);
return repr;
}

static PyObject *
Expand Down