Mailman 3 August 2013 - Python-checkins

cpython: issue9035: Prevent Windows-specific tests from running on non-Windows platforms
by tim.golden Aug. 1, 2013

Aug. 1, 2013

http://hg.python.org/cpython/rev/5258c4399f2e changeset: 84946:5258c4399f2e user: Tim Golden <mail(a)timgolden.me.uk> date: Thu Aug 01 13:58:58 2013 +0100 summary: issue9035: Prevent Windows-specific tests from running on non-Windows platforms files: Lib/test/test_ntpath.py | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -274,22 +274,22 @@ with support.temp_dir() as d: self.assertFalse(ntpath.ismount(d)) - # - # Make sure the current folder isn't the root folder - # (or any other volume root). The drive-relative - # locations below cannot then refer to mount points - # - drive, path = ntpath.splitdrive(sys.executable) - with support.change_cwd(os.path.dirname(sys.executable)): - self.assertFalse(ntpath.ismount(drive.lower())) - self.assertFalse(ntpath.ismount(drive.upper())) + if sys.platform == "win32": + # + # Make sure the current folder isn't the root folder + # (or any other volume root). The drive-relative + # locations below cannot then refer to mount points + # + drive, path = ntpath.splitdrive(sys.executable) + with support.change_cwd(os.path.dirname(sys.executable)): + self.assertFalse(ntpath.ismount(drive.lower())) + self.assertFalse(ntpath.ismount(drive.upper())) - self.assertTrue(ntpath.ismount("\\\\localhost\\c$")) - self.assertTrue(ntpath.ismount("\\\\localhost\\c$\\")) + self.assertTrue(ntpath.ismount("\\\\localhost\\c$")) + self.assertTrue(ntpath.ismount("\\\\localhost\\c$\\")) - self.assertTrue(ntpath.ismount(b"\\\\localhost\\c$")) - self.assertTrue(ntpath.ismount(b"\\\\localhost\\c$\\")) - + self.assertTrue(ntpath.ismount(b"\\\\localhost\\c$")) + self.assertTrue(ntpath.ismount(b"\\\\localhost\\c$\\")) class NtCommonTest(test_genericpath.CommonTest, unittest.TestCase): pathmodule = ntpath -- Repository URL: http://hg.python.org/cpython

1 0

devguide: Take a bit more asterisk-responsibility
by eli.bendersky Aug. 1, 2013

Aug. 1, 2013

http://hg.python.org/devguide/rev/2d93e4f13b92 changeset: 634:2d93e4f13b92 user: Eli Bendersky <eliben(a)gmail.com> date: Thu Aug 01 05:53:35 2013 -0700 summary: Take a bit more asterisk-responsibility files: experts.rst | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/experts.rst b/experts.rst --- a/experts.rst +++ b/experts.rst @@ -100,7 +100,7 @@ dummy_threading brett.cannon email barry, r.david.murray* encodings lemburg, loewis -enum eli.bendersky, barry, ethan.furman* +enum eli.bendersky*, barry, ethan.furman* errno exceptions fcntl @@ -249,7 +249,7 @@ xml.dom xml.dom.minidom xml.dom.pulldom -xml.etree effbot (inactive), eli.bendersky +xml.etree effbot (inactive), eli.bendersky* xml.parsers.expat christian.heimes xml.sax christian.heimes xml.sax.handler -- Repository URL: http://hg.python.org/devguide

1 0

peps: PEP 8 modernisation
by nick.coghlan Aug. 1, 2013

Aug. 1, 2013

