Repository: cogent3/cogent3
Pull request #1389 (2023-04-25 11:18:44)

Annotation refactor


Patch:
diff --git a/src/cogent3/core/alignment.py b/src/cogent3/core/alignment.py
index 55f36965a..5ec1d6e27 100644
--- a/src/cogent3/core/alignment.py
+++ b/src/cogent3/core/alignment.py
@@ -65,7 +65,6 @@ from cogent3.core.genetic_code import get_code
 from cogent3.core.info import Info as InfoClass
 from cogent3.core.profile import PSSM, MotifCountsArray
 from cogent3.core.sequence import ArraySequence, Sequence, frac_same
-
 # which is a circular import otherwise.
 from cogent3.format.alignment import save_to_filename
 from cogent3.format.fasta import alignment_to_fasta
@@ -2300,19 +2299,24 @@ class Aligned:
     def with_termini_unknown(self):
         return self.__class__(self.map.with_termini_unknown(), self.data)
 
-    def copy_annotations(self, other):
+    @c3warn.deprecated_callable(
+        "2023.7",
+        reason="handled by <collection>.copy_annotations()",
+        is_discontinued=True,
+    )
+    def copy_annotations(self, other):  # pragma: no cover
         self.data.copy_annotations(other)
 
     @c3warn.deprecated_callable(
         "2023.10", "handled by <collection>.annotate_from_gff()", is_discontinued=True
     )
-    def annotate_from_gff(self, f):
+    def annotate_from_gff(self, f):  # pragma: no cover
         self.data.annotate_from_gff(f)
 
     @c3warn.deprecated_callable(
         "2023.10", "handled by <collection>.add_feature()", is_discontinued=True
     )
-    def add_feature(self, *args, **kwargs):
+    def add_feature(self, *args, **kwargs):  # pragma: no cover
         self.data.add_feature(*args, **kwargs)
 
     def __str__(self):
@@ -2404,17 +2408,17 @@ class Aligned:
     @c3warn.deprecated_callable(
         "2023.10", "handled by <collection>.get_features()", is_discontinued=True
     )
-    def get_annotations_matching(self, alignment, **kwargs):
-        for feature in self.data.annotation_db.get_features_matching(**kwargs):
+    def get_annotations_matching(self, alignment, **kwargs):  # pragma: no cover
+        for feature in self.data.annotation_db.get_features(**kwargs):
             yield self.make_feature(feature, alignment)
 
     @c3warn.deprecated_callable(
         "2023.10", "handled by <collection>.get_features()", is_discontinued=True
     )
-    def get_features_matching(self, alignment, annotation_type="*", **kwargs):
-        for annot in self.data.get_features_matching(
-            feature_type=annotation_type, **kwargs
-        ):
+    def get_features_matching(
+        self, alignment, annotation_type="*", **kwargs
+    ):  # pragma: no cover
+        for annot in self.data.get_features(biotype=annotation_type, **kwargs):
             yield annot.remapped_to(alignment, self.map.inverse())
 
     def get_drawables(self):
@@ -4700,13 +4704,25 @@ class Alignment(_Annotatable, AlignmentI, SequenceCollection):
         aln_annots = self.get_features_matching(*args)
         return [self.project_annotation(seq_name, a) for a in aln_annots]
 
-    def get_annotations_from_seq(self, seq_name, annotation_type="*", **kwargs):
+    @c3warn.deprecated_callable(
+        "2023.7",
+        reason="use <collection>.get_features(seqid=<seq_name>)",
+        is_discontinued=True,
+    )
+    def get_annotations_from_seq(
+        self, seq_name, annotation_type="*", **kwargs
+    ):  # pragma: no cover
         aligned = self.named_seqs[seq_name]
         return aligned.get_features_matching(
             self, annotation_type=annotation_type, **kwargs
         )
 
