Repository: cogent3/cogent3
Pull request #1442 (2023-05-17 06:02:51)

Addresses Issue 1328


Patch:
diff --git a/src/cogent3/app/data_store_new.py b/src/cogent3/app/data_store_new.py
index 2cb831fb1..d98a25a4e 100644
--- a/src/cogent3/app/data_store_new.py
+++ b/src/cogent3/app/data_store_new.py
@@ -3,15 +3,18 @@ from __future__ import annotations
 import contextlib
 import inspect
 import json
+import pathlib
 import re
 import reprlib
+import zipfile
 
 from abc import ABC, abstractmethod
 from collections import defaultdict
 from enum import Enum
 from functools import singledispatch
+from os import PathLike
 from pathlib import Path
-from typing import Optional, Union
+from typing import Iterator, Optional, Union
 
 from scitrack import get_text_hexdigest
 
@@ -31,6 +34,9 @@ _NOT_COMPLETED_TABLE = "not_completed"
 _LOG_TABLE = "logs"
 _MD5_TABLE = "md5"
 
+# used for log files, not-completed results
+_special_suffixes = re.compile(r"\.(log|json)$")
+
 StrOrBytes = Union[str, bytes]
 NoneType = type(None)
 
@@ -138,7 +144,9 @@ class DataStoreABC(ABC):
 
     def __contains__(self, identifier):
         """whether relative identifier has been stored"""
-        return any(identifier == m.unique_id for m in self)
+        # following breaks some tests, what is the expected behaviour?
+        # return any(m.unique_id.endswith(identifier) for m in self)
+        return any(m.unique_id == identifier for m in self)
 
     @abstractmethod
     def read(self, unique_id: str) -> StrOrBytes:
@@ -379,7 +387,8 @@ class DataStoreDirectory(DataStoreABC):
         self._limit = limit
 
     def __contains__(self, item: str):
-        item = f"{item}.{self.suffix}" if self.suffix not in item else item
+        if not _special_suffixes.search(item):
+            item = f"{item}.{self.suffix}" if self.suffix not in item else item
         return super().__contains__(item)
 
     def _source_check_create(self, mode: Mode) -> None:
@@ -446,7 +455,8 @@ class DataStoreDirectory(DataStoreABC):
     def completed(self) -> list[DataMember]:
         if not self._completed:
             self._completed = []
-            for i, m in enumerate(self.source.glob(f"*.{self.suffix}")):
+            suffix = f"*.{self.suffix}" if self.suffix else "*"
+            for i, m in enumerate(self.source.glob(suffix)):
                 if self.limit and i == self.limit:
                     break
                 self._completed.append(DataMember(data_store=self, unique_id=m.name))
@@ -543,6 +553,131 @@ class DataStoreDirectory(DataStoreABC):
         return path.read_text() if path.exists() else None
 
 
+class ReadOnlyDataStoreZipped(DataStoreABC):
+    def __init__(
+        self,
+        source: Union[str, Path],
+        mode: Mode = READONLY,
+        suffix: Optional[str] = None,
+        limit: int = None,
+        verbose=False,
+    ):
+        self._mode = Mode(mode)
+        if self._mode is not READONLY:
+            raise ValueError("this is a read only data store")
+
+        suffix = suffix or ""
+        if suffix != "*":  # wild card search for all
+            suffix = re.sub(r"^[\s.*]+", "", suffix)  # tidy the suffix
+        source = Path(source)
+        self._source = source.expanduser()
+        if not self._source.exists():
+            raise IOError(f"{str(self._source)} does not exit")
+        self.suffix = suffix
+        self._verbose = verbose
+        self._limit = limit
+
+    @property
+    def limit(self):
+        return self._limit
+
+    @property
+    def mode(self):
+        return self._mode
+
+    @property
+    def source(self) -> str | Path:
+        return self._source
+
+    def read(self, unique_id: str) -> StrOrBytes:
+        unique_id = str(pathlib.Path(self.source.stem, unique_id)).replace("\\", "/")
+        with zipfile.ZipFile(self.source) as archive:
+            record = archive.open(unique_id)
+            return record.read().decode("utf8")
+
+    def _iter_matches(self, subdir: str, pattern: str) -> Iterator[PathLike]:
+        with zipfile.ZipFile(self._source) as archive:
+            names = archive.namelist()
+            for name in names:
+                name = pathlib.Path(name)
+                if subdir and name.parent.name != subdir:
+                    continue
+                if name.match(pattern):
+                    yield name
+
+    @property
+    def completed(self) -> list[DataMember]:
+        if not self._completed:
+            pattern = f"*.{self.suffix}" if self.suffix else "*"
+            self._completed = []
+            num_matches = 0
+            for name in self._iter_matches("", pattern):
+                num_matches += 1
+                member = DataMember(data_store=self, unique_id=name.name)
+                self._completed.append(member)
+
+                if self.limit and num_matches >= self.limit:
+                    break
+
+        return self._completed
+
+    @property
+    def not_completed(self):
+        if not self._not_completed:
+            self._not_completed = []
+            num_matches = 0
+            nc_dir_path = pathlib.Path(_NOT_COMPLETED_TABLE)
+            for name in self._iter_matches(_NOT_COMPLETED_TABLE, "*.json"):
+                num_matches += 1
+                member = DataMember(
+                    data_store=self, unique_id=str(nc_dir_path / name.name)
+                )
+                self._not_completed.append(member)
+                if self.limit and num_matches >= self.limit:
+                    break
+
+        return self._not_completed
+
+    @property
+    def logs(self) -> list[DataMemberABC]:
+        log_dir = pathlib.Path(_LOG_TABLE)
+        logs = []
+        for name in self._iter_matches(_LOG_TABLE, "*"):
+            m = DataMember(data_store=self, unique_id=str(log_dir / name.name))
+            logs.append(m)
+        return logs
+
+    def md5(self, unique_id: str) -> Union[str, NoneType]:
+        """
+        Parameters
+        ----------
+        unique_id
+            name of data store member
+
+        Returns
+        -------
+        md5 checksum for the member, if available, None otherwise
+        """
+        unique_id = Path(unique_id)
+        unique_id = re.sub(rf"[.]({self.suffix}|json)$", ".txt", unique_id.name)
+        md5_dir = pathlib.Path(_MD5_TABLE)
+        for name in self._iter_matches(_MD5_TABLE, unique_id):
+            m = DataMember(data_store=self, unique_id=str(md5_dir / name.name))
+            return m.read()
+
+    def drop_not_completed(self):
+        raise TypeError("zip data stores are read only")
+
+    def write(self, *, unique_id: str, data: StrOrBytes) -> None:
+        raise TypeError("zip data stores are read only")
+
+    def write_not_completed(self, *, unique_id: str, data: StrOrBytes) -> None:
+        raise TypeError("zip data stores are read only")
+
+    def write_log(self, *, unique_id: str, data: StrOrBytes) -> None:
+        raise TypeError("zip data stores are read only")
+
+
 def get_unique_id(name: str) -> str:
     """strips any format suffixes from name"""
     name = get_data_source(name)
diff --git a/src/cogent3/app/io_new.py b/src/cogent3/app/io_new.py
index 5d341980b..baa58166c 100644
--- a/src/cogent3/app/io_new.py
+++ b/src/cogent3/app/io_new.py
@@ -26,12 +26,12 @@ from cogent3.util.misc import extend_docstring_from
 from cogent3.util.table import Table
 
 from .composable import LOADER, WRITER, NotCompleted, define_app
-from .data_store import ReadOnlyTinyDbDataStore, ReadOnlyZippedDataStore
 from .data_store_new import (
     READONLY,
     DataStoreABC,
     DataStoreDirectory,
     Mode,
+    ReadOnlyDataStoreZipped,
     get_unique_id,
     load_record_from_json,
     make_record_for_json,
@@ -96,8 +96,7 @@ class register_datastore_reader:
 
 
 # register the main readers
-register_datastore_reader("zip")(ReadOnlyZippedDataStore)
-register_datastore_reader("tinydb")(ReadOnlyTinyDbDataStore)
+register_datastore_reader("zip")(ReadOnlyDataStoreZipped)
 register_datastore_reader(None)(DataStoreDirectory)
 register_datastore_reader("sqlitedb")(DataStoreSqlite)
 
@@ -138,7 +137,6 @@ def open_data_store(
         kwargs.pop("suffix")
     elif zipfile.is_zipfile(base_path):
         ds_suffix = ".zip"
-        kwargs.pop("mode")
     elif base_path.suffix:
         ds_suffix = base_path.suffix
     else:


Test patch:
diff --git a/.github/workflows/testing_develop.yml b/.github/workflows/testing_develop.yml
index 607e9326a..b66a087b8 100644
--- a/.github/workflows/testing_develop.yml
+++ b/.github/workflows/testing_develop.yml
@@ -2,9 +2,9 @@ name: CI
 
 on:
   push:
-    branches: [ "develop", "data_store_refactor", "seqview_feature", "lin_rajan_moret_feature" ]
+    branches: [ "develop", "issue_1328" ]
   pull_request:
-    branches: [ "develop", "data_store_refactor", "seqview_feature", "lin_rajan_moret_feature" ]
+    branches: [ "develop", "issue_1328" ]
 
 jobs:
   tests:
@@ -13,7 +13,7 @@ jobs:
 
     strategy:
       matrix:
-        os: [macos-latest, ubuntu-latest, windows-latest]
+        os: [ubuntu-latest, macos-latest, windows-latest]
         python-version: ["3.8", "3.9", "3.10", "3.11"]
 
     steps:
diff --git a/tests/test_app/test_data_store_new.py b/tests/test_app/test_data_store_new.py
index ab02e7a55..05cee6c7c 100644
--- a/tests/test_app/test_data_store_new.py
+++ b/tests/test_app/test_data_store_new.py
@@ -1,4 +1,5 @@
 import json
+import pathlib
 import shutil
 
 from itertools import product
@@ -20,6 +21,7 @@ from cogent3.app.data_store_new import (
     READONLY,
     DataMember,
     DataStoreDirectory,
+    ReadOnlyDataStoreZipped,
     convert_directory_datastore,
     convert_tinydb_to_sqlite,
     get_data_source,
@@ -35,8 +37,8 @@ DATA_DIR = Path(__file__).parent.parent / "data"
 
 
 @pytest.fixture(scope="function")
-def tmp_dir(tmpdir_factory):
-    return Path(tmpdir_factory.mktemp("datastore"))
+def tmp_dir(tmp_path_factory):
+    return Path(tmp_path_factory.mktemp("datastore"))
 
 
 @pytest.fixture(autouse=True)
@@ -269,18 +271,6 @@ def test_contains(ro_dstore):
     assert "brca1" in ro_dstore
 
 
-def test_iter(ro_dstore):
-    """DataStore objects allow iteration over members"""
-    members = list(ro_dstore)
-    assert members == ro_dstore.members
-
-
-def test_members(ro_dstore):
-    for m in ro_dstore:
-        assert isinstance(m, DataMember)
-    assert len(ro_dstore) == len(ro_dstore.completed) + len(ro_dstore.not_completed)
-
-
 def test_len(ro_dstore):
     """DataStore returns correct len"""
     expect = len(list(ro_dstore.source.glob("*.fasta")))
@@ -459,19 +449,6 @@ def test_summary_logs_missing_field(nc_dstore):
     assert isinstance(nc_dstore.summary_logs, Table)
 
 
-def test_summary_logs(full_dstore):
-    # log summary has a row per log file and a column for each property
-    got = full_dstore.summary_logs
-    assert got.shape == (1, 6)
-    assert isinstance(got, Table)
-
-
-def test_summary_not_completed(full_dstore):
-    got = full_dstore.summary_not_completed
-    assert got.shape >= (1, 1)
-    assert isinstance(got, Table)
-
-
 @pytest.mark.parametrize("use_dser", (False, True))
 def test_summary_not_completed_func(nc_objects, use_dser):
     dstore = io_app.open_data_store(":memory:", mode="w")
@@ -488,31 +465,6 @@ def test_summary_not_completed_func(nc_objects, use_dser):
         assert got.shape[0] == 0
 
 
-def test_describe(full_dstore):
-    got = full_dstore.describe
-    assert got.shape >= (3, 2)
-    assert isinstance(got, Table)
-
-
-def test_iter(full_dstore):
-    members = list(full_dstore)
-    assert members == full_dstore.members
-
-
-def test_members(full_dstore):
-    completed = full_dstore.completed
-    not_completed = full_dstore.not_completed
-    assert full_dstore.members == completed + not_completed
-
-
-def test_validate(full_dstore):
-    validation_table = full_dstore.validate()
-    assert isinstance(validation_table, Table)
-    assert validation_table["Num md5sum correct", "Value"] == len(full_dstore)
-    assert validation_table["Num md5sum incorrect", "Value"] == 0
-    assert validation_table["Has log", "Value"] == True
-
-
 def test_write_if_member_exists(full_dstore, write_dir):  # new changes
     """correctly write content"""
     expect = Path(write_dir / "brca1.fasta").read_text()
@@ -604,3 +556,146 @@ def test_write_read_not_completed(nc_dstore):
     assert len(nc_dstore.not_completed) == 1
     got = nc_dstore.not_completed[0].read()
     assert nc.to_json() == got
+
+
+@pytest.fixture
+def zipped_basic(fasta_dir):
+    # converts the fasta_dir into a zipped archive
+
+    path = shutil.make_archive(
+        base_name=fasta_dir.name,
+        format="zip",
+        base_dir=fasta_dir.name,
+        root_dir=fasta_dir.parent,
+    )
+    return pathlib.Path(path)
+
+
+@pytest.fixture
+def zipped_full(full_dstore):
+    # converts the fasta_dir into a zipped archive
+    source = pathlib.Path(full_dstore.source)
+    path = shutil.make_archive(
+        base_name=source.name,
+        format="zip",
+        base_dir=source.name,
+        root_dir=source.parent,
+    )
+    return ReadOnlyDataStoreZipped(pathlib.Path(path), suffix="fasta")
+
+
+def test_zipped_ro_fail(zipped_basic):
+    with pytest.raises(ValueError):
+        ReadOnlyDataStoreZipped(zipped_basic, suffix="fasta", mode="w")
+
+
+def test_zipped_ro_ioerror():
+    with pytest.raises(IOError):
+        ReadOnlyDataStoreZipped("blah-1234.zip", suffix="fasta")
+
+
+def _get_member_data(members) -> dict:
+    return {m.unique_id: m.read() for m in members}
+
+
+def test_zipped_ro_basic(zipped_basic, ro_dstore):
+    dstore = ReadOnlyDataStoreZipped(zipped_basic, suffix="fasta")
+    assert len(dstore.completed) == len(ro_dstore.completed)
+    assert len(dstore) == len(ro_dstore)
+    expect = _get_member_data(ro_dstore.completed)
+    got = _get_member_data(dstore.completed)
+    assert expect == got
+    # there are no not-completeds in a basic dstore
+    expect = _get_member_data(ro_dstore.not_completed)
+    got = _get_member_data(dstore.not_completed)
+    assert expect == got
+
+
+def test_zipped_ro_full(zipped_full, full_dstore):
+    got_ids = {m.unique_id for m in zipped_full.completed}
+    expect_ids = {m.unique_id for m in full_dstore.completed}
+    assert got_ids == expect_ids
+
+    got_ids = {m.unique_id for m in zipped_full.not_completed}
+    expect_ids = {m.unique_id for m in full_dstore.not_completed}
+    assert got_ids == expect_ids
+    assert len(zipped_full) == len(full_dstore)
+
+    expect = _get_member_data(full_dstore.completed)
+    got = _get_member_data(zipped_full.completed)
+    assert expect == got
+    # there are not-completeds in the full dstore
+    expect = _get_member_data(full_dstore.not_completed)
+    got = _get_member_data(zipped_full.not_completed)
+    assert expect == got
+
+
+def test_zipped_logs(zipped_full, full_dstore):
+    assert len(zipped_full.logs) == len(full_dstore.logs)
+    expect = _get_member_data(full_dstore.logs)
+    got = _get_member_data(zipped_full.logs)
+    assert expect == got
+
+
+def test_zipped_md5(zipped_full, full_dstore):
+    expect = {m.unique_id: full_dstore.md5(m.unique_id) for m in full_dstore.completed}
+    got = {m.unique_id: zipped_full.md5(m.unique_id) for m in zipped_full.completed}
+    assert got == expect
+
+
+@pytest.mark.parametrize("dstore", ("full_dstore", "zipped_full"))
+def test_zipped_contains(dstore, request):
+    full_dstore = request.getfixturevalue(dstore)
+    assert "formattest.fasta" in full_dstore
+    # following line does not work, should it?
+    # assert "id_2.json" in full_dstore
+
+
+@pytest.mark.parametrize("dstore", ("full_dstore", "zipped_full"))
+def test_members(dstore, request):
+    full_dstore = request.getfixturevalue(dstore)
+    completed = full_dstore.completed
+    not_completed = full_dstore.not_completed
+    assert full_dstore.members == completed + not_completed
+
+
+@pytest.mark.parametrize("dstore", ("full_dstore", "zipped_full"))
+def test_describe(dstore, request):
+    full_dstore = request.getfixturevalue(dstore)
+    got = full_dstore.describe
+    assert got.shape >= (3, 2)
+    assert isinstance(got, Table)
+
+
+@pytest.mark.parametrize("dstore", ("full_dstore", "zipped_full"))
+def test_iter(dstore, request):
+    full_dstore = request.getfixturevalue(dstore)
+    members = list(full_dstore)
+    assert members == full_dstore.members
+
+
+@pytest.mark.parametrize("dstore", ("full_dstore", "zipped_full"))
+def test_validate(dstore, request):
+    full_dstore = request.getfixturevalue(dstore)
+    validation_table = full_dstore.validate()
+    assert isinstance(validation_table, Table)
+    assert validation_table["Num md5sum correct", "Value"] == len(full_dstore)
+    assert validation_table["Num md5sum incorrect", "Value"] == 0
+    assert validation_table["Has log", "Value"] == True
+
+
+@pytest.mark.parametrize("dstore", ("full_dstore", "zipped_full"))
+def test_summary_logs(dstore, request):
+    full_dstore = request.getfixturevalue(dstore)
+    # log summary has a row per log file and a column for each property
+    got = full_dstore.summary_logs
+    assert got.shape == (1, 6)
+    assert isinstance(got, Table)
+
+
+@pytest.mark.parametrize("dstore", ("full_dstore", "zipped_full"))
+def test_summary_not_completed(dstore, request):
+    full_dstore = request.getfixturevalue(dstore)
+    got = full_dstore.summary_not_completed
+    assert got.shape >= (1, 1)
+    assert isinstance(got, Table)
diff --git a/tests/test_app/test_io_new.py b/tests/test_app/test_io_new.py
index 82e5c2d3d..a7c07591b 100644
--- a/tests/test_app/test_io_new.py
+++ b/tests/test_app/test_io_new.py
@@ -4,6 +4,7 @@ import json
 import os
 import pathlib
 import pickle
+import shutil
 
 from pathlib import Path
 
@@ -15,7 +16,12 @@ from numpy.testing import assert_allclose
 from cogent3 import DNA, get_app, open_data_store
 from cogent3.app import io_new as io_app
 from cogent3.app.composable import NotCompleted, source_proxy
-from cogent3.app.data_store_new import DataMember, DataStoreDirectory, Mode
+from cogent3.app.data_store_new import (
+    DataMember,
+    DataStoreDirectory,
+    Mode,
+    ReadOnlyDataStoreZipped,
+)
 from cogent3.app.io_new import DEFAULT_DESERIALISER, DEFAULT_SERIALISER
 from cogent3.core.alignment import ArrayAlignment, SequenceCollection
 from cogent3.core.profile import PSSM, MotifCountsArray, MotifFreqsArray
@@ -30,8 +36,8 @@ DATA_DIR = Path(__file__).parent.parent / "data"
 
 
 @pytest.fixture(scope="function")
-def tmp_dir(tmpdir_factory):
-    return Path(tmpdir_factory.mktemp("io"))
+def tmp_dir(tmp_path_factory):
+    return tmp_path_factory.mktemp("io")
 
 
 @pytest.fixture(scope="function")
@@ -58,6 +64,16 @@ def fasta_dir(tmp_dir):
     return fasta_dir
 
 
+@pytest.fixture
+def zipped_full(fasta_dir):
+    # converts the fasta_dir into a zipped archive
+    source = fasta_dir
+    path = shutil.make_archive(
+        base_name=source, format="zip", base_dir=source, root_dir=source.parent
+    )
+    return ReadOnlyDataStoreZipped(pathlib.Path(path), suffix="fasta")
+
+
 def _get_generic_result(source):
     """creates a generic result with a DNA moltype as the single value"""
     from cogent3.app.result import generic_result
@@ -618,3 +634,9 @@ def test_to_from_json():
 def test_to_json_combines():
     app = get_app("to_primitive") + get_app("to_json")
     assert app(DNA) == DNA.to_json()
+
+
+def test_open_zipped(zipped_full):
+    got = open_data_store(zipped_full.source, mode="r", suffix="fasta")
+    assert len(got) == len(zipped_full)
+    assert isinstance(got, type(zipped_full))