Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add context manager for the "try: ... except: pass" pattern #60010

Closed
rhettinger opened this issue Aug 29, 2012 · 15 comments
Closed

Add context manager for the "try: ... except: pass" pattern #60010

rhettinger opened this issue Aug 29, 2012 · 15 comments
Assignees
Labels
type-feature A feature request or enhancement

Comments

@rhettinger
Copy link
Contributor

BPO 15806
Nosy @loewis, @warsaw, @rhettinger, @jcea, @ncoghlan, @pitrou, @ericvsmith, @giampaolo, @ezio-melotti, @alex, @cjerdonek
Superseder
  • bpo-19266: Rename contextlib.ignore to contextlib.suppress
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ncoghlan'
    closed_at = <Date 2013-03-11.05:28:04.546>
    created_at = <Date 2012-08-29.05:15:14.779>
    labels = ['type-feature']
    title = 'Add context manager for the "try: ... except: pass" pattern'
    updated_at = <Date 2013-10-16.22:31:55.825>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2013-10-16.22:31:55.825>
    actor = 'belopolsky'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2013-03-11.05:28:04.546>
    closer = 'rhettinger'
    components = []
    creation = <Date 2012-08-29.05:15:14.779>
    creator = 'rhettinger'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 15806
    keywords = []
    message_count = 15.0
    messages = ['169337', '169340', '169341', '169342', '169343', '169346', '169348', '169357', '169358', '169363', '169364', '183934', '183942', '195879', '195890']
    nosy_count = 14.0
    nosy_names = ['loewis', 'barry', 'rhettinger', 'jcea', 'ncoghlan', 'pitrou', 'eric.smith', 'giampaolo.rodola', 'ezio.melotti', 'alex', 'cvrebert', 'chris.jerdonek', 'python-dev', 'zaytsev']
    pr_nums = []
    priority = 'low'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = '19266'
    type = 'enhancement'
    url = 'https://bugs.python.org/issue15806'
    versions = ['Python 3.4']

    @rhettinger
    Copy link
    Contributor Author

    It is a somewhat common pattern to write:

    try:
    do_something()
    except SomeException:
    pass

    To search examples in the standard library (or any other code base) use:

        $ egrep -C2 "except( [A-Za-z]+)?:" *py  | grep -C2 "pass"

    In the Python2.7 Lib directory alone, we find 213 examples.

    I suggest a context manager be added that can ignore specifie exceptions. Here's a possible implementation:

    class Ignore:
        ''' Context manager to ignore particular exceptions'''
    
        def __init__(self, *ignored_exceptions):
            self.ignored_exceptions = ignored_exceptions
    
        def __enter__(self):
            return self
    
        def __exit__(self, exctype, excinst, exctb):
            return exctype in self.ignored_exceptions

    The usage would be something like this:

        with Ignore(IndexError, KeyError):
            print(s[t])

    Here's a real-world example taken from zipfile.py:

        def _check_zipfile(fp):
            try:
                if _EndRecData(fp):
                    return True         # file has correct magic number                                                                        
            except IOError:
                pass
            return False

    With Ignore() context manager, the code cleans-up nicely:

        def _check_zipfile(fp):
            with Ignore(IOError):
                return bool(EndRecData(fp))  # file has correct magic number                                                                        
            return False

    I think this would make a nice addition to contextlib.

    @rhettinger rhettinger added the type-feature A feature request or enhancement label Aug 29, 2012
    @rhettinger
    Copy link
    Contributor Author

    Hmm, the __exit__ method was doing exact matches by exception type, so KeyError wouldn't match LookupError or Exception.

    There are probably a number of ways to fix this, but it may be easiest to use the builtin exception catching mechanisms:

    class Ignore:
        ''' Context manager to ignore particular exceptions'''
    
        def __init__(self, *ignored_exceptions):
            self.ignored_exceptions = ignored_exceptions
    
        def __enter__(self):
            return self
    
        def __exit__(self, exctype, excinst, exctb):
            if exctype is not None:
                try:
                    raise
                except self.ignored_exceptions:
                    return True

    @alex
    Copy link
    Member

    alex commented Aug 29, 2012

    Why not just: issubclass(exctype, self.exception_types)?

    @rhettinger
    Copy link
    Contributor Author

    Yes, something along those lines would be *much* better:

    class Ignore:
        ''' Context manager to ignore particular exceptions'''
    
        def __init__(self, *ignored_exceptions):
            self.ignored_exceptions = ignored_exceptions
    
        def __enter__(self):
            return self
    
        def __exit__(self, exctype, excinst, exctb):
            return exctype and issubclass(exctype, self.ignored_exceptions)

    @ncoghlan
    Copy link
    Contributor

    I'd just write it with @contextmanager. Making it easier to cleanly factor out exception handling is one of the main reasons that exists.

      @contextmanager
      def ignored(*exceptions):
        """Context manager to ignore particular exceptions"""
        try:
            yield
        except exceptions:
            pass

    While the class based version would likely be fractionally faster, the generator based version is more obviously correct.

    @ezio-melotti
    Copy link
    Member

    I think I'm -1 on this.

    This saves two lines, but makes the code less explicit, it can't be used for try/except/else or try/except/except, it requires an extra import, the implementation is simple enough that it doesn't necessary need to be in the stdlib, it might encourage bad style, and it's slower.

    Some of these downsides are indeed somewhat weak, but the upside of saving two lines is quite weak too IMHO.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Aug 29, 2012

    I think the zipfile example is really a bad example. IMO, it would
    best be written as

    try:
    return bool(EndRecData(fp))
    except IOError:
    return False

    i.e. there shouldn't be a pass statement at all in this code, and the if can be dropped whether you use try-except or with.

    @ncoghlan
    Copy link
    Contributor

    (Note: I'm not yet convinced this is a good idea. I'm definitely considering it, though)

    As with many context managers, a key benefit here is in the priming effect for readers. In this code:

    try:
       # Whatever
    except (A, B, C):
       pass
    

    the reader doesn't know that (A, B, C) exceptions will be ignored until the end. The with statement form makes it clear before you start reading the code that certain exceptions won't propagate:

        with ignored(A, B, C):
            # Whatever

    I'm not worried that it makes things less explicit - it's pretty obvious what a context manager called "ignored" that accepts an arbitrary number of exceptions is going to do.

    One other thing it does is interact well with ExitStack - you can stick this in the stack of exit callbacks to suppress exceptions that you don't want to propagate.

    @ezio-melotti
    Copy link
    Member

    As with many context managers, a key benefit here is
    in the priming effect for readers.

    The "focus" is mostly on what it's being executed rather than what it's being ignored though.

    "Do this operation and ignore these exceptions if they occur"
    vs.
    "Ignore these exceptions if they occur while doing this operation."

    I'm not worried that it makes things less explicit - it's pretty
    obvious what a context manager called "ignored" that accepts an
    arbitrary number of exceptions is going to do.

    It's still understandable, but while I'm familiar with the semantics of try/except, I wouldn't be sure if e.g. this just ignored those specific exceptions or even their subclasses without checking the doc/code.

    One other thing it does is interact well with ExitStack - you can
    stick this in the stack of exit callbacks to suppress exceptions that
    you don't want to propagate.

    This seems a good use case.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 29, 2012

    If this is desirable then I think it would be better as a classmethod of Exception:

    with KeyError.trap():
        do_something()

    @ericvsmith
    Copy link
    Member

    While the classmethod version has some appeal, it doesn't extend well to handling multiple exception types.

    I'm -0 on this, in any event. I think the original code is more clear. Why force people to learn (or recognize) a second idiom for something so simple?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 11, 2013

    New changeset 406b47c64480 by Raymond Hettinger in branch 'default':
    Issue bpo-15806: Add contextlib.ignored().
    http://hg.python.org/cpython/rev/406b47c64480

    @ncoghlan
    Copy link
    Contributor

    FTR, Raymond and I discussed this on IRC and I gave it a +1 before he committed it.

    The advantage the callable form has over a class method is that it readily scales to ignoring multiple exception types in a single with statement.

    @zaytsev
    Copy link
    Mannequin

    zaytsev mannequin commented Aug 22, 2013

    Hi Raymond,

    This is a brilliant idea, but before it hits the streets, couldn't you possibly consider extending it with a kwarg to control the depth of the exception stack?

    The use case I have for that are snippets like this:

    with ignored(ValueError, TypeError), ignored(ValueError, TypeError), ignored(ValueError, TypeError):
        a()
        b()
        c()
    

    Or else I could write this as

        with ignored(ValueError, TypeError):
            a()
    
        with ignored(ValueError, TypeError):
            b()
    
        with ignored(ValueError, TypeError):
            c()

    ... but either way it looks bad. This looks a bit better to me:

        with ignored(ValueError, TypeError, depth=3):
            a()
            b()
            c()

    If you deem this to be unacceptably unpythonic, then please ignore my suggestion.

    @zaytsev
    Copy link
    Mannequin

    zaytsev mannequin commented Aug 22, 2013

    Actually, please disregard my idea. It's way to dangerous, especially in the case of multiple exceptions to ignore :-(

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants