(original) (raw)

On 21 Oct 2013 12:44, "Raymond Hettinger" <raymond.hettinger@gmail.com> wrote:
\>
\> Two of the new context managers in contextlib are now wrapped in pass-through factory functions. �The intent is to make the help() look cleaner. �This practice does have downsides however.
\>
\> The usual way to detect whether something is usable with a with-statement is to check the presence of the \_\_enter\_\_ and \_\_exit\_\_ methods. � Wrapping the CM in a pass-through function defeats this and other forms of introspection.

>

> Also, the help() output itself is worse-off. �When you run help on a CM(), you're trying to find-out what happens on entry and what happens on exit. �If those methods had docstrings, the question would be answered directly. � The wrapper (intentionally) hides how it works.

>

> Since I teach intermediate and advanced python classes to experienced Python users, I've become more sensitive to problems this practice will create. �Defeating introspection can make the help look nicer, but it isn't a clean coding practice and is something I hope doesn't catch on.

>

> To the extent there is a problem with the output of help(), I think efforts should be directed at making help() better. � A lot of work needs to be done on that end -- for example abstract base classes also don't look great in help().

>

> There are a couple of other minor issues as well. �One is that the wrapper function hides the class, making it harder to do type checks such as "isinstance(x, suppress)". �The other issue is that wrappers cause extra jumping around for people who are tracing code through a debugger or using a visualization tool such as pythontutor. � These aren't terribly important issues, but it does support the notion that usually the cleanest code is the best code.

>

> In short, I recommend that efforts be directed at improving help() rather than limiting introspection by way of less clean coding practices.

That's a fair point, but I \*really\* dislike exposing implementations that don't match their documentation, and both of these are currently documented as factory functions.

There's also the fact that I prefer the current lower case names, but strongly dislike using lower case names for classes (despite the fact closing was included in the original contextlib with a non PEP 8 compliant class name).

(And yes, I now realise I completely failed to mention either of those points in the comments giving the rationale for the wrapper functions).

Exposing the full object oriented API is certainly a� reasonable option, but the docs should be updated accordingly, with suitable public attributes being defined (providing access to the exception tuple for suppress and the target stream for redirect\_stdio).

Cheers,
Nick.

>
\>
\> Raymond
\>
\>
\> -------- current code for suppress() --------
\>
\> class \_SuppressExceptions:
\> � � """Helper for suppress."""
\> � � def \_\_init\_\_(self, \*exceptions):
\> � � � � self.\_exceptions = exceptions
\>
\> � � def \_\_enter\_\_(self):
\> � � � � pass
\>
\> � � def \_\_exit\_\_(self, exctype, excinst, exctb):
\> � � � � 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):
\> � � """Context manager to suppress specified exceptions
\>
\> � � After the exception is suppressed, execution proceeds with the next
\> � � statement following the with statement.
\>
\> � � � � �with suppress(FileNotFoundError):
\> � � � � � � �os.remove(somefile)
\> � � � � �# Execution still resumes here if the file was already removed
\> � � """
\> � � return \_SuppressExceptions(\*exceptions)
\>
\>
\> -------- current help() output for suppress() --------
\>
\> Help on function suppress in module contextlib:
\>
\> suppress(\*exceptions)
\> � � Context manager to suppress specified exceptions
\>
\> � � After the exception is suppressed, execution proceeds with the next
\> � � statement following the with statement.
\>
\> � � � � �with suppress(FileNotFoundError):
\> � � � � � � �os.remove(somefile)
\> � � � � �# Execution still resumes here if the file was already removed
\>
\> -------- current help() output for closing() with does not have a function wrapper --------
\>
\> Help on class closing in module contextlib:
\>
\> class closing(builtins.object)
\> �| �Context to automatically close something at the end of a block.
\> �|
\> �| �Code like this:
\> �|
\> �| � � �with closing(.open()) as f:
\> �| � � � � �
\> �|
\> �| �is equivalent to this:
\> �|
\> �| � � �f = .open()
\> �| � � �try:
\> �| � � � � �
\> �| � � �finally:
\> �| � � � � �f.close()
\> �|
\> �| �Methods defined here:
\> �|
\> �| �\_\_enter\_\_(self)
\> �|
\> �| �\_\_exit\_\_(self, \*exc\_info)
\> �|
\> �| �\_\_init\_\_(self, thing)
\> �|
\> �| �----------------------------------------------------------------------
\> �| �Data descriptors defined here:
\> �|
\> �| �\_\_dict\_\_
\> �| � � �dictionary for instance variables (if defined)
\> �|
\> �| �\_\_weakref\_\_
\> �| � � �list of weak references to the object (if defined)
\>
\>
\>
\>
\> \_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_\_
\> Python-Dev mailing list
\> Python-Dev@python.org
\> https://mail.python.org/mailman/listinfo/python-dev
\> Unsubscribe: https://mail.python.org/mailman/options/python-dev/ncoghlan%40gmail.com