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

Turn contextlib.{redirect_stdout,suppress} into ContextDecorators #66721

Closed
anntzer mannequin opened this issue Oct 1, 2014 · 3 comments
Closed

Turn contextlib.{redirect_stdout,suppress} into ContextDecorators #66721

anntzer mannequin opened this issue Oct 1, 2014 · 3 comments
Labels
stdlib Python modules in the Lib dir

Comments

@anntzer
Copy link
Mannequin

anntzer mannequin commented Oct 1, 2014

BPO 22531
Nosy @ncoghlan, @bitdancer, @anntzer

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 = None
closed_at = <Date 2014-10-01.14:55:08.557>
created_at = <Date 2014-10-01.09:50:23.843>
labels = ['library']
title = 'Turn contextlib.{redirect_stdout,suppress} into ContextDecorators'
updated_at = <Date 2014-10-01.14:55:08.555>
user = 'https://github.com/anntzer'

bugs.python.org fields:

activity = <Date 2014-10-01.14:55:08.555>
actor = 'ncoghlan'
assignee = 'none'
closed = True
closed_date = <Date 2014-10-01.14:55:08.557>
closer = 'ncoghlan'
components = ['Library (Lib)']
creation = <Date 2014-10-01.09:50:23.843>
creator = 'Antony.Lee'
dependencies = []
files = []
hgrepos = []
issue_num = 22531
keywords = []
message_count = 3.0
messages = ['228065', '228075', '228078']
nosy_count = 3.0
nosy_names = ['ncoghlan', 'r.david.murray', 'Antony.Lee']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue22531'
versions = ['Python 3.4', 'Python 3.5']

@anntzer
Copy link
Mannequin Author

anntzer mannequin commented Oct 1, 2014

A small lib improvement suggestion could be to make contextlib.redirect_stdout and contextlib.suppress inherit from ContextDecorator.

As a side note, the source of contextlib has some classes inheriting explicitly from object while others don't, so perhaps this can be harmonized at the same time.

@anntzer anntzer mannequin added the stdlib Python modules in the Lib dir label Oct 1, 2014
@bitdancer
Copy link
Member

-100 on doing this for suppress. That would be exactly the kind of wrong-headed code that I was worried that context manager would invite.

@ncoghlan
Copy link
Contributor

ncoghlan commented Oct 1, 2014

Aye, suppress is only intended for use around a single line of code. Using it for an entire function would be "OnError Resume Next" levels of poor coding style.

I'm also -1 on implicitly wrapping redirect_stdout around functions due to the immediate thread safety problem doing so introduces. The stdout redirection really only makes sense in a single-threaded scripting context, and using it as a decorator rather than inline makes it far too easy to inadvertently violate that constraint.

As far as the "explicitly inherit from object or not" goes, it wouldn't surprise me if that's just a "migrated from Python 2" vs "first introduced in Python 3" distinction. It's not something I would change solely for the sake of consistency.

@ncoghlan ncoghlan closed this as completed Oct 1, 2014
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir
Projects
None yet
Development

No branches or pull requests

2 participants