Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-113924: Add copy from zipfile without decompress/recompressing data#125718
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
gh-113924: Add copy from zipfile without decompress/recompressing data#125718
Conversation
ghost commentedOct 19, 2024 • edited by ghost
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by ghost
Uh oh!
There was an error while loading.Please reload this page.
All commit authors signed the Contributor License Agreement. |
Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Misc/NEWS.d/next/Library/2024-10-18-20-26-01.gh-issue-113924.ObB1IG.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
| f"{caller_name}() requires mode 'w', 'x', or 'a', but " | ||
| f"mode is '{self.mode}'") | ||
| def _copy_file(self, source_zipfile, source_zinfo): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is very strange. In fact, there is not much difference between these methods. What I want to say is that maybe they can be integrated (suggestion, you don't have to take it).
sunrisesarsaparillaOct 20, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I've updated the PR with this alternative API design:
ZipFile.copy_from(source_zipfile, files=None)
Iffiles=None, then copy all files from thesource_zipfile
Iffiles is provided, then copy every file infiles
| @@ -0,0 +1 @@ | |||
| Added `zipfile.ZipFile.copy_files` to allow copying from a zipfile. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
A few things:
- This is reST, not markdown, so backticks should be used with a role: :meth:`zipfile.ZipFile.copy_files` – this is probably the CI failure of sphinx-lint (I guess, because the CI job does not show details!)
- This only works if the target exists: this is the CI failure for docs build
- The new method needs to be documented
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Rather than unpacking the wheel into a temporary directory and thenrepacking it, use zipfile to stream the contents of the original wheelto the new wheel, altering the METADATA in the process. This shouldbe more efficient, especially when dealing with large wheels.Ideally, we would copy other files without recompressing them at all,but unfortunately the zipfile module does not offer such a functionright now (seepython/cpython#125718).
Rather than unpacking the wheel into a temporary directory and thenrepacking it, use zipfile to stream the contents of the original wheelto the new wheel, altering the METADATA in the process. This shouldbe more efficient, especially when dealing with large wheels. It alsoremoves the dependency on `wheel` package.Ideally, we would copy other files without recompressing them at all,but unfortunately the zipfile module does not offer such a functionright now (seepython/cpython#125718).
* Use zipfile to copy files on-the-fly in make_variantRather than unpacking the wheel into a temporary directory and thenrepacking it, use zipfile to stream the contents of the original wheelto the new wheel, altering the METADATA in the process. This shouldbe more efficient, especially when dealing with large wheels. It alsoremoves the dependency on `wheel` package.Ideally, we would copy other files without recompressing them at all,but unfortunately the zipfile module does not offer such a functionright now (seepython/cpython#125718).* Update RECORD file
Uh oh!
There was an error while loading.Please reload this page.
Added
zipfile.ZipFile.copy_filesto allow copying from a zipfile without decompressing and recompressing the data.The commits in this PR are structured to make reading the PR on a commit-by-commit basis straightforward.
Issue:#113924
Discussion:https://discuss.python.org/t/add-copy-between-zipfiles-without-recompressing-files/67544
zipfilemodule. #113924