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

contextlib.redirect_std{out,err} stubs don't yield the new target #4283

Closed
PeterJCLaw opened this issue Jun 27, 2020 · 1 comment · Fixed by #4285
Closed

contextlib.redirect_std{out,err} stubs don't yield the new target #4283

PeterJCLaw opened this issue Jun 27, 2020 · 1 comment · Fixed by #4285

Comments

@PeterJCLaw
Copy link
Contributor

In contextlib, _RedirectStream (the class behind redirect_stdout and redirect_stderr) returns the current stream target as its context variable, which allows code like this:

with redirect_stdout(io.StringIO()) as buffer:
    do_stuff()

use(buffer.getvalue())

where you capture the redirected stream without a separate line to declare the variable.

However the typeshed doesn't allow this, because redirect_stdout is a ContextManager[None].

I think the fix here would be to have the stub be a ContextManager[T], perhaps like:

_T_io = TypeVar('_T_io', bound=Optional[IO[str]])

class redirect_stdout(ContextManager[_T_io]):
    def __init__(self, new_target: _T_io) -> None: ...

I'd be happy to put together a PR if we like this approach.

Aside: this behaviour isn't actually included in the docs, though maybe it should be?

@JelleZijlstra
Copy link
Member

This definitely should be documented! Seems like a useful behavior.

Your proposed change looks reasonable. The Optional was a bit surprising to me, but apparently if you do contextlib.redirect_stdout(None) stdout just gets thrown away.

PeterJCLaw added a commit to PeterJCLaw/typeshed that referenced this issue Jun 28, 2020
This matches the implementation, even though this behaviour isn't
actually documented yet. https://bugs.python.org/issue41147 aims
to fix the documentation issue though.

Fixes python#4283.
JelleZijlstra pushed a commit that referenced this issue Jun 28, 2020
This matches the implementation, even though this behaviour isn't
actually documented yet. https://bugs.python.org/issue41147 aims
to fix the documentation issue though.

Fixes #4283.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants