msg75647 - (view) |
Author: Brett Cannon (brett.cannon) *  |
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) *  |
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) *  |
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) *  |
Date: 2009-01-25 19:29 |
+1 on this idea |
|
|
msg80534 - (view) |
Author: Martin v. Löwis (loewis) *  |
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: Alyssa Coghlan (ncoghlan) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
Date: 2009-02-06 01:33 |
Committed in r69331 (trunk) and r69346 (py3k). |
|
|