http://hg.python.org/peps/rev/fb24c80e9afb changeset: 5012:fb24c80e9afb user: Nick Coghlan <ncoghlan(a)gmail.com> date: Thu Aug 01 22:25:37 2013 +1000 summary: PEP 8 modernisation Thanks to Guido van Rossum, Barry Warsaw, Raymond Hettinger, Terry Reedy, Thomas Wouters and others for the constructive feedback :) files: pep-0008.txt | 257 ++++++++++++++++++++++++++++++-------- 1 files changed, 198 insertions(+), 59 deletions(-) diff --git a/pep-0008.txt b/pep-0008.txt --- a/pep-0008.txt +++ b/pep-0008.txt @@ -3,12 +3,13 @@ Version: $Revision$ Last-Modified: $Date$ Author: Guido van Rossum <guido(a)python.org>, - Barry Warsaw <barry(a)python.org> + Barry Warsaw <barry(a)python.org>, + Nick Coghlan <ncoghlan(a)gmail.com> Status: Active Type: Process Content-Type: text/x-rst Created: 05-Jul-2001 -Post-History: 05-Jul-2001 +Post-History: 05-Jul-2001, 01-Aug-2013 Introduction @@ -23,6 +24,10 @@ Guido's original Python Style Guide essay, with some additions from Barry's style guide [2]_. +This style guide evolves over time as additional conventions are +identified and past conventions are rendered obsolete by changes in +the language itself. + A Foolish Consistency is the Hobgoblin of Little Minds ====================================================== @@ -41,15 +46,24 @@ judgment. Look at other examples and decide what looks best. And don't hesitate to ask! -Two good reasons to break a particular rule: +In particular: do not break backwards compatibility just to comply with +this PEP! -1. When applying the rule would make the code less readable, even for - someone who is used to reading code that follows the rules. +Some other good reasons to ignore a particular guideline: + +1. When applying the guideline would make the code less readable, even + for someone who is used to reading code that follows this PEP. 2. To be consistent with surrounding code that also breaks it (maybe for historic reasons) -- although this is also an opportunity to clean up someone else's mess (in true XP style). +3. Because the code in question predates the introduction of the + guideline and there is no other reason to be modifying that code. + +4. When the code needs to remain compatible with older versions of + Python that don't support the feature recommended by the style guide. + Code lay-out ============ @@ -59,9 +73,6 @@ Use 4 spaces per indentation level. -For really old code that you don't want to mess up, you can continue -to use 8-space tabs. - Continuation lines should align wrapped elements either vertically using Python's implicit line joining inside parentheses, brackets and braces, or using a hanging indent. When using a hanging indent the @@ -129,31 +140,41 @@ Tabs or Spaces? --------------- -Never mix tabs and spaces. +Spaces are the preferred indentation method. -The most popular way of indenting Python is with spaces only. The -second-most popular way is with tabs only. Code indented with a -mixture of tabs and spaces should be converted to using spaces -exclusively. When invoking the Python command line interpreter with +Tabs should be used solely to remain consistent with code that is +already indented with tabs. + +Python 3 disallows mixing the use of tabs and spaces for indentation. + +Python 2 code indented with a mixture of tabs and spaces should be +converted to using spaces exclusively. + +When invoking the Python 2 command line interpreter with the ``-t`` option, it issues warnings about code that illegally mixes tabs and spaces. When using ``-tt`` these warnings become errors. These options are highly recommended! -For new projects, spaces-only are strongly recommended over tabs. -Most editors have features that make this easy to do. Maximum Line Length ------------------- -Limit all lines to a maximum of 79 characters. +Aim to limit all lines to a maximum of 79 characters, but up to 99 +characters is acceptable when it improves readability. -There are still many devices around that are limited to 80 character -lines; plus, limiting windows to 80 characters makes it possible to -have several windows side-by-side. The default wrapping on such -devices disrupts the visual structure of the code, making it more -difficult to understand. Therefore, please limit all lines to a -maximum of 79 characters. For flowing long blocks of text (docstrings -or comments), limiting the length to 72 characters is recommended. +For flowing long blocks of text with fewer structural restrictions +(docstrings or comments), limiting the line length to 72 characters +is recommended. + +Limiting the required editor window width makes it possible to have +several files open side-by-side, and works well when using code +review tools that present the two versions in adjacent columns. + +The default wrapping in most tools disrupts the visual structure of the +code, making it more difficult to understand. The limits are chosen to +avoid wrapping in editors with the window width set to 80 (or 100), even +if the tool places a marker glyph in the final column when wrapping +lines. Some web based tools may not offer dynamic line wrapping at all. The preferred way of wrapping long lines is by using Python's implied line continuation inside parentheses, brackets and braces. Long lines @@ -214,15 +235,17 @@ Encodings (PEP 263) ------------------- -Code in the core Python distribution should always use the ASCII or -Latin-1 encoding (a.k.a. ISO-8859-1). For Python 3.0 and beyond, -UTF-8 is preferred over Latin-1, see PEP 3120. +Code in the core Python distribution should always use UTF-8 (or ASCII +in Python 2). -Files using ASCII should not have a coding cookie. Latin-1 (or UTF-8) -should only be used when a comment or docstring needs to mention an -author name that requires Latin-1; otherwise, using ``\x``, ``\u`` or -``\U`` escapes is the preferred way to include non-ASCII data in -string literals. +Files using ASCII (in Python 2) or UTF-8 (in Python 3) should not have a +coding cookie. + +In the standard library, non-default encodings should be used only for +test purposes or when a comment or docstring needs to mention an author +name that that contains non-ASCII characters; otherwise, using ``\x``, +``\u``, ``\U``, or ``\N`` escapes is the preferred way to include +non-ASCII data in string literals. For Python 3.0 and beyond, the following policy is prescribed for the standard library (see PEP 3131): All identifiers in the Python @@ -266,11 +289,27 @@ Put any relevant ``__all__`` specification after the imports. -- Relative imports for intra-package imports are highly discouraged. - Always use the absolute package path for all imports. Even now that - PEP 328 is fully implemented in Python 2.5, its style of explicit - relative imports is actively discouraged; absolute imports are more - portable and usually more readable. +- Absolute imports are recommended, as they are usually more readable + and tend to be better behaved (or at least give better error + messages) if the import system is incorrectly configured (such as + when a directory inside a package ends up on ``sys.path``):: + + import mypkg.sibling + from mypkg import sibling + from mypkg.sibling import example + + However, explicit relative imports are an acceptable alternative to + absolute imports, especially when dealing with complex package layouts + where using absolute imports would be unecessarily verbose:: + + from . import sibling + from .sibling import example + + Standard library code should avoid complex package layouts and always + use absolute imports. + + Implicit relative imports should *never* be used and have been removed + in Python 3. - When importing a class from a class-containing module, it's usually okay to spell this:: @@ -285,6 +324,18 @@ and use "myclass.MyClass" and "foo.bar.yourclass.YourClass". +- Wildcard imports (``from <module> import *``) should be avoided, as + they make it unclear which names are present in the namespace, + confusing both readers and many automated tools. There is one + defensible use case for a wildcard import, which is to republish an + internal interface as part of a public API (for example, overwriting + a pure Python implementation of an interface with the definitions + from an optional accelerator module and exactly which definitions + will be overwritten isn't known in advance). + + When republishing names this way, the guidelines below regarding + public and internal interfaces still apply. + Whitespace in Expressions and Statements ======================================== @@ -760,6 +811,36 @@ advanced callers. +Public and internal interfaces +------------------------------ + +Any backwards compatibility guarantees apply only to public interfaces. +Accordingly, it is important that users be able to clearly distinguish +between public and internal interfaces. + +Documented interfaces are considered public, unless the documentation +explicitly declares them to be provisional or internal interfaces exempt +from the usual backwards compatibility guarantees. All undocumented +interfaces should be assumed to be internal. + +To better support introspection, modules should explicitly declare the +names in their public API using the ``__all__`` attribute. Setting +``__all__`` to an empty list indicates that the module has no public API. + +Even with ``__all__`` set appropriately, internal interfaces (packages, +modules, classes, functions, attributes or other names) should still be +prefixed with a single leading underscore. + +An interface is also considered internal if any containing namespace +(package, module or class) is considered internal. + +Imported names should always be considered an implementation detail. +Other modules must not rely on indirect access to such imported names +unless they are an explicitly documented part of the containing module's +API, such as ``os.path`` or a package's ``__init__`` module that exposes +functionality from submodules. + + Programming Recommendations =========================== @@ -769,10 +850,12 @@ For example, do not rely on CPython's efficient implementation of in-place string concatenation for statements in the form ``a += b`` - or ``a = a + b``. Those statements run more slowly in Jython. In - performance sensitive parts of the library, the ``''.join()`` form - should be used instead. This will ensure that concatenation occurs - in linear time across various implementations. + or ``a = a + b``. This optimization is fragile even in CPython (it + only works for some types) and isn't present at all in implementations + that don't use refcounting. In performance sensitive parts of the + library, the ``''.join()`` form should be used instead. This will + ensure that concatenation occurs in linear time across various + implementations. - Comparisons to singletons like None should always be done with ``is`` or ``is not``, never the equality operators. @@ -799,29 +882,59 @@ operator. However, it is best to implement all six operations so that confusion doesn't arise in other contexts. -- Use class-based exceptions. +- Always use a def statement instead of assigning a lambda expression + to a name. - String exceptions in new code are forbidden, and this language - feature has been removed in Python 2.6. + Yes:: - Modules or packages should define their own domain-specific base - exception class, which should be subclassed from the built-in - Exception class. Always include a class docstring. E.g.:: + def f(x): return 2*x - class MessageError(Exception): - """Base class for errors in the email package.""" + No:: + + f = lambda x: 2*x + + The first form means that the name of the resulting function object is + specifically 'f' instead of the generic '<lambda>'. This is more + useful for tracebacks and string representations in general. The use + of the assignment statement eliminates the sole benefit a lambda + expression can offer over an explicit def statement (i.e. that it can + be embedded inside a larger expression) + +- Derive exceptions from ``Exception`` rather than ``BaseException``. + Direct inheritance from ``BaseException`` is reserved for exceptions + where catching them is almost always the wrong thing to do. + + Design exception hierarchies based on the distinctions that code + *catching* the exceptions is likely to need, rather than the locations + where the exceptions are raised. Aim to answer the question + "What went wrong?" programmatically, rather than only stating that + "A problem occurred" (see PEP 3151 for an example of this lesson being + learned for the builtin exception hierarchy) Class naming conventions apply here, although you should add the - suffix "Error" to your exception classes, if the exception is an - error. Non-error exceptions need no special suffix. + suffix "Error" to your exception classes if the exception is an + error. Non-error exceptions that are used for non-local flow control + or other forms of signalling need no special suffix. -- When raising an exception, use ``raise ValueError('message')`` +- Use exception chaining appropriately. In Python 3, "raise X from Y" + should be used to indicate explicit replacement without losing the + original traceback. + + When deliberately replacing an inner exception (using "raise X" in + Python 2 or "raise X from None" in Python 3.3+), ensure that relevant + details are transferred to the new exception (such as preserving the + attribute name when converting KeyError to AttributeError, or + embedding the text of the original exception in the new exception + message). + +- When raising an exception in Python 2, use ``raise ValueError('message')`` instead of the older form ``raise ValueError, 'message'``. - The paren-using form is preferred because when the exception - arguments are long or include string formatting, you don't need to - use line continuation characters thanks to the containing - parentheses. The older form is not legal syntax in Python 3. + The latter form is not legal Python 3 syntax. + + The paren-using form also means that when the exception arguments are + long or include string formatting, you don't need to use line + continuation characters thanks to the containing parentheses. - When catching exceptions, mention specific exceptions whenever possible instead of using a bare ``except:`` clause. @@ -851,6 +964,21 @@ exception propagate upwards with ``raise``. ``try...finally`` can be a better way to handle this case. +- When binding caught exceptions to a name, prefer the explicit name + binding syntax added in Python 2.6:: + + try: + process_data() + except Exception as exc: + raise DataProcessingFailedError(str(exc)) + + This is the only syntax supported in Python 3, and avoids the + ambiguity problems associated with the older comma-based syntax. + +- When catching operating system errors, prefer the explicit exception + hierarchy introduced in Python 3.3 over introspection of ``errno`` + values. + - Additionally, for all try/except clauses, limit the ``try`` clause to the absolute minimum amount of code necessary. Again, this avoids masking bugs. @@ -873,6 +1001,10 @@ # Will also catch KeyError raised by handle_value() return key_not_found(key) +- When a resource is local to a particular section of code, use a + ``with`` statement to ensure it is cleaned up promptly and reliably + after use. A try/finally statement is also acceptable. + - Context managers should be invoked through separate functions or methods whenever they do something other than acquire and release resources. For example: @@ -907,9 +1039,6 @@ Yes: if foo.startswith('bar'): No: if foo[:3] == 'bar': - The exception is if your code must work with Python 1.5.2 (but let's - hope not!). - - Object type comparisons should always use isinstance() instead of comparing types directly. :: @@ -918,11 +1047,15 @@ No: if type(obj) is type(1): When checking if an object is a string, keep in mind that it might - be a unicode string too! In Python 2.3, str and unicode have a + be a unicode string too! In Python 2, str and unicode have a common base class, basestring, so you can do:: if isinstance(obj, basestring): + Note that in Python 3, ``unicode`` and ``basestring`` no longer exist + (there is only ``str``) and a bytes object is no longer a kind of + string (it is a sequence of integers instead) + - For sequences, (strings, lists, tuples), use the fact that empty sequences are false. :: @@ -947,6 +1080,10 @@ annotation style. Instead, the annotations are left for users to discover and experiment with useful annotation styles. + It is recommended that third party experimants with annotations use an + associated decorator to indicate how the annotation should be + interpreted. + Early core developer attempts to use function annotations revealed inconsistent, ad-hoc annotation styles. For example: @@ -1004,6 +1141,8 @@ .. [3] http://www.wikipedia.com/wiki/CamelCase +.. [4] PEP 8 modernisation, July 2013 + http://bugs.python.org/issue18472 Copyright ========= -- Repository URL: http://hg.python.org/peps

