Repository: cogent3/cogent3
Pull request #1540 (2023-07-14 06:10:04)

BUG: Features now work with GenbankAnnotationDb


Patch:
diff --git a/src/cogent3/app/data_store.py b/src/cogent3/app/data_store.py
index 87f9552ac..58ebf458f 100644
--- a/src/cogent3/app/data_store.py
+++ b/src/cogent3/app/data_store.py
@@ -398,10 +398,11 @@ class DataStoreDirectory(DataStoreABC):
 
         sub_dirs = [_NOT_COMPLETED_TABLE, _LOG_TABLE, _MD5_TABLE]
         source = self.source
-        if mode is READONLY and not source.exists():
-            raise IOError(f"'{source}' does not exist")
-        elif mode is READONLY:
-            return
+        if mode is READONLY:
+            if not source.exists():
+                raise IOError(f"'{source}' does not exist")
+            else:
+                return
 
         if not source.exists():
             source.mkdir(parents=True, exist_ok=True)
@@ -411,12 +412,12 @@ class DataStoreDirectory(DataStoreABC):
 
     @property
     def source(self) -> str | Path:
-        """string that references connecting to data store, override in subclass constructor"""
+        """string that references the data store, override in subclass constructor"""
         return self._source
 
     @property
     def mode(self) -> Mode:
-        """string that references datastore mode, override in override in subclass constructor"""
+        """string that references datastore mode, override in subclass constructor"""
         return self._mode
 
     @property
diff --git a/src/cogent3/core/annotation.py b/src/cogent3/core/annotation.py
index d3c203652..8ab071d74 100644
--- a/src/cogent3/core/annotation.py
+++ b/src/cogent3/core/annotation.py
@@ -10,35 +10,6 @@ from cogent3.util.misc import get_object_provenance
 from .location import Map
 
 
-class _Serialisable:
-    def to_rich_dict(self):
-        """returns {'name': name, 'seq': sequence, 'moltype': moltype.label}"""
-        data = copy.deepcopy(self._serialisable)
-        # the first constructor argument will be the instance recreating
-        # so we pop out the two possible keys
-        data.pop("parent", None)
-        data.pop("seq", None)
-        if "original" in data:
-            data.pop("original")
-        # convert the map to coordinates
-        data["map"] = data.pop("map").to_rich_dict()
-        data = dict(annotation_construction=data)
-        data["type"] = get_object_provenance(self)
-        data["version"] = __version__
-
-        try:
-            annotations = [a.to_rich_dict() for a in self.annotations]
-            if annotations:
-                data["annotations"] = annotations
-        except AttributeError:
-            pass
-
-        return data
-
-    def to_json(self):
-        return json.dumps(self.to_rich_dict())
-
-
 # todo gah implement __repr__ and __str__ methods
 # todo gah write docstrings!
 class Feature:
