Mailing List Archive

Jean-Paul Calderone <exarkun@divmod.com> added the comment:

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

----------
nosy: +exarkun

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Ross Light <rlight2@gmail.com> added the comment:

Oh yes, you're right. Sorry!

Added file: http://bugs.python.org/file12857/patch-4285b.diff

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Raymond Hettinger <rhettinger@users.sourceforge.net> added the comment:

+1 on this idea

----------
nosy: +rhettinger

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Changes by Martin v. Löwis <martin@v.loewis.de>:


Removed file: http://bugs.python.org/file12854/patch-4285a.diff

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Martin v. Löwis <martin@v.loewis.de> added the comment:

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

----------
nosy: +loewis

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Ross Light <rlight2@gmail.com> added the comment:

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

Added file: http://bugs.python.org/file12860/patch-4285c.diff

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Nick Coghlan <ncoghlan@gmail.com> added the comment:

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.

----------
nosy: +ncoghlan

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Ross Light <rlight2@gmail.com> added the comment:

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

Added file: http://bugs.python.org/file12870/patch-4285d.diff

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Changes by Eric Smith <eric@trueblade.com>:


----------
nosy: +eric.smith

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Changes by Eric Smith <eric@trueblade.com>:


----------
assignee: brett.cannon -> eric.smith

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

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

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Brett Cannon <brett@python.org> added the comment:

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

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Eric Smith <eric@trueblade.com> added the comment:

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

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Christian Heimes <lists@cheimes.de> added the comment:

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.

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Raymond Hettinger <rhettinger@users.sourceforge.net> added the comment:

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

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Eric Smith <eric@trueblade.com> added the comment:

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

----------
resolution: -> accepted
status: open -> closed

_______________________________________
Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue4285>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com