-    def get_annotations_from_any_seq(self, annotation_type="*", **kwargs):
+    @c3warn.deprecated_callable(
+        "2023.7", reason="use <collection>.get_features()", is_discontinued=True
+    )
+    def get_annotations_from_any_seq(
+        self, annotation_type="*", **kwargs
+    ):  # pragma: no cover
         result = []
         for seq_name in self.names:
             a = self.get_annotations_from_seq(
@@ -4715,7 +4731,12 @@ class Alignment(_Annotatable, AlignmentI, SequenceCollection):
             result.extend(a)
         return result
 
-    def get_by_seq_annotation(self, seq_name, *args):
+    @c3warn.deprecated_callable(
+        "2023.7",
+        reason="handled by <collection>.get_features(seqid=<seqid>)",
+        is_discontinued=True,
+    )
+    def get_by_seq_annotation(self, seq_name, *args):  # pragma: no cover
         result = []
         for feature in self.get_annotations_from_seq(seq_name, *args):
             segment = self[feature.map.start : feature.map.end]
diff --git a/src/cogent3/core/annotation_db.py b/src/cogent3/core/annotation_db.py
index ed8277c91..fbcc54cbd 100644
--- a/src/cogent3/core/annotation_db.py
+++ b/src/cogent3/core/annotation_db.py
@@ -196,7 +196,7 @@ def _add_record_sql(
 
 def _matching_conditions(
     conditions: dict,
-    partial: bool = True,
+    allow_partial: bool = True,
 ):
     """creates WHERE clause
 
@@ -204,7 +204,7 @@ def _matching_conditions(
     ----------
     conditions : dict
         column name and values to be matched
-    partial : bool, optional
+    allow_partial : bool, optional
         if False, only records within start, end are included. If True,
         all records that overlap the segment defined by start, end are included.
 
@@ -233,30 +233,26 @@ def _matching_conditions(
         vals = tuple(vals)
 
     if isinstance(start, int) and isinstance(end, int):
-        # only matches within bounds
-        if partial:
+        if allow_partial:
+            # allow matches that overlap the segment
             cond = [
+                f"(start >= {start} AND end <= {end})",  # lies within the segment
                 f"(start <= {start} AND end > {start})",  # straddles beginning of segment
                 f"(start < {end} AND end >= {end})",  # straddles end of segment
                 f"(start <= {start} AND end >= {end})",  # includes segment
             ]
             cond = " OR ".join(cond)
         else:
+            # only matches within bounds
             cond = f"start >= {start} AND end <= {end}"
         sql.append(f"({cond})")
     elif isinstance(start, int):
-        if partial:
-            cond = " OR ".join(
-                [f"start >= {start}", f"(start <= {start} AND end >= {start})"]
-            )
-        else:
-            cond = f"start >= {start}"
+        # if query has no end, then any feature containing start
+        cond = f"(start <= {start} AND {start} < end)"
         sql.append(f"({cond})")
     elif isinstance(end, int):
-        if partial:
-            cond = " OR ".join([f"end <= {end}", f"(start <= {end} AND end >= {end})"])
-        else:
-            cond = f"end <= {end}"
+        # if query has no start, then any feature containing end
+        cond = f"(start <= {end} AND {end} < end)"
         sql.append(f"({cond})")
 
     sql = f"{' AND '.join(sql)}"
@@ -268,7 +264,7 @@ def _del_records_sql(
     conditions: dict,
     start: OptionalInt = None,
     end: OptionalInt = None,
-    partial=True,
+    allow_partial=True,
 ) -> ReturnType:
     """creates the SQL and values for identifying records to be deleted
 
@@ -280,8 +276,8 @@ def _del_records_sql(
         column name and values to be matched
     start, end : OptionalInt
         select records whose (start, end) values lie between start and end,
-        or overlap them if (partial is True)
-    partial : bool, optional
+        or overlap them if (allow_partial is True)
+    allow_partial : bool, optional
         if False, only records within start, end are included. If True,
         all records that overlap the segment defined by start, end are included.
 
@@ -291,7 +287,7 @@ def _del_records_sql(
         the SQL statement and the tuple of values
     """
     where, vals = _matching_conditions(
-        conditions=conditions, start=start, end=end, partial=partial
+        conditions=conditions, start=start, end=end, allow_partial=allow_partial
     )
     sql = f"DELETE FROM {table_name}"
     if not where:
@@ -307,7 +303,7 @@ def _select_records_sql(
     columns: typing.Optional[typing.List[str]] = None,
     start: OptionalInt = None,
     end: OptionalInt = None,
-    partial=True,
+    allow_partial=True,
 ) -> ReturnType:
     """create SQL select statement and values
 
@@ -321,8 +317,8 @@ def _select_records_sql(
         the WHERE conditions
     start, end : OptionalInt
         select records whose (start, end) values lie between start and end,
-        or overlap them if (partial is True)
-    partial : bool, optional
+        or overlap them if (allow_partial is True)
+    allow_partial : bool, optional
         if False, only records within start, end are included. If True,
         all records that overlap the segment defined by start, end are included.
 
@@ -332,7 +328,9 @@ def _select_records_sql(
         the SQL statement and the tuple of values
     """
 
-    where, vals = _matching_conditions(conditions=conditions, partial=partial)
+    where, vals = _matching_conditions(
+        conditions=conditions, allow_partial=allow_partial
+    )
     columns = f"{', '.join(columns)}" if columns else "*"
     sql = f"SELECT {columns} FROM {table_name}"
     if not where:
@@ -348,7 +346,7 @@ def _count_records_sql(
     columns: typing.Optional[typing.List[str]] = None,
     start: OptionalInt = None,
     end: OptionalInt = None,
-    partial=True,
+    allow_partial=True,
 ) -> ReturnType:
     """create SQL count statement and values
 
@@ -362,8 +360,8 @@ def _count_records_sql(
         the WHERE conditions
     start, end : OptionalInt
         select records whose (start, end) values lie between start and end,
-        or overlap them if (partial is True)
-    partial : bool, optional
+        or overlap them if (allow_partial is True)
+    allow_partial : bool, optional
         if False, only records within start, end are included. If True,
         all records that overlap the segment defined by start, end are included.
 
@@ -373,7 +371,9 @@ def _count_records_sql(
         the SQL statement and the tuple of values
     """
 
-    where, vals = _matching_conditions(conditions=conditions, partial=partial)
+    where, vals = _matching_conditions(
+        conditions=conditions, allow_partial=allow_partial
+    )
     sql = f"SELECT COUNT(*) FROM {table_name}"
     if not where:
         return sql, None
@@ -470,8 +470,12 @@ class SqliteAnnotationDbMixin:
     ) -> typing.Iterator[FeatureDataType]:
         """return all fields"""
         columns = kwargs.pop("columns", None)
+        allow_partial = kwargs.pop("allow_partial", False)
         sql, vals = _select_records_sql(
-            table_name=table_name, conditions=kwargs, columns=columns
+            table_name=table_name,
+            conditions=kwargs,
+            columns=columns,
+            allow_partial=allow_partial,
         )
         yield from self._execute_sql(sql, values=vals)
 
@@ -484,6 +488,7 @@ class SqliteAnnotationDbMixin:
         end: int = None,
         strand: bool = None,
         on_alignment: bool = None,
+        allow_partial: bool = False,
     ) -> typing.Iterator[dict]:
         """return all fields for matching records"""
         # a record is Everything, a Feature is a subset
@@ -503,6 +508,7 @@ class SqliteAnnotationDbMixin:
         end: int = None,
         strand: bool = None,
         on_alignment: bool = None,
+        allow_partial: bool = False,
     ) -> typing.Iterator[FeatureDataType]:
         # returns essential values to create a Feature
         # we define query as all defined variables from local name space,
@@ -855,7 +861,7 @@ class GenbankAnnotationDb(SqliteAnnotationDbMixin):
         start: int,
         end: int,
         biotype: OptionalStr = None,
-        partial: bool = False,
+        allow_partial: bool = False,
     ) -> typing.List[FeatureDataType]:
         # we return the parent_id because `get_feature_parent()` requires it
         sql, vals = _select_records_sql(
@@ -864,7 +870,7 @@ class GenbankAnnotationDb(SqliteAnnotationDbMixin):
             start=start,
             end=end,
             columns=["biotype", "start", "end", "spans", "strand", "name", "parent_id"],
-            partial=partial,
+            allow_partial=allow_partial,
         )
         for result in self._execute_sql(sql, values=vals):
             yield {
@@ -893,7 +899,7 @@ class GenbankAnnotationDb(SqliteAnnotationDbMixin):
                 biotype=biotype,
                 start=int(start),
                 end=int(end),
-                partial=False,
+                allow_partial=False,
             ):
                 if feat["biotype"] == exclude_biotype:
                     continue
@@ -917,7 +923,7 @@ class GenbankAnnotationDb(SqliteAnnotationDbMixin):
                 name=name,
                 start=int(start),
                 end=int(end),
-                partial=False,
+                allow_partial=False,
             ):
                 # add support for != operation to SQL where clause generation
                 if feat["biotype"] == exclude_biotype:
diff --git a/src/cogent3/core/sequence.py b/src/cogent3/core/sequence.py
index f3cba0efa..0dfcd8028 100644
--- a/src/cogent3/core/sequence.py
+++ b/src/cogent3/core/sequence.py
@@ -57,7 +57,11 @@ from cogent3.util.misc import (
     get_setting_from_environ,
 )
 from cogent3.util.transform import for_seq, per_shortest
-from cogent3.util.warning import deprecated, deprecated_args
+from cogent3.util.warning import (
+    deprecated,
+    deprecated_args,
+    deprecated_callable,
+)
 
 
 __author__ = "Rob Knight, Gavin Huttley, and Peter Maxwell"
@@ -877,23 +881,28 @@ class Sequence(_Annotatable, SequenceI):
             raise TypeError
         self._annotation_db = value
 
-    # todo deprecate feature_type for biotype
-    def get_features_matching(
+    @deprecated_args(
+        "2023.7",
+        reason="consistent nomenclature",
+        old_new=[("feature_type", "biotype")],
+    )
+    def get_features(
         self,
-        feature_type: Optional[str] = None,
+        biotype: Optional[str] = None,
         name: Optional[str] = None,
         start: Optional[int] = None,
         stop: Optional[int] = None,
+        allow_partial: bool = False,
     ):
         """yields Annotation instances
 
         Parameters
         ----------
-        feature_type
+        biotype
             biotype of the feature
         name
             name of the feature
-        start, end
+        start, stop
             start, end positions to search between, relative to offset
             of this sequence. If not provided, entire span of sequence is used.
 
@@ -907,20 +916,33 @@ class Sequence(_Annotatable, SequenceI):
         if self._annotation_db is None:
             return None
 
-        query_start = self._seq.absolute_index(start) if start is not None else None
-        query_end = self._seq.absolute_index(stop) if stop is not None else None
-
-        seq_rced = self._seq.reversed
-
+        offset = self.annotation_offset or 0
+        query_start = self._seq.absolute_index(offset + (start or 0))
+        query_end = self._seq.absolute_index(offset + (stop or len(self)))
+        # todo gah check logic of handling of negative indices
+        query_start, query_end = (
+            (query_start, query_end)
+            if query_start < query_end
+            else (query_end, query_start)
+        )
+        query_start = max(query_start, 0)
         for feature in self.annotation_db.get_features_matching(
             seqid=self.name,
             name=name,
-            biotype=feature_type,
+            biotype=biotype,
             start=query_start,
-            end=query_end,
+            end=query_end,  # todo gah end should be stop
+            allow_partial=allow_partial,
         ):
             yield self.make_feature(feature)
 
+    @deprecated_callable(
+        "2023.7", reason="simpler name", new="<instance>.get_features()"
+    )
+    def get_features_matching(self, **kwargs):
+        """use .get_features()"""
+        return self.get_features(**kwargs)
+
     def make_feature(self, feature: FeatureDataType, *args) -> Annotation:
         """
         return an Annotation instance from feature data
@@ -1150,9 +1172,9 @@ class Sequence(_Annotatable, SequenceI):
         return f"{myclass}({seq})"
 
     def __getitem__(self, index):
-        # todo gah check this omission
-        # if hasattr(index, "get_slice"):
-        #     return index.get_slice()
+        if hasattr(index, "map"):
+            index = index.map
+
         # todo: kath preserve the offset?
 
         if isinstance(index, Map):
@@ -1604,7 +1626,7 @@ class SeqView:
         self.seq = seq
         self.start = start
         self.stop = stop
-        self.offset = None
+        self.offset = None  # todo gah this should default to 0
         self.step = step
 
     @property
@@ -1682,7 +1704,7 @@ class SeqView:
             raise IndexError(val)
 
         if self.step > 0:
-            if val > 0 and val >= len(self):
+            if val > 0 and val > len(self):
                 raise IndexError(val)
             elif val < 0 and abs(val) > len(self):
                 raise IndexError(val)
@@ -1695,7 +1717,7 @@ class SeqView:
             return val, val + 1, 1
 
         elif self.step < 0:
-            if val > 0 and val >= len(self):
+            if val > 0 and val > len(self):
                 raise IndexError(val)
             elif val < 0 and abs(val) > len(self):
                 raise IndexError(val)


Test patch:
diff --git a/tests/test_core/test_alignment.py b/tests/test_core/test_alignment.py
index 3de02916a..832be2c65 100644
--- a/tests/test_core/test_alignment.py
+++ b/tests/test_core/test_alignment.py
@@ -3253,10 +3253,10 @@ def test_annotate_from_gff3(cls):
     seq_coll.annotate_from_gff(gff3_path)
     member_seq = seq_coll.get_seq(seq.name)
 
-    matches = list(member_seq.get_features_matching())
+    matches = list(member_seq.get_features())
     # 11 features
     assert len(matches) == 11
-    matches = list(member_seq.get_features_matching(feature_type="gene"))
+    matches = list(member_seq.get_features(biotype="gene"))
     assert len(matches) == 1
     matches = list(matches[0].get_children(biotype="mRNA"))
     assert len(matches) == 1
diff --git a/tests/test_core/test_annotation_db.py b/tests/test_core/test_annotation_db.py
index 32d23455c..b10010752 100644
--- a/tests/test_core/test_annotation_db.py
+++ b/tests/test_core/test_annotation_db.py
@@ -332,27 +332,29 @@ def test_annotate_from_gff(seq):
 def test_get_features_matching_start_stop(seq):
     # todo: need to implement LostSpans()
     seq.annotate_from_gff(DATA_DIR / "simple.gff")
-    got = list(seq.get_features_matching(start=2, stop=10))
+    got = list(seq.get_features_matching(start=2, stop=10, allow_partial=True))
     assert len(got) == 4
 
 
 def test_matching_conditions():
-    got, _ = _matching_conditions({"start": 1, "end": 5}, partial=True)
-    expect = "((start <= 1 AND end > 1) OR (start < 5 AND end >= 5) OR (start <= 1 AND end >= 5))"
+    got, _ = _matching_conditions({"start": 1, "end": 5}, allow_partial=True)
+    expect = "((start >= 1 AND end <= 5) OR (start <= 1 AND end > 1) OR (start < 5 AND end >= 5) OR (start <= 1 AND end >= 5))"
     assert got == expect
 
 
 def test_get_features_matching_start_stop_seqview(seq):
     """testing that get_features_matching adjusts"""
     seq.annotate_from_gff(DATA_DIR / "simple.gff")
-    seq_features = list(seq.get_features_matching(start=0, stop=3))
+    seq_features = list(seq.get_features_matching(start=0, stop=3, allow_partial=True))
     assert len(seq_features) == 3
 
     # edge case, only 1 features that overlaps with index 12
     # is actually returning [exon2 at [11:20]/13, CpG1 at [2:12]/13]
     # possibly a bug in the SQL generating code
     subseq = seq[9:]
-    seq_features_features = list(subseq.get_features_matching(start=3, stop=10))
+    seq_features_features = list(
+        subseq.get_features_matching(start=3, stop=10, allow_partial=True)
+    )
     assert len(seq_features_features) == 1
 
 
@@ -489,7 +491,16 @@ def test_sequence_collection_annotate_from_gff():
     # the seq for which the seqid was provided is annotated
     seq = seq_collection.get_seq("test_seq")
     assert seq_collection.get_seq("test_seq").annotation_db is not None
-    assert len(list(seq_collection.get_seq("test_seq").get_features_matching())) == 5
+    assert (
+        len(
+            list(
+                seq_collection.get_seq("test_seq").get_features_matching(
+                    allow_partial=True
+                )
+            )
+        )
+        == 5
+    )
     # the seq for which the seqid was NOT provided is NOT annotated
     assert seq_collection.get_seq("test_seq2").annotation_db is None
     # the annotation_db on the seq and the seq collection are the same object
diff --git a/tests/test_core/test_sequence.py b/tests/test_core/test_sequence.py
index b07f5b226..7a05a864f 100644
--- a/tests/test_core/test_sequence.py
+++ b/tests/test_core/test_sequence.py
@@ -101,12 +101,8 @@ class SequenceTests(TestCase):
         annot1 = s.add_feature(biotype="exon", name="annot1", spans=[(0, 10)])
         annot2 = s.add_feature(biotype="exon", name="annot2", spans=[(10, 14)])
         got = s.copy()
-        got_annot1 = list(
-            got.get_features_matching(feature_type="exon", name="annot1")
-        )[0]
-        got_annot2 = list(
-            got.get_features_matching(feature_type="exon", name="annot2")
-        )[0]
+        got_annot1 = list(got.get_features(biotype="exon", name="annot1"))[0]
+        got_annot2 = list(got.get_features(biotype="exon", name="annot2"))[0]
         self.assertIsNot(got, s)
         self.assertIsNot(got_annot1, annot1)
         self.assertIsNot(got_annot2, annot2)
@@ -1964,9 +1960,9 @@ def test_annotate_from_gff(worm_seq_path, worm_gff_path):
     seq = cogent3.load_seq(worm_seq_path, moltype="dna")
 
     seq.annotate_from_gff(worm_gff_path)
-    matches = list(seq.get_features_matching())
+    matches = list(seq.get_features())
     assert len(matches) == 11
-    matches = list(seq.get_features_matching(feature_type="gene"))
+    matches = list(seq.get_features(biotype="gene"))
     assert len(matches) == 1
     matches = list(matches[0].get_children(biotype="mRNA"))
     assert len(matches) == 1