[issue12196] add pipe2() to the os module

STINNER Victor report at bugs.python.org
Mon May 30 21:40:06 CEST 2011


STINNER Victor <victor.stinner at haypocalc.com> added the comment:

> See the patch attached.

I like your patch, it removes many duplicate code :-)

Some comments:
 - I don't like an if surrounding the whole function, I prefer "if not ...: 
return" to avoid the (useless) indentation. Well, it's my coding style, do as 
you want.
 - You should add docstrings. Something like "Raise a SkipTest if the OS is 
Linux and the kernel version if lesser than min_version. For example, 
support.requires_linux_version(2, 6, 28) raises a SkipTest if the version is 
lesser than 2.6.28."
 - You should move the "if version < ..." outside the try/except ValueError

+            # platform.release() is something like '2.6.33.7-desktop-2mnb'
+            version_string = platform.release().split('-')[0]

Dummy micro-optimization: .split('-', 1)[0] or .partition('-')[0] is maybe 
better than .split('-')[0].

> > By the way, I like the new os.pipe2() function! You may want to document
> > it in
> > the "What's new in Python 3.3" doc (just mention the new function, the
> > document will be rephrased later).
> 
> Sure, where is this document?

Doc/whatsnew/3.3.rst

----------

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue12196>
_______________________________________


More information about the Python-bugs-list mailing list