@@ -153,18 +124,26 @@ class Feature:
         [(start1, end1), ...]"""
         return self.map.get_coordinates()
 
-    def get_children(self, biotype: Optional[str] = None):
+    def get_children(self, biotype: Optional[str] = None, **kwargs):
         """generator returns sub-features of self optionally matching biotype"""
         make_feature = self.parent.make_feature
         db = self.parent.annotation_db
-        for child in db.get_feature_children(biotype=biotype, name=self.name):
+        for child in db.get_feature_children(
+            biotype=biotype,
+            name=self.name,
+            start=self.map.start,
+            end=self.map.end,
+            **kwargs,
+        ):
             yield make_feature(feature=child)
 
-    def get_parent(self, biotype: Optional[str] = None):
+    def get_parent(self, **kwargs):
         """generator returns parent features of self optionally matching biotype"""
         make_feature = self.parent.make_feature
         db = self.parent.annotation_db
-        for child in db.get_feature_parent(biotype=biotype, name=self.name):
+        for child in db.get_feature_parent(
+            name=self.name, start=self.map.start, end=self.map.end, **kwargs
+        ):
             yield make_feature(feature=child)
 
     def union(self, features: Iterable):
diff --git a/src/cogent3/core/annotation_db.py b/src/cogent3/core/annotation_db.py
index adecfeb1b..acfa242c9 100644
--- a/src/cogent3/core/annotation_db.py
+++ b/src/cogent3/core/annotation_db.py
@@ -70,7 +70,7 @@ class FeatureDataType(typing.TypedDict):
 
 
 @typing.runtime_checkable
-class SerialisableType(typing.Protocol):
+class SerialisableType(typing.Protocol):  # pragma: no cover
     def to_rich_dict(self) -> dict:
         ...
 
@@ -82,7 +82,8 @@ class SerialisableType(typing.Protocol):
 
 
 @typing.runtime_checkable
-class SupportsQueryFeatures(typing.Protocol):  # should be defined centrally
+class SupportsQueryFeatures(typing.Protocol):  # pragma: no cover
+    # should be defined centrally
     def get_features_matching(
         self,
         *,
@@ -98,12 +99,12 @@ class SupportsQueryFeatures(typing.Protocol):  # should be defined centrally
         ...
 
     def get_feature_children(
-        self, *, name: str, start: OptionalInt = None, end: OptionalInt = None
+        self, *, name: str, start: OptionalInt = None, end: OptionalInt = None, **kwargs
     ) -> typing.List[FeatureDataType]:
         ...
 
     def get_feature_parent(
-        self, *, name: str, start: OptionalInt = None, end: OptionalInt = None
+        self, *, name: str, start: OptionalInt = None, end: OptionalInt = None, **kwargs
     ) -> typing.List[FeatureDataType]:
         ...
 
@@ -121,7 +122,8 @@ class SupportsQueryFeatures(typing.Protocol):  # should be defined centrally
 
 
 @typing.runtime_checkable
-class SupportsWriteFeatures(typing.Protocol):  # should be defined centrally
+class SupportsWriteFeatures(typing.Protocol):  # pragma: no cover
+    # should be defined centrally
     def add_feature(
         self,
         *,
@@ -161,7 +163,7 @@ class SupportsFeatures(
     SerialisableType,
     typing.Sized,
     typing.Protocol,
-):
+):  # should be defined centrally
     def __len__(self):
         # the number of records
         ...
@@ -673,6 +675,7 @@ class SqliteAnnotationDbMixin:
                 column="parent_id",
                 name=f"%{name}%",
                 biotype=biotype,
+                **kwargs,
             ):
                 result.pop("parent_id")  # remove invalid field for the FeatureDataType
                 yield result
@@ -686,7 +689,10 @@ class SqliteAnnotationDbMixin:
             if table_name == "user":
                 columns += ("on_alignment",)
             for result in self._get_feature_by_id(
-                table_name=table_name, columns=columns, column="name", name=f"%{name}%"
+                table_name=table_name,
+                columns=columns,
+                column="name",
+                name=f"%{name}%",
             ):
                 # multiple values for parent means this is better expressed
                 # as an OR clause
@@ -1187,7 +1193,7 @@ class GffAnnotationDb(SqliteAnnotationDbMixin):
 # to establish the Parent of a feature is by knowing what other
 # features have the same ID and then which of those are candidates to contain
 # a feature based on the hierarchy of relationships.
-# In other words, we assume that children have the same ID as their parent BUT but
+# In other words, we assume that children have the same ID as their parent BUT
 # their span lies within the parents. Conversely, for a parent query, the parent
 # has the same ID as their child but their span contains the childs.
 
@@ -1317,6 +1323,11 @@ class GenbankAnnotationDb(SqliteAnnotationDbMixin):
     ) -> typing.Iterator[FeatureDataType]:
         """yields children of name"""
         # children must lie within the parent coordinates
+        if start is None or end is None:
+            name = self.__class__.__name__
+            msg = f"coordinates required to query children for {name!r}"
+            raise ValueError(msg)
+
         for table_name in self.table_names:
             columns = (
                 "seqid",
@@ -1346,6 +1357,8 @@ class GenbankAnnotationDb(SqliteAnnotationDbMixin):
                 cstart, cend = feat.pop("start"), feat.pop("end")
                 if not (start <= cstart < end and start < cend <= end):
                     continue
+
+                feat.pop("parent_id")  # remove invalid field for the FeatureDataType
                 yield feat
 
     def get_feature_parent(
@@ -1356,6 +1369,11 @@ class GenbankAnnotationDb(SqliteAnnotationDbMixin):
         end: OptionalInt = None,
     ) -> typing.Iterator[FeatureDataType]:
         """yields parents of name"""
+        if start is None or end is None:
+            name = self.__class__.__name__
+            msg = f"coordinates required to query parent for {name!r}"
+            raise ValueError(msg)
+
         # parent must match or lie outside the coordinates
         for table_name in self.table_names:
             columns = (
@@ -1386,6 +1404,8 @@ class GenbankAnnotationDb(SqliteAnnotationDbMixin):
                 cstart, cend = feat.pop("start"), feat.pop("end")
                 if cstart > start or end > cend:
                     continue
+
+                feat.pop("parent_id")  # remove invalid field for the FeatureDataType
                 yield feat
 
 
diff --git a/src/cogent3/core/sequence.py b/src/cogent3/core/sequence.py
index 8bb1ffaca..c0efa5fe6 100644
--- a/src/cogent3/core/sequence.py
+++ b/src/cogent3/core/sequence.py
@@ -741,9 +741,9 @@ class SequenceI(object):
         table.append("</table>")
         class_name = self.__class__.__name__
         if limit and limit < len(self):
-            summary = f"{class_name}, length={len(self)} (truncated to {limit if limit else len(self)})"
+            summary = f"{class_name}, length={len(self):,} (truncated to {limit if limit else len(self)})"
         else:
-            summary = f"{class_name}, length={len(self)}"
+            summary = f"{class_name}, length={len(self):,}"
 
         text = [
             "<style>",
@@ -1324,7 +1324,7 @@ class Sequence(SequenceI):
         myclass = f"{self.__class__.__name__}"
         myclass = myclass.split(".")[-1]
         if len(self) > 10:
-            seq = f"{str(self)[:7]}... {len(self)}"
+            seq = f"{str(self)[:7]}... {len(self):,}"
         else:
             seq = str(self)
         return f"{myclass}({seq})"


Test patch:
diff --git a/tests/test_core/test_annotation.py b/tests/test_core/test_annotation.py
index 5210327c3..5c8bff2af 100644
--- a/tests/test_core/test_annotation.py
+++ b/tests/test_core/test_annotation.py
@@ -2,7 +2,7 @@ import unittest
 
 import pytest
 
-from cogent3 import DNA, make_aligned_seqs, make_unaligned_seqs
+from cogent3 import DNA, load_seq, make_aligned_seqs, make_unaligned_seqs
 from cogent3.core.location import Map, Span
 
 
@@ -290,3 +290,12 @@ def test_seq_slice_seqfeat_invalid(ann_aln):
     seq2 = ann_aln.get_seq("FAKE02")
     with pytest.raises(ValueError):
         _ = seq2[list(seq1.get_features(biotype="CDS"))[0]]
+
+
+def test_gbdb_get_children_get_parent(DATA_DIR):
+    seq = load_seq(DATA_DIR / "annotated_seq.gb")
+    (orig,) = list(seq.get_features(biotype="gene", name="CNA00110"))
+    (child,) = list(orig.get_children("CDS"))
+    parent, *_ = list(child.get_parent(exclude_biotype=child.biotype))
+    assert parent.name == orig.name
+    assert parent.biotype != child.biotype
diff --git a/tests/test_core/test_annotation_db.py b/tests/test_core/test_annotation_db.py
index 3009ad522..8755151df 100644
--- a/tests/test_core/test_annotation_db.py
+++ b/tests/test_core/test_annotation_db.py
@@ -935,3 +935,13 @@ def test_load_anns_with_write(DATA_DIR, tmp_dir):
     got_data = got.to_rich_dict()
     expect_data = expect.to_rich_dict()
     assert got_data["tables"] == expect_data["tables"]
+
+
+def test_gbdb_get_children_fails_no_coords(gb_db):
+    with pytest.raises(ValueError):
+        _ = list(gb_db.get_feature_children(name="CNA00110"))
+
+
+def test_gbdb_get_parent_fails_no_coords(gb_db):
+    with pytest.raises(ValueError):
+        _ = list(gb_db.get_feature_parent(name="CNA00110"))