classification
Title: Return a namedtuple from date.isocalendar()
Type: enhancement Stage: needs patch
Components: Library (Lib) Versions: Python 3.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: belopolsky Nosy List: belopolsky, berker.peksag, bmispelon, rhettinger, serhiy.storchaka, taleinat
Priority: normal Keywords: patch

Created on 2015-06-09 12:37 by bmispelon, last changed 2016-09-20 15:24 by taleinat.

Files
File name Uploaded Description Edit
issue24416_2.diff bmispelon, 2015-06-09 21:06 review
issue24416_3.diff bmispelon, 2015-06-09 21:32 review
issue24416_4.diff bmispelon, 2015-06-14 10:42 review
issue24416_5.diff bmispelon, 2016-09-20 08:02 review
Messages (17)
msg245061 - (view) Author: Baptiste Mispelon (bmispelon) * Date: 2015-06-09 12:37
Currently, `date.isocalendar()` returns a plain tuple of (year, week, weekday).

It would be a bit more useful if this tuple could be made into a namedtuple (with fields year, week and weekday).
msg245084 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2015-06-09 19:29
Is there any way that this could break existing code?
msg245085 - (view) Author: Baptiste Mispelon (bmispelon) * Date: 2015-06-09 19:31
As far as I know, using a namedtuple in place of a tuple is fully backwards-compatible.
msg245086 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-06-09 19:36
No, it is not fully backwards-compatible. First, if pickle a namedtuple, it can't be unpickled in previous versions. Second, namedtuple is slower and larger than tuple, so it shouldn't be used in memory or performance critical code.
msg245087 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2015-06-09 19:51
> First, if pickle a namedtuple, it can't be unpickled in previous versions.

True, but I don't think Python goes as far as to promise that objects pickled in one version can be unpickled in previous versions.

> Second, namedtuple is slower and larger than tuple, so it shouldn't be used in memory or performance critical code.

True, but I doubt that such tuples are often used extensively in performance-critical areas of code. Also, code which does care about this could immediately convert these namedtuples into plain tuples, which would be backwards compatible.
msg245088 - (view) Author: Baptiste Mispelon (bmispelon) * Date: 2015-06-09 19:52
I didn't know about issues with pickling. As for the performance issue, is date.isocalendar() really performance critical?

I found a precedent for replacing a tuple by a namedtuple: https://hg.python.org/cpython/rev/ef72142eb8a2

I'm trying my hand at writing a patch (I'm a new contributor) but it seems the code is implemented twice: both in Lib/ (python) and in Modules/ (C). I'm having a hard time figuring out how to conjure up a namedutple from the C code. Any hepl is appreciated.

Thanks.
msg245089 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2015-06-09 20:10
You can take a look at lru_cache_cache_info in Modules/_functoolsmodule.c for an example of namedtuple instantiation in C code. But that code gets the namedtuple class as a parameter.

This is not my area of expertise, but you could try using PyObject_CallFunction to call the Python collections.namedtuple function, keep the result as a module attribute, and then call that whenever you want to create an instance.
msg245092 - (view) Author: Baptiste Mispelon (bmispelon) * Date: 2015-06-09 21:06
Here's a second attempt at a patch which seems to work.

I don't know C at all but I used _decimal.c to try and figure out how to use namedtuple in C.

The code compiles and the datetime test suite runs.

I've added a test for the new feature and a note in Misc/NEWS as well as a versionadded note in the relevant documentation.

Let me know if I've missed anything.
msg245093 - (view) Author: Baptiste Mispelon (bmispelon) * Date: 2015-06-09 21:32
Some C code cleanups suggested by haypo.
msg245094 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2015-06-09 21:35
Now for the color of the bike-shed: What should the namedtuple returned by date.isocalendar() be named? Perhaps CalendarDate or DateTuple?

