Repository: cogent3/cogent3
Pull request #1533 (2023-07-12 08:46:17)

ENH simplify getting a translation


Patch:
diff --git a/changelog.d/20230712_184410_Gavin.Huttley.md b/changelog.d/20230712_184410_Gavin.Huttley.md
new file mode 100644
index 000000000..c0784a8b1
--- /dev/null
+++ b/changelog.d/20230712_184410_Gavin.Huttley.md
@@ -0,0 +1,42 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Contributors
+
+- A bullet item for the Contributors category.
+
+-->
+### ENH
+
+- Added trim_stop argument to get_translation() methods. This means
+  translating DNA to protein can be done with one method call, instead of
+  two.
+
+<!--
+### BUG
+
+- A bullet item for the BUG category.
+
+-->
+<!--
+### DOC
+
+- A bullet item for the DOC category.
+
+-->
+<!--
+### Deprecations
+
+- A bullet item for the Deprecations category.
+
+-->
+<!--
+### Discontinued
+
+- A bullet item for the Discontinued category.
+
+-->
diff --git a/src/cogent3/app/translate.py b/src/cogent3/app/translate.py
index a5da8e684..d401ae405 100644
--- a/src/cogent3/app/translate.py
+++ b/src/cogent3/app/translate.py
@@ -249,6 +249,4 @@ class translate_seqs:
         if self._moltype and self._moltype != seqs.moltype:
             seqs = seqs.to_moltype(self._moltype)
 
-        if self._trim_terminal_stop:
-            seqs = seqs.trim_stop_codons(gc=self._gc)
-        return seqs.get_translation(gc=self._gc)
+        return seqs.get_translation(gc=self._gc, trim_stop=self._trim_terminal_stop)
diff --git a/src/cogent3/core/alignment.py b/src/cogent3/core/alignment.py
index 2b398bad9..a3af25ca9 100644
--- a/src/cogent3/core/alignment.py
+++ b/src/cogent3/core/alignment.py
@@ -1325,7 +1325,7 @@ class _SequenceCollectionBase:
         return self.seq_len
 
     def get_translation(
-        self, gc=None, incomplete_ok=False, include_stop=False, **kwargs
+        self, gc=None, incomplete_ok=False, include_stop=False, trim_stop=True, **kwargs
     ):
         """translate from nucleic acid to protein
 
@@ -1339,6 +1339,8 @@ class _SequenceCollectionBase:
             raises a ValueError if False
         include_stop
             whether to allow a stops in the translated sequence
+        trim_stop
+            exclude terminal stop codons if they exist
         kwargs
             related to construction of the resulting object
 
@@ -1350,14 +1352,18 @@ class _SequenceCollectionBase:
             raise TypeError("Must be a DNA/RNA")
 
         translated = []
+        if trim_stop and not include_stop:
+            seqs = self.trim_stop_codons(gc=gc, strict=not incomplete_ok)
+        else:
+            seqs = self
         # do the translation
-        for seqname in self.names:
+        for seqname in seqs.names:
             try:
-                seq = self.get_gapped_seq(seqname)
+                seq = seqs.get_gapped_seq(seqname)
             except AttributeError:
-                seq = self.named_seqs[seqname]
+                seq = seqs.named_seqs[seqname]
             pep = seq.get_translation(
-                gc, incomplete_ok=incomplete_ok, include_stop=include_stop
+                gc, incomplete_ok=True, include_stop=include_stop, trim_stop=trim_stop
             )
             translated.append((seqname, pep))
         kwargs["moltype"] = pep.moltype
@@ -1393,7 +1399,13 @@ class _SequenceCollectionBase:
         return result
 
     def degap(self, **kwargs):
-        """Returns copy in which sequences have no gaps."""
+        """Returns copy in which sequences have no gaps.
+
+        Parameters
+        ----------
+        kwargs
+            passed to class constructor
+        """
         new_seqs = []
         aligned = isinstance(self, Alignment)
         for seq_name in self.names:
@@ -1454,25 +1466,27 @@ class _SequenceCollectionBase:
         """deprecated"""
         return self.has_terminal_stop(**kwargs)
 
-    def trim_stop_codons(self, gc=None, allow_partial=False, **kwargs):
+    @c3warn.deprecated_args(
+        "2023.10", "logically inconsistent", discontinued="allow_partial"
+    )
+    def trim_stop_codons(
+        self, gc: Any = None, strict: bool = False, allow_partial=False, **kwargs
+    ):
         """Removes any terminal stop codons from the sequences
 
         Parameters
         ----------
         gc