1 0

cpython: Issue #9035: os.path.ismount now recognises volumes mounted below
by tim.golden Aug. 1, 2013

Aug. 1, 2013

http://hg.python.org/cpython/rev/f283589cb71e changeset: 84945:f283589cb71e user: Tim Golden <mail(a)timgolden.me.uk> date: Thu Aug 01 12:44:00 2013 +0100 summary: Issue #9035: os.path.ismount now recognises volumes mounted below a drive root on Windows. Original patch by Atsuo Ishimoto. files: Lib/ntpath.py | 29 ++++++++++++++++--- Lib/test/test_ntpath.py | 34 +++++++++++++++++++++++ Misc/NEWS | 3 ++ Modules/posixmodule.c | 42 +++++++++++++++++++++++++++++ 4 files changed, 103 insertions(+), 5 deletions(-) diff --git a/Lib/ntpath.py b/Lib/ntpath.py --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -335,16 +335,35 @@ return False return True -# Is a path a mount point? Either a root (with or without drive letter) -# or an UNC path with at most a / or \ after the mount point. - +# Is a path a mount point? +# Any drive letter root (eg c:\) +# Any share UNC (eg \\server\share) +# Any volume mounted on a filesystem folder +# +# No one method detects all three situations. Historically we've lexically +# detected drive letter roots and share UNCs. The canonical approach to +# detecting mounted volumes (querying the reparse tag) fails for the most +# common case: drive letter roots. The alternative which uses GetVolumePathName +# fails if the drive letter is the result of a SUBST. +try: + from nt import _getvolumepathname +except ImportError: + _getvolumepathname = None def ismount(path): - """Test whether a path is a mount point (defined as root of drive)""" + """Test whether a path is a mount point (a drive root, the root of a + share, or a mounted volume)""" seps = _get_bothseps(path) + path = abspath(path) root, rest = splitdrive(path) if root and root[0] in seps: return (not rest) or (rest in seps) - return rest in seps + if rest in seps: + return True + + if _getvolumepathname: + return path.rstrip(seps) == _getvolumepathname(path).rstrip(seps) + else: + return False # Expand paths beginning with '~' or '~user'. diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -256,6 +256,40 @@ # dialogs (#4804) ntpath.sameopenfile(-1, -1) + def test_ismount(self): + self.assertTrue(ntpath.ismount("c:\\")) + self.assertTrue(ntpath.ismount("C:\\")) + self.assertTrue(ntpath.ismount("c:/")) + self.assertTrue(ntpath.ismount("C:/")) + self.assertTrue(ntpath.ismount("\\\\.\\c:\\")) + self.assertTrue(ntpath.ismount("\\\\.\\C:\\")) + + self.assertTrue(ntpath.ismount(b"c:\\")) + self.assertTrue(ntpath.ismount(b"C:\\")) + self.assertTrue(ntpath.ismount(b"c:/")) + self.assertTrue(ntpath.ismount(b"C:/")) + self.assertTrue(ntpath.ismount(b"\\\\.\\c:\\")) + self.assertTrue(ntpath.ismount(b"\\\\.\\C:\\")) + + with support.temp_dir() as d: + self.assertFalse(ntpath.ismount(d)) + + # + # Make sure the current folder isn't the root folder + # (or any other volume root). The drive-relative + # locations below cannot then refer to mount points + # + drive, path = ntpath.splitdrive(sys.executable) + with support.change_cwd(os.path.dirname(sys.executable)): + self.assertFalse(ntpath.ismount(drive.lower())) + self.assertFalse(ntpath.ismount(drive.upper())) + + self.assertTrue(ntpath.ismount("\\\\localhost\\c$")) + self.assertTrue(ntpath.ismount("\\\\localhost\\c$\\")) + + self.assertTrue(ntpath.ismount(b"\\\\localhost\\c$")) + self.assertTrue(ntpath.ismount(b"\\\\localhost\\c$\\")) + class NtCommonTest(test_genericpath.CommonTest, unittest.TestCase): pathmodule = ntpath diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,9 @@ Core and Builtins ----------------- +- Issue #9035: ismount now recognises volumes mounted below a drive root + on Windows. Original patch by Atsuo Ishimoto. + - Issue #18214: Improve finalization of Python modules to avoid setting their globals to None, in most cases. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3711,6 +3711,47 @@ else Py_RETURN_FALSE; } + +PyDoc_STRVAR(posix__getvolumepathname__doc__, +"Return volume mount point of the specified path."); + +/* A helper function for ismount on windows */ +static PyObject * +posix__getvolumepathname(PyObject *self, PyObject *args) +{ + PyObject *po, *result; + wchar_t *path, *mountpath=NULL; + size_t bufsize; + BOOL ret; + + if (!PyArg_ParseTuple(args, "U|:_getvolumepathname", &po)) + return NULL; + path = PyUnicode_AsUnicode(po); + if (path == NULL) + return NULL; + + /* Volume path should be shorter than entire path */ + bufsize = max(MAX_PATH, wcslen(path) * 2 * sizeof(wchar_t)+1); + mountpath = (wchar_t *)PyMem_Malloc(bufsize); + if (mountpath == NULL) + return PyErr_NoMemory(); + + Py_BEGIN_ALLOW_THREADS + ret = GetVolumePathNameW(path, mountpath, bufsize); + Py_END_ALLOW_THREADS + + if (!ret) { + result = win32_error_object("_getvolumepathname", po); + goto exit; + } + result = PyUnicode_FromWideChar(mountpath, wcslen(mountpath)); + +exit: + PyMem_Free(mountpath); + return result; +} +/* end of posix__getvolumepathname */ + #endif /* MS_WINDOWS */ PyDoc_STRVAR(posix_mkdir__doc__, @@ -10885,6 +10926,7 @@ {"_getfinalpathname", posix__getfinalpathname, METH_VARARGS, NULL}, {"_isdir", posix__isdir, METH_VARARGS, posix__isdir__doc__}, {"_getdiskusage", win32__getdiskusage, METH_VARARGS, win32__getdiskusage__doc__}, + {"_getvolumepathname", posix__getvolumepathname, METH_VARARGS, posix__getvolumepathname__doc__}, #endif #ifdef HAVE_GETLOADAVG {"getloadavg", posix_getloadavg, METH_NOARGS, posix_getloadavg__doc__}, -- Repository URL: http://hg.python.org/cpython

