read() vs read1() in asyncio.StreamReader documentation

[oconnor663]

BPO	22279
Nosy	@gvanrossum, @vstinner, @1st1, @oconnor663

^{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 = None
created_at = <Date 2014-08-26.18:36:38.980>
labels = ['type-bug', 'expert-asyncio']
title = 'read() vs read1() in asyncio.StreamReader documentation'
updated_at = <Date 2014-08-27.17:45:18.037>
user = 'https://github.com/oconnor663'

bugs.python.org fields:

activity = <Date 2014-08-27.17:45:18.037>
actor = 'gvanrossum'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2014-08-26.18:36:38.980>
creator = 'oconnor663'
dependencies = []
files = []
hgrepos = []
issue_num = 22279
keywords = []
message_count = 8.0
messages = ['225924', '225925', '225933', '225937', '225942', '226001', '226002', '226003']
nosy_count = 4.0
nosy_names = ['gvanrossum', 'vstinner', 'yselivanov', 'oconnor663']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22279'
versions = ['Python 3.4']

[oconnor663]

BufferedIOBase and related classes have a read(n) and read1(n). The first will wait until n bytes are available (or EOF), while the second will return as soon as any bytes are available. In asyncio.StreamReader, there is no read1 method, but the read method seems to behave like BufferedIOBase.read1, and there is a readexactly method that behaves like BufferedIOBase.read.

Is there a reason for this naming change? Either way, could the documentation for asyncio.StreamReader.read be clarified?

[gvanrossum]

Good point. I think I had forgotten how BufferedIOBase worked... :-(

I believe we should just change this -- Victor, what do you think?

[vstinner]

Guido, what do you mean by changing this? Rename methods? What about the
backward compatibility? Is it possible that an application written with the
"read1 behaviour" blocks if read(n) waits for exactly n bytes?

I like the idea of having the same names in io and asyncio modules (even
the name of the two modules are close).

If we want an API close to the io module, do we need 3 layers? FileIO,
BufferedReader and TextIOWrapper.

[gvanrossum]

Oh, I just checked the docs. io.BufferedIOBase.read(size) is more complicated than I (or Jack) thought -- it can return a short read if the underlying raw stream is "interactive". The subclass io.BufferedReader uses the definition preferred by Jack (though the sentence is malformed and it's not entirely clear to what the "if the read call would block in non-blocking mode" applies -- but my best guess is that it only applies if size is not given or negative).

The backward compatibility issue is difficult though. It looks like examples/echo_server_tulip.py needs to be updated, which suggests a lot of 3rd party code might have to...

[gvanrossum]

Also, I'm not too fond of the read1()/read() dichotomy. I was pretty much forced into it because Python 2 streams implemented read() using the libc fread() function, which has this behavior -- but in general I find the UNIX read() syscall more convenient, and I wish I could turn time back on this issue. I guess my design for asyncio.StreamReader is what I wish the io classes used...

[gvanrossum]

Loet's just spruce up the docs a bit. (Also maybe fix that awkward sentence in the BufferedReader docs.)

[oconnor663]

Agreed that changing read() would probably break tons of people. I don't think a naming inconsistency meets the "serious flaws are uncovered" bar for breaking a provisional package. If we actually prefer the asyncio way of doing things, all the better.

That said, another thing I'm noticing is that in asyncio, read is basically two different functions. This is clear in the code, http://hg.python.org/cpython/file/fb3aee1cff59/Lib/asyncio/streams.py#l433, where the n<0 case goes off on its own branch and never comes back. (Incidentally there's another n<0 check at line 453 there that I think always returns false.) We have a read function that makes very different guarantees depending on the value of n. Contrast this with the read function from regular io, where read(n) and read() are effectively the same if n is large enough. Maybe just another point that's worth clarifying in the docs.

Thanks for the quick replies!

[gvanrossum]

Yeah, that too should just be documented. The read-until-eof behavior is
quite entrenched (in fact I had a hard time finding example read calls with
a size parameter :-). Specifying a huge buffer size in order to read until
EOF isn't really a common practice, but definitely worth pointing out in
the docs.

On Wed, Aug 27, 2014 at 10:22 AM, Jack O'Connor <report@bugs.python.org>
wrote:

Jack O'Connor added the comment:

Agreed that changing read() would probably break tons of people. I don't
think a naming inconsistency meets the "serious flaws are uncovered" bar
for breaking a provisional package. If we actually prefer the asyncio way
of doing things, all the better.

That said, another thing I'm noticing is that in asyncio, read is
basically two different functions. This is clear in the code,
http://hg.python.org/cpython/file/fb3aee1cff59/Lib/asyncio/streams.py#l433,
where the n<0 case goes off on its own branch and never comes back.
(Incidentally there's another n<0 check at line 453 there that I think
always returns false.) We have a read function that makes very different
guarantees depending on the value of n. Contrast this with the read
function from regular io, where read(n) and read() are effectively the same
if n is large enough. Maybe just another point that's worth clarifying in
the docs.

Thanks for the quick replies!

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue22279\>

---

[willingc]

Closing this as a docs issue for asyncio.StreamReader as read(n) is currently documented in asyncio docs.

[savannahostrowski]

I see that this was moved to Done in the asyncio board but was never actually closed out. @willingc Is there anything else remaining here, or should this issue be closed as completed?