Repository: cogent3/cogent3
Pull request #1498 (2023-07-02 08:35:41)

MAINT: consistency on annotation behaviour


Patch:
diff --git a/src/cogent3/core/alignment.py b/src/cogent3/core/alignment.py
index 7153ac414..a5132c465 100644
--- a/src/cogent3/core/alignment.py
+++ b/src/cogent3/core/alignment.py
@@ -2078,6 +2078,9 @@ class SequenceCollection(_SequenceCollectionBase):
             # no matching ID's, nothing to do
             return
 
+        if self.annotation_db is None:
+            self.annotation_db = type(seq_db)()
+
         if self.annotation_db.compatible(seq_db, symmetric=False):
             # our db contains the tables in other, so we update in place
             self.annotation_db.update(annot_db=seq_db, seqids=self.names)
@@ -4721,6 +4724,8 @@ class Alignment(AlignmentI, SequenceCollection):
 
     def __getitem__(self, index):
         if hasattr(index, "get_slice"):
+            if index.parent is not self:
+                raise ValueError(f"feature.parent {index.seqid!r} is not self")
             return index.get_slice()
 
         if isinstance(index, Map):
@@ -4731,6 +4736,9 @@ class Alignment(AlignmentI, SequenceCollection):
 
             new = self.__class__(seqs, name=self.name, info=self.info)
 
+        if isinstance(index, (list, tuple)):
+            raise TypeError(f"cannot slice using {type(index)}")
+
         if self.annotation_db is not None:
             new.annotation_db = self.annotation_db
 
diff --git a/src/cogent3/core/annotation.py b/src/cogent3/core/annotation.py
index b83b05084..d3c203652 100644
--- a/src/cogent3/core/annotation.py
+++ b/src/cogent3/core/annotation.py
@@ -1,7 +1,7 @@
 import copy
 import json
 
-from typing import Optional
+from typing import Iterable, Optional
 
 from cogent3._version import __version__
 from cogent3.util import warning as c3warn
@@ -167,7 +167,7 @@ class Feature:
         for child in db.get_feature_parent(biotype=biotype, name=self.name):
             yield make_feature(feature=child)
 
-    def union(self, features):
+    def union(self, features: Iterable):
         """return as a single Feature
 
         Notes
@@ -175,16 +175,16 @@ class Feature:
         Overlapping spans are merged
         """
         combined = self.map.spans[:]
-        feat_names = set()
-        biotypes = set()
-        seqids = set()
+        feat_names = [self.name] if self.name else set()
+        biotypes = {self.biotype} if self.biotype else set()
+        seqids = {self.seqid} if self.seqid else set()
         for feature in features:
             if feature.parent is not self.parent:
                 raise ValueError(f"cannot merge annotations from different objects")
 
             combined.extend(feature.map.spans)
             if feature.name:
-                feat_names.add(feature.name)
+                feat_names.append(feature.name)
             if feature.seqid:
                 seqids.add(feature.seqid)
             if feature.biotype:
diff --git a/src/cogent3/core/sequence.py b/src/cogent3/core/sequence.py
index 1bfc94301..81090c9bf 100644
--- a/src/cogent3/core/sequence.py
+++ b/src/cogent3/core/sequence.py
@@ -1214,10 +1214,10 @@ class Sequence(SequenceI):
         if self.annotation_db and not self.annotation_db.compatible(seq_db):
             raise TypeError(f"type {type(seq_db)} != {type(self.annotation_db)}")
 
-        if not self.annotation_db:
+        if self.annotation_db is None:
             self.annotation_db = type(seq_db)()
 
-        self.annotation_db.union(seq_db, seqids=self.name)
+        self.annotation_db.update(seq_db, seqids=self.name)
 
     def copy(self, exclude_annotations=False):
         """returns a copy of self"""
@@ -1331,6 +1331,11 @@ class Sequence(SequenceI):
 
     def __getitem__(self, index):
         preserve_offset = False
