Sketch of a base class for `ArchiveOperations` #372

mih · 2023-05-24T08:17:27Z

Related to #184 but different in scope and aims.

datalad_next/archive_operations/__init__.py

codecov · 2023-05-24T09:39:40Z

Codecov Report

Patch coverage: 95.06% and project coverage change: +0.05 🎉

Comparison is base (85250d5) 92.27% compared to head (487cc58) 92.32%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #372      +/-   ##
==========================================
+ Coverage   92.27%   92.32%   +0.05%     
==========================================
  Files          98      107       +9     
  Lines        7820     8005     +185     
==========================================
+ Hits         7216     7391     +175     
- Misses        604      614      +10

Impacted Files	Coverage Δ
datalad_next/iter_collections/tarfile.py	`100.00% <ø> (ø)`
datalad_next/archive_operations/__init__.py	`75.86% <75.86%> (ø)`
datalad_next/archive_operations/tarfile.py	`95.00% <95.00%> (ø)`
datalad_next/archive_operations/zipfile.py	`95.12% <95.12%> (ø)`
datalad_next/archive_operations/tests/common.py	`100.00% <100.00%> (ø)`
...perations/tests/test_tarfile_archive_operations.py	`100.00% <100.00%> (ø)`
...perations/tests/test_zipfile_archive_operations.py	`100.00% <100.00%> (ø)`
...lad_next/commands/tests/test_ls_file_collection.py	`100.00% <100.00%> (ø)`
...atalad_next/iter_collections/tests/test_iterdir.py	`100.00% <100.00%> (ø)`
...atalad_next/iter_collections/tests/test_itertar.py	`100.00% <100.00%> (ø)`
... and 3 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

This commit adds a zipfile collection iterator including some basic tests. This is a preparation for the implementation of zip archive operations

This commit adds a first version of ZipArchiveOperations, i.e. an implementation of archive-operations on ZIP files. TODO: tests

This commit changes comments to reflect the current functionality of the code. In addition it reduces the cognitive complexity of iter_zip, as measured by "code-climate".

This commit changes a few more comments that were lying about the code.

This commit adds tests for archive operations. In addition it refactors the iter_collection tests and improves the sample_<collection/archive> fixtures.

datalad_next/archive_operations/__init__.py

mih · 2023-05-25T04:56:05Z

datalad_next/archive_operations/tarfile.py

+)
+
+from . import ArchiveOperations
+from ..config import ConfigManager


The must not be relative importing across datalad_next subpackages. Only within subpackage.

mih · 2023-05-25T04:56:27Z

datalad_next/archive_operations/tests/common.py

+from __future__ import annotations
+
+from .. import ArchiveOperations
+from ...iter_collections.tests.utils import TestCollection


Must be absolute

mih · 2023-05-25T04:57:24Z

datalad_next/archive_operations/tests/test_tarfile_archive_operations.py