-            genetic code object
-        allow_partial
-            if True and the sequence length is not divisible
-            by 3, ignores the 3' terminal incomplete codon
-
+            valid input to cogent3.get_code(), a genetic code object, number
+            or name
+        strict
+            If True, raises an exception if a seq length not divisible by 3
         """
-        gc = get_code(gc)
-        new_seqs = []
+        if not self.has_terminal_stop(gc=gc, strict=strict):
+            return self
 
-        for seq_name in self.names:
-            old_seq = self.named_seqs[seq_name]
-            new_seq = old_seq.trim_stop_codon(gc=gc, allow_partial=allow_partial)
-            new_seqs.append((seq_name, new_seq))
+        gc = get_code(gc)
+        new_seqs = {s.name: s.trim_stop_codon(gc=gc, strict=strict) for s in self.seqs}
 
         result = self.__class__(
             moltype=self.moltype, data=new_seqs, info=self.info, **kwargs
@@ -3791,6 +3805,63 @@ class AlignmentI(object):
             width=width, height=height, wrap=wrap, vspace=vspace, colours=colours
         )
 
+    @c3warn.deprecated_args(
+        "2023.10", "logically inconsistent", discontinued="allow_partial"
+    )
+    @extend_docstring_from(_SequenceCollectionBase.trim_stop_codons)
+    def trim_stop_codons(
+        self, gc: Any = None, strict: bool = False, allow_partial=False, **kwargs
+    ):
+        if not self.has_terminal_stop(gc=gc, strict=strict):
+            return self
+
+        # define a regex for finding stop codons followed by terminal gaps
+        gc = get_code(gc)
+        gaps = "".join(self.moltype.gaps)
+        pattern = f"({'|'.join(gc['*'])})[{gaps}]*$"
+        terminal_stop = re.compile(pattern)
+
+        data = self.to_dict()
+        for name, seq in data.items():
+            if match := terminal_stop.search(seq):
+                diff = len(seq) - match.start()
+                seq = terminal_stop.sub("-" * diff, seq)
+            data[name] = seq
+
+        result = self.__class__(
+            data=data, info=self.info, moltype=self.moltype, **kwargs
+        )
+
+        if hasattr(self, "annotation_db"):
+            result.annotation_db = self.annotation_db
+
+        return result
+
+    @extend_docstring_from(_SequenceCollectionBase.get_translation)
+    def get_translation(
+        self, gc=None, incomplete_ok=False, include_stop=False, trim_stop=True, **kwargs
+    ):
+        if len(self.moltype.alphabet) != 4:
+            raise TypeError("Must be a DNA/RNA")
+
+        translated = []
+        if not trim_stop or include_stop:
+            seqs = self
+        else:
+            seqs = self.trim_stop_codons(gc=gc, strict=not incomplete_ok)
+        # do the translation
+        for seqname in seqs.names:
+            try:
+                seq = seqs.get_gapped_seq(seqname)
+            except AttributeError:
+                seq = seqs.named_seqs[seqname]
+            pep = seq.get_translation(
+                gc, incomplete_ok=incomplete_ok, include_stop=include_stop
+            )
+            translated.append((seqname, pep))
+        kwargs["moltype"] = pep.moltype
+        return self.__class__(translated, info=self.info, **kwargs)
+
 
 def _one_length(seqs):
     """raises ValueError if seqs not all same length"""
@@ -4300,71 +4371,6 @@ class ArrayAlignment(AlignmentI, _SequenceCollectionBase):
                 s = s.replace(gapchar, ambig)
         return s
 
-    def trim_stop_codons(self, gc=1, allow_partial=False, **kwargs):
-        """Removes any terminal stop codons from the sequences
-
-        Parameters
-        ----------
-        gc
-            genetic code object
-        allow_partial
-            if True and the sequence length is not divisible
-            by 3, ignores the 3' terminal incomplete codon
-
-        """
-        gc = get_code(gc)
-        if len(self) % 3 != 0 and not allow_partial:
-            raise ValueError("alignment length not divisible by 3")
-
-        stops = gc["*"]
-        get_index = self.alphabet.degen.index
-        stop_indices = {tuple(map(get_index, stop)) for stop in stops}
-        new_data = self.array_seqs.copy()
-
-        gap_indices = {get_index(gap) for gap in self.moltype.gaps}
-        gap_index = get_index(self.moltype.gap)
-
-        trim_length = len(self)
-        seqs_with_stops = 0
-        for seq_num, seq in enumerate(new_data):
-            # reverse find first non-gap character
-            nondegen_index = None
-            for i in range(len(self) - 1, -1, -1):
-                e = seq[i]
-                if e not in gap_indices:
-                    nondegen_index = i + 1
-                    if nondegen_index % 3 != 0 and not allow_partial:
-                        raise ValueError(
-                            f"{self.names[seq_num]!r} length not divisible by 3"
-                        )
-                    break
-
-            if nondegen_index is None or nondegen_index < 3:
-                continue
-
-            # slice last three valid positions and see if stop
-            end_codon = tuple(seq[nondegen_index - 3 : nondegen_index])
-            if end_codon in stop_indices:
-                seqs_with_stops += 1
-                seq[nondegen_index - 3 : nondegen_index] = gap_index
-                trim_length = min([trim_length, nondegen_index - 3])
-
-        result = self.__class__(
-            new_data.T, moltype=self.moltype, names=self.names, info=self.info
-        )
-        # this is an ugly hack for rather odd standard behaviour
-        # we find the last alignment column to have not just gap chars
-        # and trim up to that
-        i = 0
-        for i in range(len(result) - 1, -1, -1):
-            col = set(result.array_seqs[:, i])
-            if not col <= gap_indices:
-                break
-        if len(result) != i:
-            result = result[: i + 1]
-
-        return result
-
     def get_degapped_relative_to(self, name):
         """Remove all columns with gaps in sequence with given name.
 