1 0

Daily reference leaks (4f3b6eff2ede): sum=12358
by solipsis＠pitrou.net Aug. 1, 2013

Aug. 1, 2013

results for 4f3b6eff2ede on branch "default" -------------------------------------------- test_capi leaked [3464, 3464, 3464] references, sum=10392 test_capi leaked [654, 656, 656] memory blocks, sum=1966 Command line was: ['./python', '-m', 'test.regrtest', '-uall', '-R', '3:3:/home/antoine/cpython/refleaks/reflogavskOF', '-x']

1 0

cpython: #17616: Improve context manager tests, fix bugs in close method and mode docs.
by r.david.murray Aug. 1, 2013

Aug. 1, 2013

http://hg.python.org/cpython/rev/4f3b6eff2ede changeset: 84944:4f3b6eff2ede user: R David Murray <rdmurray(a)bitdance.com> date: Wed Jul 31 20:48:26 2013 -0400 summary: #17616: Improve context manager tests, fix bugs in close method and mode docs. 'mode' docs fix: the file must always be opened in binary in Python3. Bug in Wave_write.close: when the close method calls the check that the header exists and it raises an error, the _file attribute never gets set to None, so the next close tries to close the file again and we get an ignored traceback in the __del__ method. The fix is to set _file to None in a finally clause. This represents a behavior change...in theory a program could be checking for the error on close and then doing a recovery action on the still open file and closing it again. But this change will only go into 3.4, so I think that behavior change is acceptable given that it would be pretty weird and unlikely logic to begin with. files: Doc/library/wave.rst | 15 ++++---- Lib/test/test_wave.py | 53 +++++++++++++++++++++++------- Lib/wave.py | 12 ++++-- 3 files changed, 54 insertions(+), 26 deletions(-) diff --git a/Doc/library/wave.rst b/Doc/library/wave.rst --- a/Doc/library/wave.rst +++ b/Doc/library/wave.rst @@ -19,21 +19,20 @@ .. function:: open(file, mode=None) If *file* is a string, open the file by that name, otherwise treat it as a - seekable file-like object. *mode* can be any of + seekable file-like object. *mode* can be: - ``'r'``, ``'rb'`` + ``'rb'`` Read only mode. - ``'w'``, ``'wb'`` + ``'wb'`` Write only mode. Note that it does not allow read/write WAV files. - A *mode* of ``'r'`` or ``'rb'`` returns a :class:`Wave_read` object, while a - *mode* of ``'w'`` or ``'wb'`` returns a :class:`Wave_write` object. If - *mode* is omitted and a file-like object is passed as *file*, ``file.mode`` - is used as the default value for *mode* (the ``'b'`` flag is still added if - necessary). + A *mode* of ``'rb'`` returns a :class:`Wave_read` object, while a *mode* of + ``'wb'`` returns a :class:`Wave_write` object. If *mode* is omitted and a + file-like object is passed as *file*, ``file.mode`` is used as the default + value for *mode*. If you pass in a file-like object, the wave object will not close it when its :meth:`close` method is called; it is the caller's responsibility to close diff --git a/Lib/test/test_wave.py b/Lib/test/test_wave.py --- a/Lib/test/test_wave.py +++ b/Lib/test/test_wave.py @@ -69,22 +69,49 @@ self.assertEqual(params.comptype, self.f.getcomptype()) self.assertEqual(params.compname, self.f.getcompname()) - def test_context_manager(self): - self.f = wave.open(TESTFN, 'wb') - self.f.setnchannels(nchannels) - self.f.setsampwidth(sampwidth) - self.f.setframerate(framerate) - self.f.close() + def test_wave_write_context_manager_calls_close(self): + # Close checks for a minimum header and will raise an error + # if it is not set, so this proves that close is called. + with self.assertRaises(wave.Error): + with wave.open(TESTFN, 'wb') as f: + pass + print('in test:', f._file) + with self.assertRaises(wave.Error): + with open(TESTFN, 'wb') as testfile: + with wave.open(testfile): + pass + def test_context_manager_with_open_file(self): + with open(TESTFN, 'wb') as testfile: + with wave.open(testfile) as f: + f.setnchannels(nchannels) + f.setsampwidth(sampwidth) + f.setframerate(framerate) + self.assertFalse(testfile.closed) + with open(TESTFN, 'rb') as testfile: + with wave.open(testfile) as f: + self.assertFalse(f.getfp().closed) + params = f.getparams() + self.assertEqual(params.nchannels, nchannels) + self.assertEqual(params.sampwidth, sampwidth) + self.assertEqual(params.framerate, framerate) + self.assertIsNone(f.getfp()) + self.assertFalse(testfile.closed) + + def test_context_manager_with_filename(self): + # If the file doesn't get closed, this test won't fail, but it will + # produce a resource leak warning. + with wave.open(TESTFN, 'wb') as f: + f.setnchannels(nchannels) + f.setsampwidth(sampwidth) + f.setframerate(framerate) with wave.open(TESTFN) as f: self.assertFalse(f.getfp().closed) - self.assertIs(f.getfp(), None) - - with open(TESTFN, 'wb') as testfile: - with self.assertRaises(wave.Error): - with wave.open(testfile, 'wb'): - pass - self.assertEqual(testfile.closed, False) + params = f.getparams() + self.assertEqual(params.nchannels, nchannels) + self.assertEqual(params.sampwidth, sampwidth) + self.assertEqual(params.framerate, framerate) + self.assertIsNone(f.getfp()) if __name__ == '__main__': diff --git a/Lib/wave.py b/Lib/wave.py --- a/Lib/wave.py +++ b/Lib/wave.py @@ -448,11 +448,13 @@ def close(self): if self._file: - self._ensure_header_written(0) - if self._datalength != self._datawritten: - self._patchheader() - self._file.flush() - self._file = None + try: + self._ensure_header_written(0) + if self._datalength != self._datawritten: + self._patchheader() + self._file.flush() + finally: + self._file = None if self._i_opened_the_file: self._i_opened_the_file.close() self._i_opened_the_file = None -- Repository URL: http://hg.python.org/cpython

1 0