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

Documentation Recommends Broken Pattern #65563

Closed
mitsuhiko opened this issue Apr 27, 2014 · 14 comments
Closed

Documentation Recommends Broken Pattern #65563

mitsuhiko opened this issue Apr 27, 2014 · 14 comments
Labels
topic-IO type-bug An unexpected behavior, bug, or error

Comments

@mitsuhiko
Copy link
Member

BPO 21364
Nosy @pitrou, @benjaminp, @mitsuhiko, @bitdancer, @florentx, @hynek, @vadmium

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-05-15.20:40:42.578>
created_at = <Date 2014-04-27.15:03:53.209>
labels = ['type-bug', 'expert-IO']
title = 'Documentation Recommends Broken Pattern'
updated_at = <Date 2014-05-15.21:55:06.473>
user = 'https://github.com/mitsuhiko'

bugs.python.org fields:

activity = <Date 2014-05-15.21:55:06.473>
actor = 'r.david.murray'
assignee = 'none'
closed = True
closed_date = <Date 2014-05-15.20:40:42.578>
closer = 'pitrou'
components = ['IO']
creation = <Date 2014-04-27.15:03:53.209>
creator = 'aronacher'
dependencies = []
files = []
hgrepos = []
issue_num = 21364
keywords = []
message_count = 14.0
messages = ['217270', '217286', '217741', '218580', '218581', '218626', '218627', '218628', '218629', '218631', '218632', '218633', '218634', '218635']
nosy_count = 9.0
nosy_names = ['pitrou', 'benjamin.peterson', 'stutzbach', 'aronacher', 'r.david.murray', 'flox', 'python-dev', 'hynek', 'martin.panter']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21364'
versions = ['Python 3.4', 'Python 3.5']

@mitsuhiko
Copy link
Member Author

The documentation recommends replacing sys.stdin with a binary stream currently: https://docs.python.org/3/library/sys.html#sys.stdin

This sounds like a bad idea because it will break pretty much everything in Python in the process.

As example:

>>> import sys
>>> sys.stdin = sys.stdin.detach()
>>> input('Test: ')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: '_io.BufferedReader' object has no attribute 'errors'

>>> sys.stdout = sys.stdout.detach()
>>> print('Hello World!')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'str' does not support the buffer interface

@florentx florentx mannequin added topic-IO type-bug An unexpected behavior, bug, or error labels Apr 27, 2014
@pitrou
Copy link
Member

pitrou commented Apr 27, 2014

Initial introduction is 59cb9c074e09.

@vadmium
Copy link
Member

vadmium commented May 2, 2014

That particular revision isn’t sound so bad; I think the next revision, 78fb7f8cd349, which adds the make_streams_binary() function that replaces the variables is a worry though.

This is the kind of code I use when I want to write binary data to stdout:

output = sys.stdout.detach()
sys.stdout = None  # Avoid error message during shutdown, due to output being closed or garbage-collected
output.write(...)

This raises two issues:

  1. Is the practice of setting sys.stdout to None documented or recommended anywhere? If not, could it be, or is there another way to do this?

  2. The error message I am avoiding looks more broken than it could be:

$ python3 -c 'from sys import stdout; stdout.detach().close()'
Exception ignored in: $

The trailing dollar ($) sign just indicates that there was no newline printed.

@bitdancer
Copy link
Member

The issue of (mixed) string and binary input/output on the standard streams is still a bit of a work in progress, I think, both documentation wise and code wise. So I'm not sure we know yet what the best practice is to recommend here.

I think I agree with Armin, though. I think mentioning reading binary from sys.stdin.buffer is fine, but suggesting replacing the streams is bad. If someone figures that out on their own, it's on their own head, but I don't think we should suggest it.

That said, that shutdown error message is probably a bug of one sort or another.

@mitsuhiko
Copy link
Member Author