diff --git a/src/cogent3/core/sequence.py b/src/cogent3/core/sequence.py
index 80b2c4d65..8bb1ffaca 100644
--- a/src/cogent3/core/sequence.py
+++ b/src/cogent3/core/sequence.py
@@ -1676,54 +1676,90 @@ class NucleicAcidSequence(Sequence):
             If True, raises an exception if length not divisible by 3
         """
         gc = get_code(gc)
-        divisible_by_3 = len(self) % 3 == 0
+        _, s = self.parse_out_gaps()
+
+        divisible_by_3 = len(s) % 3 == 0
         if divisible_by_3:
-            end3 = str(self[-3:])
+            end3 = str(s[-3:])
             return gc.is_stop(end3)
 
         if strict:
-            raise ValueError(f"{self.name!r} length not divisible by 3")
+            raise AlphabetError(f"{self.name!r} length not divisible by 3")
 
         return False
 
-    def trim_stop_codon(self, gc=None, allow_partial=False):
+    @deprecated_args("2023.10", "replaced by strict", discontinued="allow_partial")
+    def trim_stop_codon(
+        self, gc: Any = None, strict: bool = False, allow_partial=False
+    ):
         """Removes a terminal stop codon from the sequence
 
         Parameters
         ----------
         gc
-            genetic code object
-        allow_partial
-            if True and the sequence length is not divisible
-            by 3, ignores the 3' terminal incomplete codon
+            valid input to cogent3.get_code(), a genetic code object, number
+            or name
+        strict
+            If True, raises an exception if length not divisible by 3
 