+
+from .common import check_basic_functionality
+from ..tarfile import TarArchiveOperations
+from ...iter_collections.tests.utils import (


Must be absolute

mih · 2023-05-25T04:57:35Z

datalad_next/archive_operations/tests/test_zipfile_archive_operations.py

+
+from .common import check_basic_functionality
+from ..zipfile import ZipArchiveOperations
+from ...iter_collections.tests.utils import (


Must be absolute

mih · 2023-05-25T04:57:46Z

datalad_next/archive_operations/zipfile.py

+)
+
+from . import ArchiveOperations
+from ..config import ConfigManager


Must be absolute

Co-authored-by: Michael Hanke <michael.hanke@gmail.com>

This commit converts relative import across datalad_next subpackages into absolute imports

mih · 2023-05-25T07:31:17Z

datalad_next/archive_operations/__init__.py

+
+    @contextmanager
+    @abstractmethod
+    def open(self, item: Any) -> IO:


Likely needs to become

Suggested change

def open(self, item: Any) -> IO:

def open(self, item: PurePosixPath) -> IO:

to set concrete expectations. The archivist/datalad-archives use case will always come with such paths. I am not sure of particular archive formats use non-POSIX paths internally, but knowing the type of input path should help make any transformations that are necessary.

Are there any archives that use non-path item/member identification schemes?

mih · 2023-05-25T07:33:47Z

datalad_next/archive_operations/__init__.py

+        raise NotImplementedError
+
+    @abstractmethod
+    def __iter__(self) -> Generator[FileSystemItem, None, None]:


I am not convinced that this is a good choice, but maybe it is. We are operating under the assumption that all iterator will produce that. I have no good feeling whether that is valid.

Maybe being too strict is actually better -- need to think about this more.

I share a "slightly weird" feeling. It is better than TarfileItem. I think we have to answer a general question:

Should the yield-type of ArchiveOperations.__iter__ and of iter_<x> be in the same class hierarchy?

For example:

# The name below is stupid and just used to illustrate the point @dataclass class CollectionOrArchiveItem: name: PurePosixPath @dataclass class FilesystemItem(CollectionOrArchiveItem): type: FileSystemItemType size: int mtime: float | None = None mode: int | None = None uid: int | None = None gid: int | None = None link_target: PurePath | None = None

Pro: it would make sense because both are members of some of set that distinguishes its elements by their name (although the elements usually have additional properties, the names are the unique distinction criteria). So there could be a common base-class for those types that basically contains a name.

Con: there are distinctions between the names in archives and names in collections. Names in archives are specified by the archive specs and should be identical across platforms. Names in collections might not be identical across platforms, i.e. Windows-dirs vs. Unix-dirs, unless they are represented as strings or a sequence of bytes (which we are currently doing)

I arrived at similar conclusions. Please check #350 and specifically 63d418e for my approach from 1-2 weeks ago.

mih · 2023-05-29T15:19:49Z

I am closing this PR now. The actual "Sketch of a base class for ArchiveOperations" has already been merged into main with #384. Other aspects of the diff here have been proposed at separate PR(s):

iter_zip() #383

The remaining pieces also need to go down this path.

christian-monch reviewed May 24, 2023

View reviewed changes

datalad_next/archive_operations/__init__.py Outdated Show resolved Hide resolved

christian-monch reviewed May 24, 2023

View reviewed changes

datalad_next/archive_operations/__init__.py Show resolved Hide resolved

mih added 2 commits May 24, 2023 11:11

Sketch of a base class for ArchiveOperations

ebc8a9d

Sketch of TarArchiveOperations

b410ea6

mih force-pushed the archives-the-3rd branch from 274ff17 to b410ea6 Compare May 24, 2023 09:11

christian-monch added 8 commits May 24, 2023 12:54

add a zipfile-collection iterator including tests

ed4a071

This commit adds a zipfile collection iterator including some basic tests. This is a preparation for the implementation of zip archive operations

add zip-archive operations

2d7c91b

This commit adds a first version of ZipArchiveOperations, i.e. an implementation of archive-operations on ZIP files. TODO: tests

fix comments and reduce iter_zip complexity

9301157

This commit changes comments to reflect the current functionality of the code. In addition it reduces the cognitive complexity of iter_zip, as measured by "code-climate".

modify comments to reflect code

346cb21

This commit changes a few more comments that were lying about the code.

Merge branch 'main' into archives-the-3rd

09976b1

remove an unused import

b50e5ca

add archive operation tests, refactor tests

6c1269d

This commit adds tests for archive operations. In addition it refactors the iter_collection tests and improves the sample_<collection/archive> fixtures.

adapt tests to changes sample_tar_xz type

b2eaf21

mih commented May 25, 2023

View reviewed changes

christian-monch and others added 2 commits May 25, 2023 08:26

Update datalad_next/archive_operations/__init__.py

1ca06a3

Co-authored-by: Michael Hanke <michael.hanke@gmail.com>

convert relative imports to absolute imports

487cc58

This commit converts relative import across datalad_next subpackages into absolute imports

mih commented May 25, 2023

View reviewed changes

This was referenced May 26, 2023

iter_zip() #383

Merged

datalad_next.archive_operations #384

Merged

mih closed this May 29, 2023

christian-monch mentioned this pull request Jun 1, 2023

Diversify Item type classes #392

Merged

mih deleted the archives-the-3rd branch December 18, 2023 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sketch of a base class for `ArchiveOperations` #372

Sketch of a base class for `ArchiveOperations` #372

mih commented May 24, 2023 •

edited

Loading

codecov bot commented May 24, 2023 •

edited

Loading

mih May 25, 2023

christian-monch May 25, 2023

mih May 25, 2023

christian-monch May 25, 2023

mih May 25, 2023

christian-monch May 25, 2023

mih May 25, 2023

christian-monch May 25, 2023

mih May 25, 2023

christian-monch May 25, 2023

mih May 25, 2023

mih May 25, 2023 •

edited

Loading

christian-monch May 29, 2023

mih May 29, 2023

mih commented May 29, 2023

	def open(self, item: Any) -> IO:
	def open(self, item: PurePosixPath) -> IO:

Sketch of a base class for ArchiveOperations #372

Sketch of a base class for ArchiveOperations #372

Conversation

mih commented May 24, 2023 • edited Loading

codecov bot commented May 24, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mih May 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mih commented May 29, 2023

Sketch of a base class for `ArchiveOperations` #372

Sketch of a base class for `ArchiveOperations` #372

mih commented May 24, 2023 •

edited

Loading

codecov bot commented May 24, 2023 •

edited

Loading

mih May 25, 2023 •

edited

Loading