Use a named tuple for sys.version_info

[brettcannon]

BPO	4285
Nosy	@loewis, @brettcannon, @rhettinger, @ncoghlan, @ericvsmith, @tiran
Files	patch-4285b.diff: Second draft of bug patch patch-4285c.diff: Third draft of patch patch-4285d.diff: Fourth draft of patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ericvsmith'
closed_at = <Date 2009-02-06.01:33:27.177>
created_at = <Date 2008-11-09.00:04:51.883>
labels = ['extension-modules', 'easy', 'type-bug']
title = 'Use a named tuple for sys.version_info'
updated_at = <Date 2009-02-06.01:33:27.176>
user = 'https://github.com/brettcannon'

bugs.python.org fields:

activity = <Date 2009-02-06.01:33:27.176>
actor = 'eric.smith'
assignee = 'eric.smith'
closed = True
closed_date = <Date 2009-02-06.01:33:27.177>
closer = 'eric.smith'
components = ['Extension Modules']
creation = <Date 2008-11-09.00:04:51.883>
creator = 'brett.cannon'
dependencies = []
files = ['12857', '12860', '12870']
hgrepos = []
issue_num = 4285
keywords = ['patch', 'easy']
message_count = 16.0
messages = ['75647', '75655', '80521', '80526', '80527', '80530', '80534', '80538', '80557', '80599', '81139', '81148', '81149', '81169', '81172', '81245']
nosy_count = 8.0
nosy_names = ['loewis', 'brett.cannon', 'rhettinger', 'exarkun', 'ncoghlan', 'eric.smith', 'christian.heimes', 'ross.light']
pr_nums = []
priority = 'low'
resolution = 'accepted'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue4285'
versions = ['Python 3.1', 'Python 2.7']

[brettcannon]

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.

[tiran]

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().

[rosslight]

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.

[exarkun]

You also need to add unit tests for the new behavior you've implemented.

[rosslight]

Oh yes, you're right. Sorry!

[rhettinger]

+1 on this idea

[loewis]

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

[rosslight]

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

[ncoghlan]

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.

[rosslight]

Tests added and new patch uploaded. Anything else, anyone?

[ericvsmith]

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.

[brettcannon]

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

[ericvsmith]

"... as a named tuple" works for me. I'll go with that. Thanks!

[tiran]

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.

[rhettinger]

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.

[ericvsmith]

Committed in r69331 (trunk) and r69346 (py3k).