bpo-22908: Add seek and tell functionality to ZipExtFile #4966
Conversation
- Added seek, tell, and seekable functions to ZipExtFile
- Added internal variables to ZipExtFile that preserves original
values in order to reset the zipfile to it's initial state
- Raises io.UnsupportedOperation when accessed without a seekable file
object
- Could be optimized to seek within the _readbuffer
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
Lib/zipfile.py
Outdated
| def seekable(self): | ||
| return self._seekable | ||
|
|
||
| def seek(self, offset, from_what = 0): |
There was a problem hiding this comment.
I would recommend matching the signature of the same method in the io module: seek(self, pos, whence=0)
There was a problem hiding this comment.
I chose offset because I can better differentiate between offset value (relative values) and position values (absolute values). Also, according to the official documentation "offset" should be the first argument for seek (although there is no consensus when I search through the cpython source). https://docs.python.org/3/library/io.html#io.IOBase.seek
I agree with changing "from_what" to "whence". The former always felt awkward to me. Thank you for the suggestion
There was a problem hiding this comment.
It seems like older modules use pos (e.g. mmap), and the more recent io ABCs use offset. This should be fine, and I expect people to pass the parameter as a positional argument anyway.
|
Found a problem during seek where the python executable was crashing with a segfault. When advancing the file pointer by reading from the file, seek was reading the entire file - which is A Bad Thing(tm). Seek now reads a block at a time (set at 16MB). Also made change as suggested by @merwok. Thanks. |
|
It's been a month since submission. Is there any update for this PR? If I should wait further, let me know and I'll wait patiently. |
raises ValueError in that case.
|
Thanks for your patience. Reviewed and in just under the wire for the 3.7 feature freeze. :) |
|
Excellent! Thank you. |
values in order to reset the zipfile to it's initial state
object
https://bugs.python.org/issue22908