Sidestepping: The shutdown message is a related issue. TextIOWrapper tends to internally log errors apparently which is super annoying and probably should be fixed. I encountered the same problem with sockets disconnecting wrapped in TextIOWrapper always writing some dummy traceback to stderr which I can't silence.

@python-dev
Copy link
Mannequin

python-dev mannequin commented May 15, 2014

New changeset 4621bb82ceec by Antoine Pitrou in branch '3.4':
Issue bpo-21364: remove recommendation of broken pattern.
http://hg.python.org/cpython/rev/4621bb82ceec

New changeset dbf728f9a2f0 by Antoine Pitrou in branch 'default':
Issue bpo-21364: remove recommendation of broken pattern.
http://hg.python.org/cpython/rev/dbf728f9a2f0

@pitrou
Copy link
Member

pitrou commented May 15, 2014

Thanks for the report, Armin. I've removed that recommendation and changed the surrounding wording to insist that standard streams are always text streams.

@pitrou pitrou closed this as completed May 15, 2014
@mitsuhiko
Copy link
Member Author

To avoid further problems may I also recommend documenting how exactly people are supposed to wrap sys.stdout and so forth. Clearly putting a StringIO there is insufficient as StringIO does not have a buffer.

Something like this maybe?

import io
buf = io.BytesIO()
sys.stdout = io.TextIOWrapper(buf,
    encoding='utf-8',
    errors='strict', # or surrogate-escape as this is the default for stdout now? not sure
    line_buffering=True
)

@pitrou
Copy link
Member

pitrou commented May 15, 2014

To avoid further problems may I also recommend documenting how exactly
people are supposed to wrap sys.stdout and so forth. Clearly putting
a StringIO there is insufficient as StringIO does not have a buffer.

I would like to know of some situations where you want to write some
code that accesses standard streams as binary *and* don't control the
application setup (i.e. library code rather than application code). It
seems to me that a library should take the binary streams as parameters
rather than force the use of stdin/stdout.

@mitsuhiko
Copy link
Member Author

I would like to know of some situations where you want to write some
code that accesses standard streams as binary *and* don't control the
application setup (i.e. library code rather than application code). It
seems to me that a library should take the binary streams as parameters
rather than force the use of stdin/stdout.

The same situations people wrapped streams before on python 2:

  • code.py users. Werkzeug's traceback system implements a remote python
    shell through it.
  • any system that wants to unittest shell scripts on a high level.
  • any system that wants to execute arbitrary python code and then
    capture whatever output it did. This is for instance what I see
    Sphinx users frequently do (or doctests)

In fact, the reasons people wrap sys.stdout/sys.stderr on 2.x are the same reasons why people would do it on 3.x: they have arbitrary code they did not write and they want to capture what it does.

Since you do not know if that is binary or text you need a stream object that behaves the same way as the default one.

@pitrou
Copy link
Member

pitrou commented May 15, 2014

The same situations people wrapped streams before on python 2:

  • code.py users. Werkzeug's traceback system implements a remote python
    shell through it.
  • any system that wants to unittest shell scripts on a high level.
  • any system that wants to execute arbitrary python code and then
    capture whatever output it did. This is for instance what I see
    Sphinx users frequently do (or doctests)

I see, I misunderstood you. You actually want to get back the bytes
output of e.g. stdout, right?
You could indeed use a TextIOWrapper wrapping a BytesIO object. Or of
course another possibility is to do the encoding yourself, e.g.
sys.stdout.getvalue().encode('utf-8', 'surrogateescape').

@mitsuhiko
Copy link
Member Author

Pretty much, yes. Just that you probably want 'replace' instead.
surrogate-escape does not do anything useful here I think.

@bitdancer
Copy link
Member

Note that in 3.4 we have contextlib.replace_stdout, but it doesn't give any examples of how to construct file-like objects that will work well with it.

@bitdancer
Copy link
Member

I mean redirect_stdout.

@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
topic-IO type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants