bpo-22908: Add seek and tell functionality to ZipExtFile by jjolly · Pull Request #4966 · python/cpython (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation8 Commits10 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

• 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

https://bugs.python.org/issue22908

• 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!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend matching the signature of the same method in the io module: seek(self, pos, whence=0)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

… object returned by ZipFile.open

raises ValueError in that case.

Thanks for your patience. Reviewed and in just under the wire for the 3.7 feature freeze. :)