msg117476 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-09-27 21:38 |
GzipFile claims to implement BufferedIOBase but doesn't have peek(). |
|
|
msg117491 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-09-27 23:56 |
Here is a first patch, tests still need to be written. |
|
|
msg117552 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-09-28 21:35 |
Same patch with tests. |
|
|
msg117594 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-09-29 10:50 |
Committed in r85100. |
|
|
msg117706 - (view) |
Author: Nir Aides (nirai)  |
Date: 2010-09-30 08:03 |
Hi Antoine, BufferedIOBase is not documented to have peek(): http://docs.python.org/dev/py3k/library/io.html Small note about patch: 1) IOError string says "read() on write-only...", should be "peek() on write-only..." ? 2) Should be min() in self._read(max(self.max_read_chunk, n)) |
|
|
msg117752 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-09-30 17:01 |
Hir Nir, > BufferedIOBase is not documented to have peek(): > http://docs.python.org/dev/py3k/library/io.html Ah, you're right. > Small note about patch: > 1) IOError string says "read() on write-only...", should be "peek() on write-only..." ? Indeed. > 2) Should be min() in self._read(max(self.max_read_chunk, n)) Actually, I think I should have reproduced the algorithm in read(), where there's a read_size distinct from the size requested by the user. Thanks for the review, it looks like I should have waited a bit before committing :) |
|
|
msg117766 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-09-30 22:37 |
Here is a patch fixing these issues. |
|
|
msg117782 - (view) |
Author: Nir Aides (nirai)  |
Date: 2010-10-01 10:01 |
Should be min(n, 1024) instead of max(...) |
|
|
msg117785 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-10-01 11:15 |
> Should be min(n, 1024) instead of max(...) Well, no, because we want to buffer a non-trivial amount of bytes for the next accesses. So, if n < 1024, buffer at least 1024 bytes. |
|
|
msg117786 - (view) |
Author: Nir Aides (nirai)  |
Date: 2010-10-01 11:39 |
Right, I missed the change from self.max_read_chunk to 1024 (read_size). Should not peek() limit to self.max_read_chunk as read() does? |
|
|
msg117788 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-10-01 11:58 |
> Right, I missed the change from self.max_read_chunk to 1024 > (read_size). Should not peek() limit to self.max_read_chunk as read() > does? This is used for the chunking of huge reads, but for peek(): 1) there is no chunking (peek() should do at most one raw read) 2) huge reads are not really the use case peek() is intended for |
|
|
msg117986 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2010-10-04 21:55 |
I've committed the improvements in r85221. Thank you! |
|
|