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 contextlib.redirect_stderr() #66583

Closed
warsaw opened this issue Sep 11, 2014 · 17 comments
Closed

Add contextlib.redirect_stderr() #66583

warsaw opened this issue Sep 11, 2014 · 17 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@warsaw
Copy link
Member

warsaw commented Sep 11, 2014

BPO 22389
Nosy @warsaw, @rhettinger, @ncoghlan, @vstinner, @berkerpeksag, @1st1
Files
  • issue22389.diff
  • issue22389_v2.diff
  • issue22389_v3.diff
  • 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/berkerpeksag'
    closed_at = <Date 2014-11-28.21:29:02.364>
    created_at = <Date 2014-09-11.14:25:13.633>
    labels = ['type-feature', 'library']
    title = 'Add contextlib.redirect_stderr()'
    updated_at = <Date 2014-11-28.21:29:02.363>
    user = 'https://github.com/warsaw'

    bugs.python.org fields:

    activity = <Date 2014-11-28.21:29:02.363>
    actor = 'berker.peksag'
    assignee = 'berker.peksag'
    closed = True
    closed_date = <Date 2014-11-28.21:29:02.364>
    closer = 'berker.peksag'
    components = ['Library (Lib)']
    creation = <Date 2014-09-11.14:25:13.633>
    creator = 'barry'
    dependencies = []
    files = ['36608', '36844', '36874']
    hgrepos = []
    issue_num = 22389
    keywords = ['patch']
    message_count = 17.0
    messages = ['226774', '226775', '226776', '226810', '226811', '226814', '226825', '227656', '227657', '227658', '227792', '228839', '228840', '228878', '229062', '229109', '231830']
    nosy_count = 8.0
    nosy_names = ['barry', 'rhettinger', 'ncoghlan', 'vstinner', 'python-dev', 'berker.peksag', 'yselivanov', 'John Isidore']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue22389'
    versions = ['Python 3.5']

    @warsaw
    Copy link
    Member Author

    warsaw commented Sep 11, 2014

    redirect_stdout is almost exactly what I want, except I want to redirect stderr! redirect_stdout.__init__() should take a 'stream_name' argument (possibly keyword-only) which could be set to 'stderr'. I propose it's implemented as setattr(sys, stream_name, new_target)

    @warsaw warsaw added the stdlib Python modules in the Lib dir label Sep 11, 2014
    @vstinner
    Copy link
    Member

    vstinner commented Sep 11, 2014

    Why not adding a new redirect_stderr() function?

    @warsaw
    Copy link
    Member Author

    warsaw commented Sep 11, 2014

    On Sep 11, 2014, at 02:25 PM, STINNER Victor wrote:

    Why not adding a new redirect_stderr() function?

    With a little refactoring redirect_stdout into a subclass, that would work
    too.

    @rhettinger
    Copy link
    Contributor

    rhettinger commented Sep 12, 2014

    +1 on Victor's suggestion. I don't think hypergeneralizing it is the way to go. That adds too much complexity for too little benefit.

    @vstinner
    Copy link
    Member

    vstinner commented Sep 12, 2014

    redirect_stdout("stderr", stream) looks wrong to be: you want to redirect "stdout" or "stderr"?

    If you want to redirect something else (ex: stdin), you can still implement the very simple pattern:

    old_stdin = sys.stdin
    try:
      sys.stdin = mock_input
      ...
    finally:
      sys.stdin = old_stdin

    By the way, I'm not convinced that we should add redirect_stderr.

    @barry: How many usage of this new functions do you see in the standard library?

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Sep 12, 2014

    I'm fine with adding "redirect_stderr" - better to have the obvious
    counterpart, rather than hypergeneralising, or having to explain why it's
    missing. It's *currently* missing largely on a "wait for someone to ask"
    basis, and Barry asked.

    (Tangentially related, I should do a contextlib2 release at some point, but
    my previous CI provider shut down...)

    @berkerpeksag
    Copy link
    Member

    berkerpeksag commented Sep 12, 2014

    Here's a simple implementation. I will add tests and update the documentation.

    @1st1
    Copy link
    Member

    1st1 commented Sep 26, 2014

    @berker Peksag: The patch looks fine, although I would rename 'redirect_stream' -> '_redirect_stream' or '_RedirectStream'

    @berkerpeksag
    Copy link
    Member

    berkerpeksag commented Sep 26, 2014

    Good point. Will update the patch. Thanks!

    @berkerpeksag berkerpeksag added the type-feature A feature request or enhancement label Sep 26, 2014
    @1st1
    Copy link
    Member

    1st1 commented Sep 26, 2014

    @berker Peksag: Also, please update the docs.

    @JohnIsidore
    Copy link
    Mannequin

    JohnIsidore mannequin commented Sep 29, 2014

    There is stdout_redirected() function [1] that allows to redirect a file object given as stdout patameter including sys.stderr. It works at a file descriptor level i.e. it supports redirecting subprocess' output too but it doesn't work for StringIO (no fd).

    [1] http://stackoverflow.com/questions/4675728/redirect-stdout-to-a-file-in-python/22434262#22434262

    @berkerpeksag
    Copy link
    Member

    berkerpeksag commented Oct 9, 2014

    Here's an updated patch.

    @vstinner vstinner changed the title Generalize contextlib.redirect_stdout Add contextlib.redirect_stderr() Oct 9, 2014
    @vstinner
    Copy link
    Member

    vstinner commented Oct 9, 2014

    issue22389_v2.diff:

    .. function:: redirect_stdout(new_target)
    + redirect_stderr(new_target)

    I would prefer to have two distinct entries in the documentation. The redirect_stderr() doc can be after redirect_stdout() and just say "Similar to redirect_stdout() but redirects sys.stderr".

    @1st1
    Copy link
    Member

    1st1 commented Oct 9, 2014

    I think that Victor is right, it would be better to have two distinct entries in the docs. Besides that - LGTM.

    @rhettinger rhettinger assigned rhettinger and unassigned ncoghlan Oct 11, 2014
    @berkerpeksag
    Copy link
    Member

    berkerpeksag commented Oct 11, 2014

    Good point. Patch updated. Thanks for the reviews!

    @ncoghlan
    Copy link
    Contributor

    ncoghlan commented Oct 12, 2014

    I just pushed the docs fix for issue bpo-21061, so the docs part of the patch may need tweaking in order to apply automatically.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 28, 2014

    New changeset 7f12c9c09fb6 by Berker Peksag in branch 'default':
    Issue bpo-22389: Add contextlib.redirect_stderr().
    https://hg.python.org/cpython/rev/7f12c9c09fb6

    @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 type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants