homepage

The documented behavior of io.BufferedReader.peek([n]) states:
peek([n])
Return 1 (or n if specified) bytes from a buffer without advancing the 
position.
Thereas the parameter n is the _max_ length of returned bytes.
Implementation is:
 def _peek_unlocked(self, n=0):
 want = min(n, self.buffer_size)
 have = len(self._read_buf) - self._read_pos
 if have < want:
 to_read = self.buffer_size - have
 current = self.raw.read(to_read)
 if current:
 self._read_buf = self._read_buf[self._read_pos:] + 
current
 self._read_pos = 0
 return self._read_buf[self._read_pos:]
Where you may see that the parameter n is the _min_ length
of returned bytes. So peek(1) will return _not_ just 1 Byte, but the
remaining bytes in the buffer.

Note: this is also in Python 2.6

Proposed patch to fix this:
set the default of n to 1 as stated by docs:
def _peek_unlocked(self, n=1):
return n bytes:
return self._read_buf[self._read_pos:self._read_pos+n]

Assigned to Benjamin for assessment - this should be considered for rc2
since it's still broken in 3.1:
>>> f = open('setup.py', 'rb')
>>> len(f.peek(10))
4096
>>> len(f.peek(1))
4096
>>> len(f.peek(4095))
4096
>>> len(f.peek(10095))
4096
Brought up on python-dev in this thread:
http://mail.python.org/pipermail/python-dev/2009-June/089986.html
And previously here:
http://mail.python.org/pipermail/python-dev/2009-April/088229.html
The thread from April suggests the current behaviour may be intentional,
in which case it is the documentation that needs to be fixed, as it is
currently not just misleading but flat out wrong. Then again, Benjamin's
initial response to that thread was to support the idea of changing
peek() so that the argument actually was a cap.
The previous documentation that Alexandre quotes in the April was
changed to the current description in late April without any
corresponding change to the implementation:
http://svn.python.org/view/python/branches/py3k/Doc/library/io.rst?r1=62422&r2=62430
However, the old description was also wrong for the io-c implementation
since it just returns the current buffered data from peek, no matter
what argument you pass in.

I think the argument should be used as a upper bound; I will look at
this tomorrow.

Hey guys, I did a patch about this one.
I didn't do many tests but I guess it is ok (it works like I think it
should).
What do you think?

Oops I overlooked I minor flaw.
A second version.

There's a problem with my patch... When the size of the data we want to
peek is too big ( > buffer_len - start ) the cursor will move, thus
there isn't a case where the peek function would work properly (except
when we want to peek() just 1 byte).
Couldn't we use a read() followed by a seek() instead?

Lucas, it is indeed impossible for peek() to return more than the buffer
size and remain compatible with non-seekable raw streams. That's why it
/never/ returns more than the buffer size.
As for the fact that peek() doesn't behave as documented, I disagree.
Here is what the docstring says:
 """Returns buffered bytes without advancing the position.
 The argument indicates a desired minimal number of bytes; we
 do at most one raw read to satisfy it. We never return more
 than self.buffer_size.
 """
Please note : "a desired /minimal/ number of bytes" (minimal, not
maximal). Furthermore, "We never return more than self.buffer_size." The
behaviour looks ok to me.

We could fill the buffer while moving its start point to 0. I guess this
behavior would require a new function (or a new parameter to
Modules/_io/bufferedio.c:_bufferedreader_fill_buffer() ).
If you are ok with that I could write a patch.

We could, however, enforce the passed argument and only return the whole
remaining buffer when no argument is given. This is a bit like Frederick
Reeve's proposal on python-dev, but less sophisticated and therefore
less tedious to implement.
In any case, I'm not sure it should be committed before the 3.1 release.
The second and last release candidate is supposed to be today.

> We could fill the buffer while moving its start point to 0. I guess this
> behavior would require a new function (or a new parameter to
> Modules/_io/bufferedio.c:_bufferedreader_fill_buffer() ).
> If you are ok with that I could write a patch.
The buffer is used for both reading and writing and you have to be
careful when shifting it. Besides, the same change (or similar) should
also be done in the Python implementation (in _pyio.py).
If you come up with a patch, please add some tests and check the whole
regression suite passes.

I'm downgrading this because it can't be changed until after 3.1 is
released.

It's not the docstring that is wrong for the current behaviour, it's the
IO.BufferedReader documentation:
"""
peek([n])
 Return 1 (or n if specified) bytes from a buffer without advancing
the position. Only a single read on the raw stream is done to satisfy
the call. The number of bytes returned may be less than requested since
at most all the buffer’s bytes from the current position to the end are
returned.
"""
That gives absolutely no indication that the call might return more
bytes than expected, and the indication that leaving out the argument
will return only the next byte is flat out wrong.

I updated the documentation in r73429. Is that better?

Rather than "only a single read on the raw stream", it should be "at
most a single read on the raw stream", IMHO.

Here is a patch that passes all the tests (I had to change some of them
though, they were expecting erroneous behaviours IMHO).
The biggest problem was the read1 testing, I've tried to get the maximum
of bytes less than or equal to what the user wanted while executing at
most 1 raw_read()'s.
I have created a new test for peek()'ing a number of bytes bigger than
could possibly be stored on the buffer.

Here, it's a patch that passes all the tests (I had to change some of them
though, they were expecting erroneous behaviours IMHO).
The biggest problem was the read1 testing, I've tried to get the maximum
of bytes less than or equal to what the user wanted while executing at
most 1 raw_read()'s.
I have created a new test for peek()'ing a number of bytes bigger than
could possibly be stored on the buffer.

I haven't read the patch in detail but I don't think you should have
changed read1(). read1() is there for optimization purposes and its
current behaviour makes sense IMO.

The doc revision definitely does a better job of characterising the
current underspecified behaviour :)
I agree with Antoine that "at most a single read" would be better wording.

Ok
A new patch without read1() changes.
Only one test fails, a read1() test:
======================================================================
FAIL: test_read1 (test.test_io.PyBufferedRWPairTest)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/home/lucas/Codes/python-stuff/py3k/Lib/test/test_io.py", line
1139, in test_read1
 self.assertEqual(pair.read1(3), b"abc")
AssertionError: b'a' != b'abc'
Since I've changed peek_unlocked() (which is used once by read1()), I
guess there's a problem with read1() expectations about it.

Looks like this has slipped under the radar. I'll leave working on it to the experts :)

Is the current documentation as accurate as it can be?
"The number of bytes returned may be less or more than requested"
To me this has always made this method practically useless. A valid implementation could just always return b"". I noticed the BZ2File.peek() documentation (BZ2File is apparently trying to imitate BufferedReader) is slightly more useful:
"At least one byte of data will be returned (unless at EOF)"
That could be used for (say) peeking for a LF following a CR. But still the "size" parameter does not seem very useful. In fact, LZMAFile.peek() says the size is ignored.

Here is a simple documentation patch to guarantee that at least one byte is normally returned. This would make the method much more useful, and compatible with the BZ2File and LZMAFile interfaces, allowing them to use BufferedReader, as I propose to do in Issue 15955.
Even if nobody is interested in Torsten’s patch to limit the return length, I suggest my patch be considered :)

The non-blocking behaviour that I documented in my patch is under question in Issue 13322. I think it would be nice change the implementation to either return None or raise BlockingIOError.