[issue32158] Suppress (and other contextlib context managers) should work as decorators (where appropriate)

[R. David Murray]

R. David Murray  added the comment:

The designed use case of suppress is to cover a *single line* of python code.  
It should *not* be a decorator.

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32158] Suppress (and other contextlib context managers) should work as decorators (where appropriate)

[Jason R. Coombs]

Jason R. Coombs  added the comment:

In [this question](https://stackoverflow.com/a/48710609/70170), I encounter 
another case where a decorator would be useful. Without the decorator:

def is_docker():
path = '/proc/self/cgroup'
return (
os.path.exists('/.dockerenv')
or os.path.isfile(path)
and any('docker' in line for line in open(path))
)

With the decorator:

@suppress(FileNotFoundError)
def is_docker():
return (
os.path.exists('/.dockerenv')
or any('docker' in line for line in open('/proc/self/cgroup'))
)

The decorator enables several improvements:

- The boolean expression is now two expressions joined by 'or', which is 
semantically easier to parse and thus less prone to error than the three joined 
by and/or.
- There's no longer a need to create a path variable and reference it twice, 
allowing the value to appear inline where it's most relevant.
- The code is one line shorter.
- The body of the function is two lines shorter.
- The key behaviors the function is seeking to achieve are prominently 
presented.

Acknowledged there are two caveats:

- It's unclear the exception really is only expected in the 'open' call.
- In the case where the exception is suppressed, the function will return None, 
which while resolving to boolean False, isn't False.

Those caveats could be addressed, but will sacrifice readability or conciseness.

I don't think this use-case warrants re-opening the ticket or revisiting the 
issue, but I wanted to share for consideration.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32158] Suppress (and other contextlib context managers) should work as decorators (where appropriate)

[Barry A. Warsaw]

Change by Barry A. Warsaw :


--
nosy: +barry

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32158] Suppress (and other contextlib context managers) should work as decorators (where appropriate)

[Jason R. Coombs]

Jason R. Coombs  added the comment:

Fair enough.

For an example, here's the case where I wanted to use the decorator to avoid 
excess indentation and keep the most meaningful part of the function at the 
base of the body:

@suppress(KeyError)
def v12_to_13(manager, case):
case['sample_id'] = case.pop('caseid')


In my opinion, it's nominally nicer and clearer than:

def v12_to_13(manager, case):
with suppress(KeyError):
case['sample_id'] = case.pop('caseid')


But I see your points about encouraging overly-broad catching of exceptions... 
so it's better to have the indentation as something of a wart to dissuade 
excess wrapping.

--
resolution:  -> rejected
stage:  -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32158] Suppress (and other contextlib context managers) should work as decorators (where appropriate)

[Raymond Hettinger]

Raymond Hettinger  added the comment:

> But it got me thinking - couldn't suppress (and possibly other)
> contextlib decorators support this usage out of the box?

They possibly could but probably shouldn't.   My experience is that giving them 
a dual role makes them more complicated and harder to understand.

For suppress() in particular, wrapping a whole function is likely an 
anti-pattern.  Usually we advise people to put as little as possible in the 
try-block to avoid catching unexpected exceptions.

Also, I think it would be inevitable that people would try to apply these to 
generators or awaitables and find that they don't mix well.

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue32158] Suppress (and other contextlib context managers) should work as decorators (where appropriate)

[Jason R. Coombs]

New submission from Jason R. Coombs :

The contextlib docs explain why a decorator is nice:

> It makes it clear that the cm applies to the whole function, rather than just 
> a piece of it (and saving an indentation level is nice, too).

However, the built-in context managers that could readily supply this interface 
don't derive from DecoratorContext.

In jaraco.context, I [added decorator support by simply deriving from the two 
base 
classes](https://github.com/jaraco/jaraco.context/commit/5aa7b0bb222cff5009a2f0dc3ea49db9e7a6b71a#diff-efbedfbbcb7f61268cfeff04a32fa59d).

But it got me thinking - couldn't suppress (and possibly other) contextlib 
decorators support this usage out of the box?

If there's interest, I'll put together a patch with test.

--
components: Library (Lib)
messages: 307155
nosy: jason.coombs
priority: normal
severity: normal
status: open
title: Suppress (and other contextlib context managers) should work as 
decorators (where appropriate)
versions: Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com