From 059def58a5201d4626396ad39cc885645235b959 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sat, 26 Oct 2013 18:08:15 +1000 Subject: [PATCH] Close #19330 by using public classes in contextlib - added test cases to ensure docstrings are reasonable - also updates various comments in contextlib for accuracy - identifed #19404 as an issue making it difficult to provide good help output on generator based context manager instances --- Lib/contextlib.py | 86 ++++++++++++++++++------------------- Lib/test/test_contextlib.py | 63 ++++++++++++++++++++++++--- Misc/NEWS | 5 +++ 3 files changed, 105 insertions(+), 49 deletions(-) diff --git a/Lib/contextlib.py b/Lib/contextlib.py index a564943d87a..fb89118052e 100644 --- a/Lib/contextlib.py +++ b/Lib/contextlib.py @@ -37,6 +37,16 @@ class _GeneratorContextManager(ContextDecorator): def __init__(self, func, *args, **kwds): self.gen = func(*args, **kwds) self.func, self.args, self.kwds = func, args, kwds + # Issue 19330: ensure context manager instances have good docstrings + doc = getattr(func, "__doc__", None) + if doc is None: + doc = type(self).__doc__ + self.__doc__ = doc + # Unfortunately, this still doesn't provide good help output when + # inspecting the created context manager instances, since pydoc + # currently bypasses the instance docstring and shows the docstring + # for the class instead. + # See http://bugs.python.org/issue19404 for more details. def _recreate_cm(self): # _GCM instances are one-shot context managers, so the @@ -117,9 +127,6 @@ def contextmanager(func): return helper -# Unfortunately, this was originally published as a class, so -# backwards compatibility prevents the use of the wrapper function -# approach used for the other classes class closing(object): """Context to automatically close something at the end of a block. @@ -144,8 +151,18 @@ class closing(object): def __exit__(self, *exc_info): self.thing.close() -class _RedirectStdout: - """Helper for redirect_stdout.""" +class redirect_stdout: + """Context manager for temporarily redirecting stdout to another file + + # How to send help() to stderr + with redirect_stdout(sys.stderr): + help(dir) + + # How to write help() to a file + with open('help.txt', 'w') as f: + with redirect_stdout(f): + help(pow) + """ def __init__(self, new_target): self._new_target = new_target @@ -163,46 +180,9 @@ class _RedirectStdout: self._old_target = self._sentinel sys.stdout = restore_stdout -# Use a wrapper function since we don't care about supporting inheritance -# and a function gives much cleaner output in help() -def redirect_stdout(target): - """Context manager for temporarily redirecting stdout to another file - - # How to send help() to stderr - with redirect_stdout(sys.stderr): - help(dir) - - # How to write help() to a file - with open('help.txt', 'w') as f: - with redirect_stdout(f): - help(pow) - """ - return _RedirectStdout(target) -class _SuppressExceptions: - """Helper for suppress.""" - def __init__(self, *exceptions): - self._exceptions = exceptions - - def __enter__(self): - pass - - def __exit__(self, exctype, excinst, exctb): - # Unlike isinstance and issubclass, exception handling only - # looks at the concrete type heirarchy (ignoring the instance - # and subclass checking hooks). However, all exceptions are - # also required to be concrete subclasses of BaseException, so - # if there's a discrepancy in behaviour, we currently consider it - # the fault of the strange way the exception has been defined rather - # than the fact that issubclass can be customised while the - # exception checks can't. - # See http://bugs.python.org/issue12029 for more details - return exctype is not None and issubclass(exctype, self._exceptions) - -# Use a wrapper function since we don't care about supporting inheritance -# and a function gives much cleaner output in help() -def suppress(*exceptions): +class suppress: """Context manager to suppress specified exceptions After the exception is suppressed, execution proceeds with the next @@ -212,7 +192,25 @@ def suppress(*exceptions): os.remove(somefile) # Execution still resumes here if the file was already removed """ - return _SuppressExceptions(*exceptions) + + def __init__(self, *exceptions): + self._exceptions = exceptions + + def __enter__(self): + pass + + def __exit__(self, exctype, excinst, exctb): + # Unlike isinstance and issubclass, CPython exception handling + # currently only looks at the concrete type hierarchy (ignoring + # the instance and subclass checking hooks). While Guido considers + # that a bug rather than a feature, it's a fairly hard one to fix + # due to various internal implementation details. suppress provides + # the simpler issubclass based semantics, rather than trying to + # exactly reproduce the limitations of the CPython interpreter. + # + # See http://bugs.python.org/issue12029 for more details + return exctype is not None and issubclass(exctype, self._exceptions) + # Inspired by discussions on http://bugs.python.org/issue13585 class ExitStack(object): diff --git a/Lib/test/test_contextlib.py b/Lib/test/test_contextlib.py index 419104ddb0b..6362a9700db 100644 --- a/Lib/test/test_contextlib.py +++ b/Lib/test/test_contextlib.py @@ -14,6 +14,20 @@ except ImportError: class ContextManagerTestCase(unittest.TestCase): + def test_instance_docstring_given_function_docstring(self): + # Issue 19330: ensure context manager instances have good docstrings + # See http://bugs.python.org/issue19404 for why this doesn't currently + # affect help() output :( + def gen_with_docstring(): + """This has a docstring""" + yield + gen_docstring = gen_with_docstring.__doc__ + cm_with_docstring = contextmanager(gen_with_docstring) + self.assertEqual(cm_with_docstring.__doc__, gen_docstring) + obj = cm_with_docstring() + self.assertEqual(obj.__doc__, gen_docstring) + self.assertNotEqual(obj.__doc__, type(obj).__doc__) + def test_contextmanager_plain(self): state = [] @contextmanager @@ -109,7 +123,11 @@ class ContextManagerTestCase(unittest.TestCase): class ClosingTestCase(unittest.TestCase): - # XXX This needs more work + def test_instance_docs(self): + # Issue 19330: ensure context manager instances have good docstrings + cm_docstring = closing.__doc__ + obj = closing(None) + self.assertEqual(obj.__doc__, cm_docstring) def test_closing(self): state = [] @@ -205,6 +223,7 @@ class LockContextTestCase(unittest.TestCase): class mycontext(ContextDecorator): + """Example decoration-compatible context manager for testing""" started = False exc = None catch = False @@ -220,6 +239,12 @@ class mycontext(ContextDecorator): class TestContextDecorator(unittest.TestCase): + def test_instance_docs(self): + # Issue 19330: ensure context manager instances have good docstrings + cm_docstring = mycontext.__doc__ + obj = mycontext() + self.assertEqual(obj.__doc__, cm_docstring) + def test_contextdecorator(self): context = mycontext() with context as result: @@ -373,6 +398,12 @@ class TestContextDecorator(unittest.TestCase): class TestExitStack(unittest.TestCase): + def test_instance_docs(self): + # Issue 19330: ensure context manager instances have good docstrings + cm_docstring = ExitStack.__doc__ + obj = ExitStack() + self.assertEqual(obj.__doc__, cm_docstring) + def test_no_resources(self): with ExitStack(): pass @@ -634,6 +665,12 @@ class TestExitStack(unittest.TestCase): class TestRedirectStdout(unittest.TestCase): + def test_instance_docs(self): + # Issue 19330: ensure context manager instances have good docstrings + cm_docstring = redirect_stdout.__doc__ + obj = redirect_stdout(None) + self.assertEqual(obj.__doc__, cm_docstring) + def test_redirect_to_string_io(self): f = io.StringIO() msg = "Consider an API like help(), which prints directly to stdout" @@ -671,6 +708,12 @@ class TestRedirectStdout(unittest.TestCase): class TestSuppress(unittest.TestCase): + def test_instance_docs(self): + # Issue 19330: ensure context manager instances have good docstrings + cm_docstring = suppress.__doc__ + obj = suppress() + self.assertEqual(obj.__doc__, cm_docstring) + def test_no_result_from_enter(self): with suppress(ValueError) as enter_result: self.assertIsNone(enter_result) @@ -683,16 +726,26 @@ class TestSuppress(unittest.TestCase): with suppress(TypeError): len(5) + def test_exception_hierarchy(self): + with suppress(LookupError): + 'Hello'[50] + + def test_other_exception(self): + with self.assertRaises(ZeroDivisionError): + with suppress(TypeError): + 1/0 + + def test_no_args(self): + with self.assertRaises(ZeroDivisionError): + with suppress(): + 1/0 + def test_multiple_exception_args(self): with suppress(ZeroDivisionError, TypeError): 1/0 with suppress(ZeroDivisionError, TypeError): len(5) - def test_exception_hierarchy(self): - with suppress(LookupError): - 'Hello'[50] - def test_cm_is_reentrant(self): ignore_exceptions = suppress(Exception) with ignore_exceptions: diff --git a/Misc/NEWS b/Misc/NEWS index 3ba47ef7d57..85fbfaa253e 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -21,6 +21,11 @@ Core and Builtins Library ------- +- Issue #19330: the unnecessary wrapper functions have been removed from the + implementations of the new contextlib.redirect_stdout and + contextlib.suppress context managers, which also ensures they provide + reasonable help() output on instances + - Issue #18685: Restore re performance to pre-PEP 393 levels. - Issue #19339: telnetlib module is now using time.monotonic() when available