+        Notes
+        -----
+        If sequence contains gap characters, the result preserves the sequence
+        length by adding gap characters at the end.
         """
+        if not self.has_terminal_stop(gc=gc, strict=strict):
+            return self
+
         gc = get_code(gc)
-        codons = self._seq
-        divisible_by_3 = len(codons) % 3 == 0
+        m, s = self.parse_out_gaps()
+
+        divisible_by_3 = len(s) % 3 == 0
+        if not divisible_by_3:
+            return self
+
+        end = str(s[-3:])
 
-        if not allow_partial and not divisible_by_3:
-            raise ValueError("seq length not divisible by 3")
+        if not gc.is_stop(end):
+            return self
 
-        if divisible_by_3 and codons and gc.is_stop(str(codons[-3:])):
-            codons = codons[:-3]
+        if not len(m.gaps()):
+            # has zero length if no gaps
+            return self[:-3]
 
-        result = self.__class__(codons, name=self.name, info=self.info)
+        # determine terminal gap needed to fill in the sequence
+        s = str(self)
+        gaps = "".join(self.moltype.gaps)
+        pattern = f"({'|'.join(gc['*'])})[{gaps}]*$"
+        terminal_stop = re.compile(pattern)
+        if match := terminal_stop.search(s):
+            diff = len(s) - match.start()
+            s = terminal_stop.sub("-" * diff, s)
+
+        result = self.__class__(s, name=self.name, info=self.info)
         result.annotation_db = self.annotation_db
         return result
 
-    def get_translation(self, gc=None, incomplete_ok=False, include_stop=False):
+    def get_translation(
+        self,
+        gc: Any = None,
+        incomplete_ok: bool = False,
+        include_stop: bool = False,
+        trim_stop: bool = True,
+    ):
         """translate to amino acid sequence
 
         Parameters
         ----------
         gc
-            name or ID of genetic code
+            valid input to cogent3.get_code(), a genetic code object, number
+            or name
         incomplete_ok
             codons that are mixes of nucleotide and gaps converted to '?'.
             raises a ValueError if False
         include_stop
             allows stop codons in translation
+        trim_stop
+            trims a terminal stop codon if it exists
 
         Returns
         -------
@@ -1740,14 +1776,21 @@ class NucleicAcidSequence(Sequence):
         codon_alphabet = gc.get_alphabet(include_stop=include_stop).with_gap_motif()
         # translate the codons
         translation = []
-        seq = str(self)
+        if include_stop or not trim_stop:
+            # we just deal with sequence as is
+            seq = str(self)
+        else:
+            seq = str(self.trim_stop_codon(gc=gc, strict=not incomplete_ok))
+
         for posn in range(0, len(seq) - 2, 3):
             orig_codon = str(seq[posn : posn + 3])
             try:
                 resolved = codon_alphabet.resolve_ambiguity(orig_codon)
             except AlphabetError:
                 if not incomplete_ok or "-" not in orig_codon:
-                    raise
+                    raise AlphabetError(
+                        f"unresolvable codon {orig_codon!r} in {self.name}"
+                    )
                 resolved = (orig_codon,)
             trans = []
             for codon in resolved:
@@ -1763,7 +1806,7 @@ class NucleicAcidSequence(Sequence):
                         continue
                 trans.append(aa)
             if not trans:
-                raise ValueError(orig_codon)
+                raise AlphabetError(orig_codon)
             aa = protein.what_ambiguity(trans)
             translation.append(aa)
 


Test patch:
diff --git a/tests/test_core/test_alignment.py b/tests/test_core/test_alignment.py
index 78ad9574a..7cbeaa39b 100644
--- a/tests/test_core/test_alignment.py
+++ b/tests/test_core/test_alignment.py
@@ -895,24 +895,6 @@ class SequenceCollectionBaseTests(object):
         aln = self.Class([("a", "AAAA"), ("b", "TTTT"), ("c", "CCCC")])
         self.assertEqual(len(aln), 4)
 
-    def test_get_translation_info(self):
-        """SequenceCollection.get_translation preserves info attribute"""
-        for seqs in [
-            {"seq1": "GATTTT", "seq2": "GATC??"},
-            {"seq1": "GAT---", "seq2": "?GATCT"},
-        ]:
-            alignment = self.Class(data=seqs, moltype=DNA, info={"key": "value"})
-            got = alignment.get_translation()
-            self.assertEqual(got.info["key"], "value")
-
-    def test_get_translation_incomplete(self):
-        """get translation works on incomplete codons"""
-        alignment = self.Class(data={"seq1": "GATN--", "seq2": "?GATCT"}, moltype=DNA)
-        got = alignment.get_translation(incomplete_ok=True)
-        self.assertEqual(got.to_dict(), {"seq1": "D?", "seq2": "XS"})
-        with self.assertRaises(AlphabetError):
-            got = alignment.get_translation(incomplete_ok=False)
-
     def test_get_seq(self):
         """SequenceCollection.get_seq should return specified seq"""
         aln = self.Class({"seq1": "GATTTT", "seq2": "GATC??"})
