classification
Title: Use a named tuple for sys.version_info
Type: behavior Stage: needs patch
Components: Extension Modules Versions: Python 3.1, Python 2.7
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: brett.cannon, christian.heimes, eric.smith, exarkun, loewis, ncoghlan, rhettinger, ross.light
Priority: low Keywords: easy, patch

Created on 2008-11-09 00:04 by brett.cannon, last changed 2009-02-06 01:33 by eric.smith. This issue is now closed.

Files
File name Uploaded Description Edit
patch-4285b.diff ross.light, 2009-01-25 19:25 Second draft of bug patch
patch-4285c.diff ross.light, 2009-01-25 20:22 Third draft of patch
patch-4285d.diff ross.light, 2009-01-26 23:13 Fourth draft of patch
Messages (16)
msg75647 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2008-11-09 00:04
sys.version_info is just asking for a named tuple consisting of major,
minor, micro, releaselevel, and serial.

This is assuming, of course, that bootstrapping doesn't get in the way.
msg75655 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-11-09 17:35
I concur that bootstrapping may be a problem. Using a NamedTuple also
increases the number of loaded modules by 4 (_collections.so, keyword.py
and operator.so).

But we could reimplement it with a PyStructSequence like I did for
sys.float_info. It's straight forward and easy to implement with the
example code in Object/floatobject.c:PyFloat_GetInfo().
msg80521 - (view) Author: Ross Light (ross.light) * Date: 2009-01-25 18:37
Hello, my name is Ross Light.  I've written a patch for this, but this
is my first patch, so someone please review.

This does pass all regression tests, but I did have to modify the
test_sys case to not check for sys.version_info being a tuple.
msg80526 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2009-01-25 19:18
You also need to add unit tests for the new behavior you've implemented.
msg80527 - (view) Author: Ross Light (ross.light) * Date: 2009-01-25 19:25
Oh yes, you're right.  Sorry!
msg80530 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-01-25 19:29
+1 on this idea
msg80534 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-01-25 19:40
A couple of further comments:
- please use tabs for indentation consistently.
- please change Doc/library/sys.rst, including adding a versionchanged
indication
- I find the naming of the macros *Flag confusing; to me, a "flag" says
"boolean" - but I'm not a native speaker (of English). Common names are
"item", "field", "element".
msg80538 - (view) Author: Ross Light (ross.light) * Date: 2009-01-25 20:22
Okay, here's a patch with the requested changes.  You're right in saying
that flag is usually boolean, I was just going along with several other
files where there's float and int flags (i.e. floatobject.c).
msg80557 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2009-01-26 01:42
Rather than deleting the isinstance() check from the tests completely, I
suggest changing it to be:

self.assert_(isinstance(vi[:], tuple))

Also, comparing directly with a tuple is also a fairly common use of
version_info so it would be worth adding a test to explicitly guarantee
that comparison:

self.assert_(vi > (1,0,0))

Patch applied and built cleanly for me, but I haven't checked the doc
build yet.
msg80599 - (view) Author: Ross Light (ross.light) * Date: 2009-01-26 23:13
Tests added and new patch uploaded.  Anything else, anyone?
msg81139 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2009-02-04 13:56
The doc string for sys includes:
version_info -- version information as a tuple

I'm not sure changing this to "... as a structseq" makes it any more
useful, but it's more correct. Does anyone have a preference? I'd use
the same wording as float_info, but that's missing from the doc string
(and I'll deal with that as a separate issue).

Other than that, this all looks good to me. I also tested that the docs
build. I'll check it in once I get or invent new wording for the doc string.
msg81148 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2009-02-04 18:27
On Wed, Feb 4, 2009 at 05:56, Eric Smith <report@bugs.python.org> wrote:
>
> Eric Smith <eric@trueblade.com> added the comment:
>
> The doc string for sys includes:
> version_info -- version information as a tuple
>
> I'm not sure changing this to "... as a structseq" makes it any more
> useful, but it's more correct. Does anyone have a preference?

Does "... as a named tuple" make sense?

-Brett
msg81149 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2009-02-04 18:30
"... as a named tuple" works for me. I'll go with that. Thanks!
msg81169 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2009-02-04 22:31
Technically it's not a named tuple. Calling it named tuple may cause
confusing. http://docs.python.org/library/os.html#os.stat calls it a
structure.
msg81172 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2009-02-04 22:33
> Eric Smith <eric@trueblade.com> added the comment:
> "... as a named tuple" works for me. I'll go with that. Thanks!

+1 

Remember, "named tuple" is a concept, not a class.  It is anything that
provides attribute access as an alternative to indexed access (see the
definition in the glossary where time.struct_time is given as an
example).  Running the collections.named_tuple() factory function
creates a new class with named tuple features, but it is just one of
several ways of creating named tuples.
msg81245 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2009-02-06 01:33
Committed in r69331 (trunk) and r69346 (py3k).
History
Date User Action Args
2009-02-06 01:33:27eric.smithsetstatus: open -> closed
resolution: accepted
messages: + msg81245
2009-02-04 22:33:50rhettingersetmessages: + msg81172
2009-02-04 22:31:10christian.heimessetmessages: + msg81169
2009-02-04 18:30:20eric.smithsetmessages: + msg81149
2009-02-04 18:27:30brett.cannonsetmessages: + msg81148
2009-02-04 13:56:31eric.smithsetmessages: + msg81139
2009-02-03 21:56:46eric.smithsetassignee: brett.cannon -> eric.smith
2009-02-03 21:23:22eric.smithsetnosy: + eric.smith
2009-01-26 23:13:29ross.lightsetfiles: + patch-4285d.diff
messages: + msg80599
2009-01-26 01:42:26ncoghlansetnosy: + ncoghlan
messages: + msg80557
2009-01-25 20:22:41ross.lightsetfiles: + patch-4285c.diff
messages: + msg80538
2009-01-25 19:40:48loewissetnosy: + loewis
messages: + msg80534
2009-01-25 19:33:34loewissetfiles: - patch-4285a.diff
2009-01-25 19:29:44rhettingersetnosy: + rhettinger
messages: + msg80530
2009-01-25 19:25:21ross.lightsetfiles: + patch-4285b.diff
messages: + msg80527
2009-01-25 19:18:14exarkunsetnosy: + exarkun
messages: + msg80526
2009-01-25 18:37:14ross.lightsetfiles: + patch-4285a.diff
nosy: + ross.light
messages: + msg80521
keywords: + patch
2008-11-09 17:35:04christian.heimessetnosy: + christian.heimes
messages: + msg75655
2008-11-09 00:04:51brett.cannoncreate