Baptiste's patches use ISOCalendarResult. In my opinion that is a poor name since it does not tell you anything about what it represents (you have to know or look up what isocalendar() returns).
msg245095 - (view) Author: Baptiste Mispelon (bmispelon) * Date: 2015-06-09 21:38
For the name, I took (un)inspiration from ParseResult: https://docs.python.org/3/library/urllib.parse.html?highlight=urlparse#urllib.parse.ParseResult

Any better suggestion is welcome of course.
msg245115 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2015-06-10 06:16
How about IsoCalendarDate?
msg245339 - (view) Author: Baptiste Mispelon (bmispelon) * Date: 2015-06-14 10:42
Here's a new patch with the name `IsoCalendarDate` used (it also fixes a wrong version in the `versionadded` section in the docs).

Thanks
msg276835 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-09-17 20:07
+1 This looks like a reasonable and useful API improvement.
msg276978 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-09-19 13:32
C code should generally use structseq instead of collections.namedtuple.  See the code for time.localtime() in Modules/timemodule.c.

Also, in the docs use "named tuple" as two words and link to the glossary: https://docs.python.org/3/glossary.html#term-named-tuple
msg277016 - (view) Author: Baptiste Mispelon (bmispelon) * Date: 2016-09-20 08:02
I updated the patch based on Raymond's feedback.

I don't know C at all and I tried to mimic the namedtuple usage of timemodule.c as much as I could (until the code compiled and the test suite passed).

I still have two questions:

* It seems that the Python implementation (Lib/datetime.py) is not tested when I run `./python -m test test_datetime` (I introduced a deliberate error in datetime.isocalendar() there but the test still succeeded). Is there a particular procedure to trigger those tests?

* I'm currently using the name `IsoCalendarDate` as suggested by user taleinat. However, shouldn't it be `ISOCalendarDate`?

Thanks.
msg277035 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2016-09-20 15:24
Regarding the name "IsoCalendarDate", see for example this question on Stack Overflow[1] where both of the leading answers suggest beginning with "Iso" or "iso" rather than "ISO". However, taking a look at the relatively new module urllib.request[2], it uses names such as "HTTPHandler" rather than "HttpHandler". I'll leave this up to someone more knowledgeable about the conventions to settle.

.. [1]: http://stackoverflow.com/questions/15526107/acronyms-in-camelcase
.. [2]: https://docs.python.org/3.5/library/urllib.request.html#module-urllib.request
History
Date User Action Args
2016-09-20 15:24:59taleinatsetmessages: + msg277035
2016-09-20 08:02:26bmispelonsetfiles: + issue24416_5.diff

messages: + msg277016
2016-09-19 13:32:16rhettingersetmessages: + msg276978
2016-09-17 20:07:35rhettingersetnosy: + rhettinger
messages: + msg276835
2016-09-16 18:57:08belopolskysetassignee: belopolsky

nosy: + belopolsky
versions: + Python 3.7, - Python 3.6
2015-06-14 10:42:58bmispelonsetfiles: + issue24416_4.diff

messages: + msg245339
2015-06-10 06:16:39taleinatsetmessages: + msg245115
2015-06-09 21:38:56bmispelonsetmessages: + msg245095
2015-06-09 21:35:32taleinatsetmessages: + msg245094
2015-06-09 21:32:48bmispelonsetfiles: + issue24416_3.diff

messages: + msg245093
2015-06-09 21:06:36bmispelonsetfiles: + issue24416_2.diff
keywords: + patch
messages: + msg245092
2015-06-09 20:10:39taleinatsetmessages: + msg245089
2015-06-09 19:52:40bmispelonsetmessages: + msg245088
2015-06-09 19:51:36taleinatsetmessages: + msg245087
2015-06-09 19:36:33serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg245086
2015-06-09 19:31:12bmispelonsetmessages: + msg245085
2015-06-09 19:29:06taleinatsetnosy: + taleinat
messages: + msg245084
2015-06-09 15:04:56berker.peksagsetnosy: + berker.peksag

type: enhancement
stage: needs patch
2015-06-09 12:37:20bmispeloncreate