@@ -3497,13 +3479,24 @@ def test_get_translation2(cls, seqs):
 
 @pytest.mark.parametrize("cls", (SequenceCollection, Alignment, ArrayAlignment))
 def test_get_translation_with_stop(cls):
-    """SequenceCollection.get_translation translates each seq"""
     seqs = {"seq1": "GATTAG", "seq2": "?GATCT"}
     alignment = cls(data=seqs, moltype=DNA)
     got = alignment.get_translation(include_stop=True)
     assert got.to_dict() == {"seq1": "D*", "seq2": "XS"}
 
 
+@pytest.mark.parametrize("cls", (Alignment, ArrayAlignment, SequenceCollection))
+def test_get_translation_trim_stop(cls):
+    seqs = {"seq1": "GATTCCTAG", "seq2": "GATTCCTCC"}
+    alignment = cls(data=seqs, moltype=DNA)
+    expect = {"seq1": "DS", "seq2": "DSS"}
+    if cls != SequenceCollection:
+        expect = {"seq1": "DS-", "seq2": "DSS"}
+
+    got = alignment.get_translation(trim_stop=True)
+    assert got.to_dict() == expect
+
+
 @pytest.mark.parametrize("cls", (SequenceCollection, Alignment, ArrayAlignment))
 @pytest.mark.parametrize(
     "seqs", ({"seq1": "GATTTT", "seq2": "GATC??"}, {"seq1": "GAT---", "seq2": "?GATCT"})
@@ -3598,7 +3591,27 @@ def test_aligned_rich_dict(reverse):
 
 @pytest.mark.parametrize("cls", (SequenceCollection, Alignment, ArrayAlignment))
 @pytest.mark.parametrize(
-    "gc,seqs", ((1, ("TCCTGA", "GATTT?")), (2, ("GATTTT", "TCCAGG")))
+    "seqs",
+    (
+        {"seq1": "GATTTT", "seq2": "GATC??"},
+        {"seq1": "GAT---", "seq2": "?GATCT"},
+    ),
+)
+def test_get_translation_info(cls, seqs):
+    """SequenceCollection.get_translation preserves info attribute"""
+    alignment = cls(data=seqs, moltype=DNA, info={"key": "value"})
+    got = alignment.get_translation()
+    assert got.info["key"] == "value"
+
+
+@pytest.mark.parametrize("cls", (SequenceCollection, Alignment, ArrayAlignment))
+@pytest.mark.parametrize(
+    "gc,seqs",
+    (
+        (1, ("TCCTGA", "GATTT?")),
+        (1, ("ACGTAA---", "ACGAC----", "ACGCAATGA")),
+        (2, ("GATTTT", "TCCAGG")),
+    ),
 )
 def test_has_terminal_stop_true(cls, gc, seqs):
     gc = get_code(gc)
@@ -3624,5 +3637,78 @@ def test_has_terminal_stop_strict(cls):
     gc = get_code(1)
     data = {f"s{i}": s for i, s in enumerate(("CCTCA", "ATTTT"))}
     seqs = cls(data=data, moltype="dna")
-    with pytest.raises(ValueError):
+    with pytest.raises(AlphabetError):
         seqs.has_terminal_stop(gc=gc, strict=True)
+
+
+@pytest.mark.parametrize("cls", (SequenceCollection, Alignment, ArrayAlignment))
+@pytest.mark.parametrize(
+    "gc,seqs",
+    (
+        (1, ("--AT-CTGA", "GATAAATT?")),
+        (1, ("ACGTGA---", "ACGAC----", "ACGCAATGA")),
+        (1, ("CCTCA-", "ATTTTA")),
+        (2, ("GATTTT", "TCCAGG")),
+    ),
+)
+def test_trim_stops_true(cls, gc, seqs):
+    gc = get_code(gc)
+    data = {f"s{i}": s for i, s in enumerate(seqs)}
+
+    expect = {}
+    for k, v in data.items():
+        if cls != SequenceCollection or "-" in v:
+            v = re.sub("(TGA|AGG)(?=[-]*$)", "---", v)
+        else:
+            v = re.sub("(TGA|AGG)", "", v)
+        expect[k] = v
+
+    seqs = cls(data=data, moltype="dna")
+    got = seqs.trim_stop_codons(gc=gc).to_dict()
+
+    assert got == expect
+
+
+@pytest.mark.parametrize("cls", (SequenceCollection, Alignment, ArrayAlignment))
+@pytest.mark.parametrize(
+    "gc,seqs",
+    ((1, ("T-CTGC", "GATAA?")), (2, ("GATTTT", "TCCCGG")), (1, ("CCTGC", "GATAA"))),
+)
+def test_trim_terminal_stops_nostop(cls, gc, seqs):
+    gc = get_code(gc)
+    data = {f"s{i}": s for i, s in enumerate(seqs)}
+    seqs = cls(data=data, moltype="dna")
+    got = seqs.trim_stop_codons(gc=gc)
+    assert got is seqs
+
+
+@pytest.mark.parametrize("cls", (SequenceCollection, Alignment, ArrayAlignment))
+@pytest.mark.parametrize("seqs", (("CCTCA", "ATTTT"), ("CCTCA-", "ATTTTA")))
+def test_trim_terminal_stops_strict(cls, seqs):
+    gc = get_code(1)
+    data = {f"s{i}": s for i, s in enumerate(seqs)}
+    seqs = cls(data=data, moltype="dna")
+    with pytest.raises(AlphabetError):
+        seqs.trim_stop_codons(gc=gc, strict=True)
+
+
+@pytest.mark.parametrize("cls", (SequenceCollection, Alignment, ArrayAlignment))
+def test_trim_stop_codons_info(cls):
+    """trim_stop_codons should preserve info attribute"""
+    coll = cls(
+        data={"seq1": "ACGTAA", "seq2": "ACGACG", "seq3": "ACGCGT"},
+        moltype=DNA,
+        info={"key": "value"},
+    )
+    coll = coll.trim_stop_codons()
+    assert coll.info["key"] == "value"
+
+
+@pytest.mark.parametrize("cls", (SequenceCollection, Alignment, ArrayAlignment))
+def test_get_translation_incomplete(cls):
+    """get translation works on incomplete codons"""
+    alignment = cls(data={"seq1": "GATN--", "seq2": "?GATCT"}, moltype=DNA)
+    got = alignment.get_translation(incomplete_ok=True)
+    assert got.to_dict() == {"seq1": "D?", "seq2": "XS"}
+    with pytest.raises(AlphabetError):
+        _ = alignment.get_translation(incomplete_ok=False)
diff --git a/tests/test_core/test_core_standalone.py b/tests/test_core/test_core_standalone.py
index c7782d201..7dd4e4b12 100644
--- a/tests/test_core/test_core_standalone.py
+++ b/tests/test_core/test_core_standalone.py
@@ -674,92 +674,6 @@ class AlignmentTestMethods(unittest.TestCase):
         s1 = self.alignment.get_seq("Mouse")
         self.assertEqual(s1.get_name(), "Mouse")
 
-    def test_trim_stop_codons(self):
-        """test without terminal stop handling"""
-        seq_coll = make_unaligned_seqs(
-            data={"seq1": "ACGTAA", "seq2": "ACGACG", "seq3": "ACGCGT"}, moltype=DNA
-        )
-        seq_coll = seq_coll.trim_stop_codons()
-        seqs = seq_coll.to_dict()
-        self.assertEqual(seqs["seq1"], "ACG")  # note: not 'acg---'
-        self.assertEqual(seqs["seq2"], "ACGACG")
-        # aligned
-        aln = make_aligned_seqs(
-            data={"seq1": "ACGTAA", "seq2": "ACGTGA", "seq3": "ACGTAA"}, moltype=DNA
-        )
-        aln = aln.trim_stop_codons()
-        self.assertEqual(
-            aln.to_dict(), {"seq1": "ACG", "seq2": "ACG", "seq3": "ACG"}
-        )  # note: not 'acg---'
-        aln = make_aligned_seqs(
-            data={"seq1": "ACGAAA", "seq2": "ACGTGA", "seq3": "ACGTAA"}, moltype=DNA
-        )
-        aln = aln.trim_stop_codons()
-        self.assertEqual(
-            aln.to_dict(), {"seq1": "ACGAAA", "seq2": "ACG---", "seq3": "ACG---"}
-        )
-
-        # for case where a sequence length is not divisible by 3
-        seq_coll = make_unaligned_seqs(
-            data={"seq1": "ACGTAA", "seq2": "ACGAC"}, moltype=DNA
-        )
-        # fail
-        self.assertRaises(ValueError, seq_coll.trim_stop_codons)
-        # unless explicitly over-ridden with allow_partial
-        new_coll = seq_coll.trim_stop_codons(allow_partial=True)
-        self.assertEqual(new_coll.to_dict(), dict(seq1="ACG", seq2="ACGAC"))
-
-        # should work for alignments too
-        aln = make_aligned_seqs(
-            data={"seq1": "ACGTAA---", "seq2": "ACGAC----", "seq3": "ACGCAATTT"},
-            moltype=DNA,
-        )
-        # fail
-        self.assertRaises(ValueError, aln.trim_stop_codons)
-        # unless explicitly over-ridden with allow_partial
-        aln = aln.trim_stop_codons(allow_partial=True)
-        self.assertEqual(
-            aln.to_dict(),
-            {"seq1": "ACG------", "seq2": "ACGAC----", "seq3": "ACGCAATTT"},
-        )
-        # mixed lengths
-        aln = make_aligned_seqs(
-            data={"seq1": "ACGTAA---", "seq2": "ACGAC----", "seq3": "ACGCAATGA"},
-            moltype=DNA,
-        )
-        aln = aln.trim_stop_codons(allow_partial=True)
-        self.assertEqual(
-            aln.to_dict(), {"seq1": "ACG---", "seq2": "ACGAC-", "seq3": "ACGCAA"}
-        )
-        # longest seq not divisible by 3
-        aln = make_aligned_seqs(
-            data={"seq1": "ACGTAA--", "seq2": "ACGAC---", "seq3": "ACGC-ATG"},
-            moltype=DNA,
-        )
-        aln = aln.trim_stop_codons(allow_partial=True)
-        self.assertEqual(
-            aln.to_dict(), {"seq1": "ACG-----", "seq2": "ACGAC---", "seq3": "ACGC-ATG"}
-        )
-
-    def test_trim_stop_codons_info(self):
-        """trim_stop_codons should preserve info attribute"""
-        seq_coll = SequenceCollection(
-            data={"seq1": "ACGTAA", "seq2": "ACGACG", "seq3": "ACGCGT"},
-            moltype=DNA,
-            info={"key": "value"},
-        )
-        seq_coll = seq_coll.trim_stop_codons()
-        self.assertEqual(seq_coll.info["key"], "value")
-
-        # aligned
-        aln = ArrayAlignment(
-            data={"seq1": "ACGTAA", "seq2": "ACGTGA", "seq3": "ACGTAA"},
-            moltype=DNA,
-            info={"key": "value"},
-        )
-        aln = aln.trim_stop_codons()
-        self.assertEqual(aln.info["key"], "value")
-
     def test_slice(self):
         seqs = {"seq1": "ACGTANGT", "seq2": "ACGTACGT", "seq3": "ACGTACGT"}
         alignment = make_aligned_seqs(data=seqs)
@@ -905,25 +819,11 @@ class SequenceTestMethods(unittest.TestCase):
         seq = make_seq(moltype=DNA, seq="ACTG-TAA")
         rev = seq.reverse_complement()
         self.assertEqual(str(rev), "TTA-CAGT")
-        # try amigbuities
+        # try ambiguities
         seq = make_seq(moltype=DNA, seq="ACHNRTAA")
         rev = seq.reverse_complement()
         self.assertEqual(str(rev), "TTAYNDGT")
 
-    def test_without_terminal_stop_sodon(self):
-        """testing deleting terminal stop"""
-        # for standard code
-        seq = make_seq(moltype=DNA, seq="ACTTAA")
-        seq2 = seq.trim_stop_codon()
-        self.assertEqual(str(seq2), "ACT")
-
-        # for sequence not divisible by 3
-        seq = make_seq(moltype=DNA, seq="ACTTA")
-        # fail
-        self.assertRaises(ValueError, seq.trim_stop_codon)
-        # unless explicitly over-ride length issue using allow_partial
-        seq2 = seq.trim_stop_codon(allow_partial=True)
-
 
 def test_load_seq_new():
     """load single sequence"""
diff --git a/tests/test_core/test_sequence.py b/tests/test_core/test_sequence.py
index b129af45f..51a082c42 100644
--- a/tests/test_core/test_sequence.py
+++ b/tests/test_core/test_sequence.py
@@ -1265,6 +1265,16 @@ def test_get_translation_include_stop():
     assert str(aa) == "I*L"
 
 
+def test_get_translation_trim_stop():
+    s = DNA.make_seq("ATTTCCTGA", name="s1")
+    aa = s.get_translation(trim_stop=True)
+    assert str(aa) == "IS"
+    # no effect on internal stops
+    s = DNA.make_seq("ATTTAACTT", name="s1")
+    aa = s.get_translation(include_stop=True, trim_stop=True)
+    assert str(aa) == "I*L"
+
+
 @pytest.mark.parametrize("start", (None, 0, 1, 10, -1, -10))
 @pytest.mark.parametrize("stop", (None, 10, 8, 1, 0, -1, -11))
 @pytest.mark.parametrize("step", (None, 1, 2, -1, -2))
@@ -2390,7 +2400,7 @@ def test_get_drawable():
         assert "(incomplete)" in trace.text
 
 
-@pytest.mark.parametrize("gc,seq", ((1, "TCCTGA"), (2, "TCCAGG")))
+@pytest.mark.parametrize("gc,seq", ((1, "TCCTGA"), (1, "ACGTAA---"), (2, "TCCAGG")))
 def test_has_terminal_stop_true(gc, seq):
     gc = cogent3.get_code(gc)
     seq = cogent3.make_seq(seq, moltype="dna")
@@ -2409,5 +2419,49 @@ def test_has_terminal_stop_false(gc, seq):
 def test_has_terminal_stop_strict():
     gc = cogent3.get_code(1)
     seq = cogent3.make_seq("TCCAG", moltype="dna")
-    with pytest.raises(ValueError):
+    with pytest.raises(AlphabetError):
         seq.has_terminal_stop(gc=gc, strict=True)
+
+
+@pytest.mark.parametrize(
+    "gc,seq",
+    (
+        (2, "TCCAGG"),
+        (1, "TAATGA"),
+        (1, "ACGTGA---"),
+        (1, "--AT-CTGA"),
+    ),
+)
+def test_trim_terminal_stop_true(gc, seq):
+    gc = cogent3.get_code(gc)
+    expect = re.sub("(TGA|AGG)(?=[-]*$)", "---" if "-" in seq else "", seq)
+
+    seq = cogent3.make_seq(seq, moltype="dna")
+    got = str(seq.trim_stop_codon(gc=gc))
+    assert got == expect
+
+
+@pytest.mark.parametrize("gc,seq", ((1, "T?CTGC"), (2, "TCCAAG")))
+def test_trim_terminal_stop_nostop(gc, seq):
+    gc = cogent3.get_code(gc)
+    seq = cogent3.make_seq(seq, moltype="dna")
+    got = seq.trim_stop_codon(gc=gc)
+    assert str(got) == str(seq)
+    # since there's no stop, we just return the same object
+    assert got is seq
+
+
+@pytest.mark.parametrize(
+    "gc,seq", ((1, "TCCAGG"), (2, "TCCAAA"), (1, "CCTGA"), (2, "CCAGG"))
+)
+def test_trim_terminal_stop_false(gc, seq):
+    gc = cogent3.get_code(gc)
+    seq = cogent3.make_seq(seq, moltype="dna")
+    assert str(seq.trim_stop_codon(gc=gc)) == str(seq)
+
+
+def test_trim_terminal_stop_strict():
+    gc = cogent3.get_code(1)
+    seq = cogent3.make_seq("TCCAG", moltype="dna")
+    with pytest.raises(AlphabetError):
+        seq.trim_stop_codon(gc=gc, strict=True)