-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
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 our terms of service and privacy 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
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb 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
Outdated
Show resolved
Hide resolved
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).
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)
If files=None
, then copy all files from the source_zipfile
If files
is provided, then copy every file in files
@@ -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 then repacking it, use zipfile to stream the contents of the original wheel to the new wheel, altering the METADATA in the process. This should be 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 function right now (see python/cpython#125718).
Rather than unpacking the wheel into a temporary directory and then repacking it, use zipfile to stream the contents of the original wheel to the new wheel, altering the METADATA in the process. This should be more efficient, especially when dealing with large wheels. It also removes 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 function right now (see python/cpython#125718).
* Use zipfile to copy files on-the-fly in make_variant Rather than unpacking the wheel into a temporary directory and then repacking it, use zipfile to stream the contents of the original wheel to the new wheel, altering the METADATA in the process. This should be more efficient, especially when dealing with large wheels. It also removes 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 function right now (see python/cpython#125718). * Update RECORD file
Added
zipfile.ZipFile.copy_files
to 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
zipfile
module. #113924