+        if hasattr(index, "get_slice"):
+            if index.parent is not self:
+                raise ValueError("cannot slice Feature not bound to self")
+            return index.get_slice()
+
         if hasattr(index, "map"):
             index = index.map
 
@@ -1345,6 +1350,9 @@ class Sequence(SequenceI):
             stride = getattr(index, "step", 1) or 1
             preserve_offset = stride > 0
 
+        if isinstance(index, (list, tuple)):
+            raise TypeError("cannot slice using list or tuple")
+
         if self.annotation_db is not None and preserve_offset:
             new.replace_annotation_db(self.annotation_db, check=False)
             new.annotation_offset = self.annotation_offset


Test patch:
diff --git a/tests/test_core/test_annotation.py b/tests/test_core/test_annotation.py
index abd90281d..5210327c3 100644
--- a/tests/test_core/test_annotation.py
+++ b/tests/test_core/test_annotation.py
@@ -273,3 +273,20 @@ def test_aln_feature_to_dict():
     f = aln.make_feature(feature=feature_data)
     d = f.to_dict()
     assert d == expect
+
+
+@pytest.mark.parametrize("fix", ("ann_seq", "ann_aln"))
+@pytest.mark.parametrize("type_", (list, tuple))
+def test_aln_slice_feat_invalid(type_, fix, request):
+    # incorrect parent
+    obj = request.getfixturevalue(fix)
+    with pytest.raises(TypeError):
+        _ = obj[type_(obj.get_features(biotype="exon"))]
+
+
+def test_seq_slice_seqfeat_invalid(ann_aln):
+    # incorrect parent
+    seq1 = ann_aln.get_seq("FAKE01")
+    seq2 = ann_aln.get_seq("FAKE02")
+    with pytest.raises(ValueError):
+        _ = seq2[list(seq1.get_features(biotype="CDS"))[0]]
diff --git a/tests/test_core/test_features.py b/tests/test_core/test_features.py
index 6c0359c2d..22d0cd11a 100755
--- a/tests/test_core/test_features.py
+++ b/tests/test_core/test_features.py
@@ -3,7 +3,7 @@ from unittest import TestCase
 import pytest
 
 from cogent3 import ASCII, DNA, get_moltype, make_aligned_seqs
-from cogent3.core.annotation_db import GffAnnotationDb
+from cogent3.core.annotation_db import BasicAnnotationDb, GffAnnotationDb
 # Complete version of manipulating sequence annotations
 from cogent3.util.deserialise import deserialise_object
 
@@ -105,6 +105,19 @@ def test_copy_annotations():
     assert feat.get_slice().to_dict() == dict(x="AAA", y="CCT")
 
 
+def test_copy_annotations_onto_seq():
+    """copying features onto a sequence"""
+    aln = make_aligned_seqs(
+        data=[["x", "-AAAAAAAAA"], ["y", "TTTT--CCCT"]], array_align=False
+    )
+    db = BasicAnnotationDb()
+    db.add_feature(seqid="y", biotype="exon", name="A", spans=[(5, 8)])
+    y = aln.get_seq("y")
+    y.copy_annotations(db)
+    feat = list(aln.get_features(seqid="y", biotype="exon"))[0]
+    assert feat.get_slice().to_dict() == dict(x="AAA", y="CCT")
+
+
 def test_feature_residue():
     """seq features on alignment operate in sequence coordinates"""
     # In this case, only those residues included within the feature are
diff --git a/tests/test_core/test_maps.py b/tests/test_core/test_maps.py
index a6a098a79..6ccb21b31 100644
--- a/tests/test_core/test_maps.py
+++ b/tests/test_core/test_maps.py
@@ -43,7 +43,7 @@ class MapTest(unittest.TestCase):
         sliced_seq = seq[:17]
         answer = list(sliced_seq.get_features(biotype="test_type", allow_partial=False))
         self.assertEqual(len(answer), 1)
-        self.assertEqual(str(seq[answer[0]]), "TCGAT")
+        self.assertEqual(str(sliced_seq[answer[0]]), "TCGAT")
 
 
 def test_get_by_seq_annotation_allow_gaps():