classification
Title: GzipFile doesn't have peek()
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: nirai, pitrou
Priority: normal Keywords: patch

Created on 2010-09-27 21:38 by pitrou, last changed 2010-10-04 21:55 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
gzippeek.patch pitrou, 2010-09-27 23:56
gzippeek2.patch pitrou, 2010-09-28 21:35 review
gzipfixup.patch pitrou, 2010-09-30 22:37
Messages (12)
msg117476 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-27 21:38
GzipFile claims to implement BufferedIOBase but doesn't have peek().
msg117491 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-27 23:56
Here is a first patch, tests still need to be written.
msg117552 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-09-28 21:35
Same patch with tests.
msg117594 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2010-10-04 21:55
I've committed the improvements in r85221. Thank you!
History
Date User Action Args
2010-10-04 21:55:55pitrousetstatus: open -> closed

messages: + msg117986
2010-10-01 11:58:00pitrousetmessages: + msg117788
2010-10-01 11:39:24niraisetmessages: + msg117786
2010-10-01 11:15:58pitrousetmessages: + msg117785
2010-10-01 10:01:54niraisetmessages: + msg117782
2010-09-30 22:37:01pitrousetfiles: + gzipfixup.patch

messages: + msg117766
2010-09-30 22:26:03pitrousetstatus: closed -> open
2010-09-30 17:01:14pitrousetmessages: + msg117752
2010-09-30 08:03:05niraisetmessages: + msg117706
2010-09-29 10:50:07pitrousetstatus: open -> closed
resolution: fixed
messages: + msg117594

stage: patch review -> resolved
2010-09-28 22:04:01pitrousetstage: needs patch -> patch review
2010-09-28 21:35:10pitrousetfiles: + gzippeek2.patch

messages: + msg117552
2010-09-27 23:56:49pitrousetfiles: + gzippeek.patch
keywords: + patch
messages: + msg117491
2010-09-27 21:38:34pitroucreate