Add the class ZipArchiveOperations, which implements archive operations on zip-files
#407
Conversation
This file contains 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
christian-monch
force-pushed
the
zip-archive-operations
branch
2 times, most recently
from
177fccb to
b258777
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #407 +/- ##
==========================================
- Coverage 92.45% 92.07% -0.38%
==========================================
Files 128 124 -4
Lines 9641 9128 -513
Branches 1045 0 -1045
==========================================
- Hits 8914 8405 -509
- Misses 703 723 +20
+ Partials 24 0 -24
☔ View full report in Codecov by Sentry. |
mih
added a commit
to mih/datalad-next
that referenced
this pull request
Jun 10, 2023
Modelled after the `ZipArchiveOperations` tests in datalad#407
mih
added a commit
to mih/datalad-next
that referenced
this pull request
Jun 11, 2023
Modelled after the `ZipArchiveOperations` tests in datalad#407
mih
added a commit
to mih/datalad-next
that referenced
this pull request
Jun 11, 2023
- More sensible `__repr__` for `ArchiveOperations` - Document methods - More flexible handling of archive item names. `open()` and all other such methods can now accept `item.name` from `__iter__` results directly. Based on the assumption that all TAR archives use member names in POSIX notation. - Add unit tests with comprehensive coverage. Modelled after the `ZipArchiveOperations` tests in datalad#407 but with more context manager use.
mih
reviewed
Jun 11, 2023
There was a problem hiding this comment.
Thanks for this PR. It motivated me to implement #415 which addresses a number of insufficiencies that became apparent while reviewing this PR. Please have a look re documentation and flexibility re archive item identifiers. I think it would make sense to align zip and tar support in this regard.
mih
requested changes
Jun 20, 2023
|
Here is the plan:
|
christian-monch
force-pushed
the
zip-archive-operations
branch
from
f52ffc6 to
c179406
Compare
|
The zip file format introduces another naming-related quirk. Directory names in zip-files end with "/". All driectory-items (and file-items) that are yielded by |
|
FMPOV this is ready to go |
This commit adds `ZipArchiveOperations`, a class that implements `ArchiveOperations` for zip files. It makes use of `datalad_next.iter_collections.zipfile.iter_zip` to implement iteration. The commit comes with tests. The test code includes a pytest-fixture called `sample_zip`, which creates a sample zip-file with known content, much like the `sample_tar_xz`-fixture.
This commit fixes a bug in the tests where Unix path-separators were always used to identify directory entries in a zip-archive. That is wrong on Windows, where Windows-path separators have to be used.
This commit ensures that all zip-file names that are used as parameters in methods of ZipArchiveOperations use '/' in names of zip-archive items. That is due to the fact that zip enforces the use '/' in the names of its items (zip-APPNOTE.TXT 6.3.3 from September 1st, 2012). Until PR datalad#409 is merged this will ensure that the proper file names are used to check for items in zip-archives. The commit can be reverted once PR datalad#409 is merged.
This commit removes duplicated code for the sample_zip-fixture. It makes the sample_zip- fixture avaiable through conftest.py and re-uses it on the ZipArchiveOperations tests.
This commit uses PurePosixPath to properly read the path of a zip-archive on Windows in test_open()
This commit renames the file `datalad_next/archive_operations/tests/test_zipfile_archive_operations.py` to `datalad_next/archive_operations/tests/test_zipfile.py`. This is better readable and in sync with the name of the tests for `datalad_next/archive_operations/tarfile.py`
This commit adapts the ZipArchiveOperations code to the current type of `ZipfileItem.name`, i.e. to `PurePosixPath`. The commit also adapts the tests for ZipArchiveOperations to the updated types, and aligns the tests to the test code for TarArchiveOperations. This is in preparation for a test code consolidation
mih
force-pushed
the
zip-archive-operations
branch
from
9bf90b6 to
5fbb2a0
Compare
|
Looks like this PR is not longer up-to-speed with the other developments that happened since it was posted. |
|
Started working on an update |
|
Replaced by #578 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds
ZipArchiveOperations, a class that implementsArchiveOperationsfor zip files. It makes use ofdatalad_next.iter_collections.zipfile.iter_zipto implement iteration.The PR includes tests. The test code comes with a new pytest-fixture called
sample_zip, which creates a sample zip-file with known content, much like thesample_tar_xz-fixture.Remark: while working on this PR, I came across a type-annotation issue #406. I will create a PR to solve #406 later