Repository: cogent3/cogent3
Pull request #1912 (2024-06-24 06:16:33)

refactor seqs.get_translation and add method to rename seqs in a collection

coveralls: ## Pull Request Test Coverage Report for [Build 9640515796](https://coveralls.io/builds/68251840)


### Details

* **42** of **52**   **(80.77%)**  changed or added relevant lines in **4** files are covered.
* **3** unchanged lines in **2** files lost coverage.
* Overall coverage increased (+**0.09%**) to **91.231%**

---

|  Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [src/cogent3/core/new_sequence.py](https://coveralls.io/builds/68251840/source?filename=src%2Fcogent3%2Fcore%2Fnew_sequence.py#L1447) | 10 | 12 | 83.33%
| [src/cogent3/core/profile.py](https://coveralls.io/builds/68251840/source?filename=src%2Fcogent3%2Fcore%2Fprofile.py#L553) | 11 | 14 | 78.57%
| [src/cogent3/core/new_alignment.py](https://coveralls.io/builds/68251840/source?filename=src%2Fcogent3%2Fcore%2Fnew_alignment.py#L208) | 20 | 25 | 80.0%
<!-- | **Total:** | **42** | **52** | **80.77%** | -->

|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [src/cogent3/core/new_sequence.py](https://coveralls.io/builds/68251840/source?filename=src%2Fcogent3%2Fcore%2Fnew_sequence.py#L1451) | 1 | 65.1% |
| [src/cogent3/core/new_genetic_code.py](https://coveralls.io/builds/68251840/source?filename=src%2Fcogent3%2Fcore%2Fnew_genetic_code.py#L178) | 2 | 94.74% |
<!-- | **Total:** | **3** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/68251840/badge)](https://coveralls.io/builds/68251840) |
| :-- | --: |
| Change from base [Build 9639392196](https://coveralls.io/builds/68250454): |  0.09% |
| Covered Lines: | 32479 |
| Relevant Lines: | 35601 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/src/cogent3/core/new_alignment.py b/src/cogent3/core/new_alignment.py
index ef1f681dd..08c11ec05 100644
--- a/src/cogent3/core/new_alignment.py
+++ b/src/cogent3/core/new_alignment.py
@@ -25,9 +25,14 @@ from cogent3.core.annotation_db import (
 )
 from cogent3.core.info import Info as InfoClass
 from cogent3.core.location import IndelMap
-from cogent3.core.profile import PSSM, MotifCountsArray, MotifFreqsArray
+from cogent3.core.profile import (
+    PSSM,
+    MotifCountsArray,
+    MotifFreqsArray,
+    load_pssm,
+)
 from cogent3.format.alignment import save_to_filename
-from cogent3.format.fasta import alignment_to_fasta
+from cogent3.format.fasta import seqs_to_fasta
 from cogent3.format.phylip import alignment_to_phylip
 from cogent3.util import progress_display as UI
 from cogent3.util import warning as c3warn
@@ -57,13 +62,25 @@ class MakeSeqCallable(typing.Protocol):
 
 
 def assign_sequential_names(num_seqs: int, base_name: str = "seq", start_at: int = 0):
-    """Returns list of num_seqs sequential, unique names."""
+    """Returns list of sequential, unique names, e.g., ['seq_0' ... 'seq_n'] for
+     num_seqs = n+1
+
+    Parameters
+    ----------
+    num_seqs
+        number of names to generate
+    base_name
+        the sequence name prefix
+    start_at
+        the number to start the sequence names at
+    """
     return [f"{base_name}_{i}" for i in range(start_at, start_at + num_seqs)]
 
 
 class SeqDataView(new_seq.SeqViewABC, new_seq.SliceRecordABC):
     """
-    A view class for SeqsData, providing properties for different representations.
+    A view class for SeqsData, providing methods for different representations
+    of a single sequence.
 
     self.seq is a SeqsData() instance, but other properties are a reference to a
     single seqid only.
@@ -183,11 +200,12 @@ class SeqsDataABC(ABC):
     @abstractmethod
     def make_seq(self): ...
 
-    # refactor: docstring
-
     @make_seq.setter
     @abstractmethod
-    def make_seq(self, make_seq: MakeSeqCallable) -> None: ...
+    def make_seq(self, make_seq: MakeSeqCallable) -> None:
+        """Can be set with any callable function that takes 'seq' and 'name' as
+        keyword arguments. Typically set with '<moltype-instance>.make_seq'."""
+        ...
 
     @property
     @abstractmethod
@@ -230,6 +248,7 @@ class SeqsDataABC(ABC):
 
 
 class SeqsData(SeqsDataABC):
+    # refactor: docstring
     __slots__ = ("_data", "_alphabet", "_make_seq")
     # todo: kath
     # refactor: design
@@ -444,9 +463,7 @@ class SequenceCollection:
 
     def iter_seqs(
         self, seq_order: OptList = None
-    ) -> Iterator[
-        Union[new_seq.Sequence, new_seq.SeqViewABC]
-    ]:  # refactor: need to deal with optional args type hints
+    ) -> Iterator[Union[new_seq.Sequence, new_seq.SeqViewABC]]:
         """Iterates over sequences in the collection, in order.
 
         Parameters
@@ -593,10 +610,33 @@ class SequenceCollection:
             annotation_db=self.annotation_db,
         )
 
-    def rename_seqs(self, renamer):
-        # todo: kath
-        # returns new SeqsData instance
-        ...
+    def rename_seqs(self, renamer: Callable[[str], str]):
+        """Returns new collection with renamed sequences."""
+
+        # refactor: design
+        # consider whether we want new seq instances rather than just renaming
+        data = self.to_dict(as_array=True)
+        new_data = {renamer(name): seq for name, seq in data.items()}
+
+        seqs_data = self.seqs.__class__(
+            data=new_data, alphabet=self.seqs.alphabet, make_seq=self.seqs.make_seq
+        )
+
+        result = self.__class__(
+            seqs_data=seqs_data,
+            moltype=self.moltype,
+            info=self.info,
+            source=self.source,
+        )
+        if self.annotation_db:
+            # refactor: design
+            # how to manage to keep track of aliases? possibly stored
+            # in the annotation db
+            name_map = {renamer(name): name for name in self.names}
+            result.info.name_map = name_map
+            result.annotation_db = self.annotation_db
+
+        return result
 
     def to_dict(self, as_array: bool = False) -> dict[str, Union[str, numpy.ndarray]]:
         """Return a dictionary of sequences.
@@ -610,6 +650,17 @@ class SequenceCollection:
         get = self.seqs.get_seq_array if as_array else self.seqs.get_seq_str
         return {name: get(seqid=name) for name in self.names}
 
+    def to_json(self):
+        # todo
+        # refactor: design - waiting for alphabet serialisation
+        ...
+
+    def to_rich_dict(self) -> dict:
+        # todo
+        # refactor: design - waiting for alphabet serialisation
+        # add SeqsData.to_dict method which returns a dict [str: str]
+        ...
+
     def degap(self) -> SequenceCollection:
         """Returns new collection in which sequences have no gaps.
 
@@ -622,10 +673,6 @@ class SequenceCollection:
             name: self.moltype.degap(self.seqs.get_seq_array(seqid=name))
             for name in self.names
         }
-        # refactor: design
-        # this wont work if using a SeqsData that is initialised differently
-        # should we put a degap method on the SeqsDataABC class?
-        # or something alike get_init_kwargs()?
         seqs_data = self.seqs.__class__(
             data=seqs, alphabet=self.seqs.alphabet, make_seq=self.seqs.make_seq
         )
@@ -669,13 +716,11 @@ class SequenceCollection:
         )
 
     def to_dna(self):
-        """returns copy of self as an alignment of DNA moltype seqs"""
-        # refactor: design
-        # do we need this in addition to to_moltype?
+        """returns copy of self as a collection of DNA moltype seqs"""
         return self.to_moltype("dna")
 
     def to_rna(self):
-        """returns copy of self as an alignment of RNA moltype seqs"""
+        """returns copy of self as a collection of RNA moltype seqs"""
         return self.to_moltype("rna")
 
     def get_translation(
@@ -686,7 +731,7 @@ class SequenceCollection:
         trim_stop: bool = True,
         **kwargs,
     ):
-        """translate from nucleic acid to protein
+        """translate sequences from nucleic acid to protein
 
         Parameters
         ----------
@@ -706,33 +751,41 @@ class SequenceCollection:
         Returns
         -------
         A new instance of self translated into protein
+
+        Notes
+        -----
+        Translating will break the relationship to an annotation_db if present.
         """
-        # todo: kath, add check for when include_stop==True, the seq.get_translation method should be
-        # returning sequences with a protein_with_stop moltype.
 
         if len(self.moltype.alphabet) != 4:
-            raise TypeError("Sequences must be a DNA/RNA")
+            raise new_alpha.AlphabetError("Sequences 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 seqs.names:
-            seq = seqs.seqs[seqname]
+        for seqname in self.names:
+            seq = self.seqs[seqname]
             pep = seq.get_translation(
-                gc, incomplete_ok=True, include_stop=include_stop, trim_stop=trim_stop
+                gc,
+                incomplete_ok=incomplete_ok,
+                include_stop=include_stop,
+                trim_stop=trim_stop,
             )
             translated[seqname] = pep
+
+        seqs_data = coerce_to_seqs_data_dict(translated, label_to_name=None)
         pep_moltype = pep.moltype
-        seqs_data = SeqsData(
-            data=translated,
+
+        seqs_data = self.seqs.__class__(
+            data=seqs_data,
             alphabet=pep_moltype.most_degen_alphabet(),
             make_seq=pep_moltype.make_seq,
         )
         return self.__class__(
-            seqs_data=seqs_data, info=self.info, source=self.source, **kwargs
+            seqs_data=seqs_data,
+            moltype=pep_moltype,
+            info=self.info,
+            source=self.source,
+            **kwargs,
         )
 
     def rc(self):
@@ -747,7 +800,9 @@ class SequenceCollection:
         """
         rc_seqs = {name: self.seqs[name].rc() for name in self.names}
         seqs_data = self.seqs.__class__(
-            data=rc_seqs, alphabet=self.moltype.alphabet, make_seq=self.moltype.make_seq
+            data=rc_seqs,
+            alphabet=self.moltype.most_degen_alphabet(),
+            make_seq=self.moltype.make_seq,
         )
         return self.__class__(
             seqs_data=seqs_data,
@@ -929,7 +984,8 @@ class SequenceCollection:
         start: OptInt = None,
         stop: OptInt = None,
         allow_partial: bool = False,
-    ) -> Iterator[Feature]:  # refactor: need to deal with optional args type hints
+        **kwargs,
+    ) -> Iterator[Feature]:
         """yields Feature instances
 
         Parameters
@@ -946,6 +1002,8 @@ class SequenceCollection:
             stop position of the feature (inclusive)
         allow_partial
             allow features partially overlaping self
+        kwargs
+            additional keyword arguments to query the annotation db
 
         Notes
         -----
@@ -959,6 +1017,7 @@ class SequenceCollection:
         if not self.annotation_db:
             return None
 
+        # create a map between original seqid and current seqname (if renamed)
         seqid_to_seqname = {seq.parent_coordinates()[0]: seq.name for seq in self.seqs}
         if seqid and (
             seqid not in seqid_to_seqname and seqid not in seqid_to_seqname.values()
@@ -973,6 +1032,7 @@ class SequenceCollection:
             start=start,
             stop=stop,
             allow_partial=allow_partial,
+            **kwargs,
         ):
             seqname = seqid_to_seqname[feature["seqid"]]
             seq = self.seqs[seqname]
@@ -993,7 +1053,7 @@ class SequenceCollection:
         -------
         The collection in Fasta format.
         """
-        return alignment_to_fasta(self.to_dict(), block_size=block_size)
+        return seqs_to_fasta(self.to_dict(), block_size=block_size)
 
     def to_phylip(self):
         """
@@ -1001,7 +1061,7 @@ class SequenceCollection:
 
         Notes
         -----
-        raises exception if invalid sequences do not all have the same length
+        raises exception if sequences do not all have the same length
         """
         if self.is_ragged():
             raise ValueError("not all seqs same length, cannot convert to phylip")
@@ -1156,7 +1216,7 @@ class SequenceCollection:
         pseudocount: int = 0,
         names: OptList = None,
         ui=None,
-    ) -> numpy.array:
+    ) -> numpy.array:  # refactor: design: move to rich for progress bars?
         """scores sequences using the specified pssm
 
         Parameters
@@ -1172,39 +1232,21 @@ class SequenceCollection:
             adjustment for zero in matrix
         names
             returns only scores for these sequences and in the name order
-        ui
-            argument to control the display of a progress bar. i.e.,
-            'show_progress=True'
-
 
         Returns
         -------
         numpy array of log2 based scores at every position
         """
         assert not self.is_ragged(), "all sequences must have same length"
-        from cogent3.parse import cisbp, jaspar
-
         assert pssm or path, "Must specify a PSSM or a path"
         assert not (pssm and path), "Can only specify one of pssm, path"
 
         if isinstance(names, str):
             names = [names]
 
-        # refactor: design - delegate below logic to seperate function load_pssm()
         if path:
-            is_cisbp = path.endswith("cisbp")
-            is_jaspar = path.endswith("jaspar")
-            if not (is_cisbp or is_jaspar):
-                raise NotImplementedError(f"Unknown format {path.split('.')[-1]}")
-
-            if is_cisbp:
-                pfp = cisbp.read(path)
-                pssm = pfp.to_pssm(background=background)
-            else:
-                _, pwm = jaspar.read(path)
-                pssm = pwm.to_pssm(background=background, pseudocount=pseudocount)
+            pssm = load_pssm(path, background=background, pseudocount=pseudocount)
 
-        assert isinstance(pssm, PSSM)
         assert set(pssm.motifs) == set(self.moltype)
 
         seqs = [self.seqs[n] for n in names] if names else self.seqs
@@ -2007,7 +2049,9 @@ def _(
 ) -> dict[str, PrimitiveSeqTypes]:
     is_sequence = isinstance(next(iter(data.values()), None), new_seq.Sequence)
     return {
-        (label_to_name(k) if label_to_name else k): (str(v) if is_sequence else v)
+        (label_to_name(k) if label_to_name else k): (
+            numpy.array(v) if is_sequence else v
+        )
         for k, v in data.items()
     }
 
diff --git a/src/cogent3/core/new_genetic_code.py b/src/cogent3/core/new_genetic_code.py
index cd50100df..66dbb5d85 100644
--- a/src/cogent3/core/new_genetic_code.py
+++ b/src/cogent3/core/new_genetic_code.py
@@ -225,11 +225,13 @@ class GeneticCode:
         -------
         String containing amino acid sequence.
         """
+        # refactor: design
+        # previous implementation converted mix of ambigs and nt to '?', potentially
+        # we should change the behaviour to match this.
         if start:
             dna = dna[start:]
 
-        diff = len(dna) % 3
-        if diff != 0:
+        if diff := len(dna) % 3:
             dna = dna[:-diff]
 
         # convert to indices and then bytes
diff --git a/src/cogent3/core/new_sequence.py b/src/cogent3/core/new_sequence.py
index 8b14f67aa..dfa4d0ad4 100644
--- a/src/cogent3/core/new_sequence.py
+++ b/src/cogent3/core/new_sequence.py
@@ -1444,11 +1444,9 @@ class Sequence:
         -------
         Returns a list of Feature instances.
         """
-        try:
+        with contextlib.suppress(ValueError):
+            # assume ValueError is due to already being a regex
             pattern = self.moltype.to_regex(seq=pattern)
-        except ValueError:
-            # assume already a regex
-            pass
 
         pos = [m.span() for m in re.finditer(pattern, str(self))]
         if not pos:
@@ -1746,7 +1744,7 @@ class NucleicAcidSequenceMixin:
 
     def get_translation(
         self,
-        gc: OptGeneticCodeType = None,
+        gc: OptGeneticCodeType = 1,
         incomplete_ok: bool = False,
         include_stop: bool = False,
         trim_stop: bool = True,
@@ -1759,8 +1757,9 @@ class NucleicAcidSequenceMixin:
             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
+            codons that are mixes of nucleotide and gaps converted to '-'.
+            codons containing ambiguous nucleotides are translated as 'X'.
+            raises a AlphabetError if False
         include_stop
             allows stop codons in translation
         trim_stop
@@ -1774,59 +1773,32 @@ class NucleicAcidSequenceMixin:
         ------
         AlphabetError if include_stop is False and a stop codon occurs
         """
-        # todo gavin the genetic code should have the moltype for protein
-        #  with stop. The genetic code should also do the translation off
-        #  raw data, either an array of ints or a string. So this method
-        #  should only deal with trimming terminal stops, modulo 3 etc...
+
         from cogent3.core import new_genetic_code, new_moltype
 
         protein = new_moltype.get_moltype(
             "protein_with_stop" if include_stop else "protein"
         )
         gc = new_genetic_code.get_code(gc)
-        codon_alphabet = gc.get_alphabet(include_stop=include_stop).with_gap_motif()
-        moltype = self.moltype
-        # translate the codons
-        translation = []
-        if include_stop or not trim_stop:
-            # we just deal with sequence as is
-            seq = str(self)
+
+        if trim_stop:
+            seq = self.trim_stop_codon(gc=gc, strict=not incomplete_ok)
         else:
-            seq = str(self.trim_stop_codon(gc=gc, strict=not incomplete_ok))
+            seq = self
 
-        for posn in range(0, len(seq) - 2, 3):
-            orig_codon = str(seq[posn : posn + 3])
-            try:
-                resolved = moltype.resolve_ambiguity(
-                    orig_codon, alphabet=codon_alphabet
-                )
-            except new_alphabet.AlphabetError:
-                if not incomplete_ok or "-" not in orig_codon:
-                    raise new_alphabet.AlphabetError(
-                        f"unresolvable codon {orig_codon!r} in {self.name}"
-                    )
-                resolved = (orig_codon,)
-            trans = []
-            for codon in resolved:
-                if codon == "---":
-                    aa = "-"
-                elif "-" in codon:
-                    aa = "?"
-                    if not incomplete_ok:
-                        raise new_alphabet.AlphabetError(
-                            f"incomplete codon {codon} in {self.name}"
-                        )
-                else:
-                    aa = gc[codon]
-                    if aa == "*" and not include_stop:
-                        continue
-                trans.append(aa)
-            if not trans:
-                raise new_alphabet.AlphabetError(orig_codon)
-            aa = protein.what_ambiguity(trans)
-            translation.append(aa)
-
-        return protein.make_seq(seq="".join(translation), name=self.name)
+        # since we are realising the view, reverse complementing will be
+        # dealt with, so rc=False
+        pep = gc.translate(array(seq), rc=False)
+
+        if not include_stop and "*" in pep:
+            raise new_alphabet.AlphabetError("stop codon in translation")
+
+        if not incomplete_ok and ("-" in pep or "X" in pep):
+            raise new_alphabet.AlphabetError(
+                "Incomplete codon in translation, set incomplete_ok=True to "
+                "allow translation"
+            )
+        return protein.make_seq(seq=pep, name=self.name)
 
     def to_rna(self):
         """Returns copy of self as RNA."""
@@ -2450,7 +2422,7 @@ class SeqView(SeqViewABC, SliceRecordABC):
         return self.from_rich_dict(self.to_rich_dict())
 
 
-class DnaSequence(Sequence, NucleicAcidSequenceMixin):
+class DnaSequence(NucleicAcidSequenceMixin, Sequence):
     """Holds the standard DNA sequence."""
 
     # constructed by DNA moltype
@@ -2460,7 +2432,7 @@ class DnaSequence(Sequence, NucleicAcidSequenceMixin):
         return seq.replace("u", "t").replace("U", "T")
 
 
-class RnaSequence(Sequence, NucleicAcidSequenceMixin):
+class RnaSequence(NucleicAcidSequenceMixin, Sequence):
     """Holds the standard RNA sequence."""
 
     def _seq_filter(self, seq):
diff --git a/src/cogent3/core/profile.py b/src/cogent3/core/profile.py
index 011156392..8b2393c89 100644
--- a/src/cogent3/core/profile.py
+++ b/src/cogent3/core/profile.py
@@ -518,7 +518,7 @@ class PSSM(_MotifNumberArray):
         num_motifs = len(self.motifs)
         scores = []
 
-        for i in range(0, indexed.shape[0] - self.shape[0] + 1):
+        for i in range(indexed.shape[0] - self.shape[0] + 1):
             segment = indexed[i : i + self.shape[0]]
             un_ambig = segment < num_motifs
             if not un_ambig.all():
@@ -529,3 +529,35 @@ class PSSM(_MotifNumberArray):
             score = pssm[self._indices[: pssm.shape[0]], segment].sum()
             scores.append(score)
         return scores
+
+
+def load_pssm(path: str, background: numpy.ndarray = None, pseudocount: int = 0):
+    """loads a PSSM from a file
+
+    Parameters
+    ----------
+    path
+        path to either a jaspar or cisbp matrix (path must end have a suffix
+        matching the format).
+    background
+        background frequencies distribution
+    pseudocount
+        adjustment for zero in matrix
+
+    """
+    from cogent3.parse import cisbp, jaspar
+
+    is_cisbp = path.endswith("cisbp")
+    is_jaspar = path.endswith("jaspar")
+    if not (is_cisbp or is_jaspar):
+        raise NotImplementedError(f"Unknown format {path.split('.')[-1]}")
+
+    if is_cisbp:
+        pfp = cisbp.read(path)
+        pssm = pfp.to_pssm(background=background)
+    else:
+        _, pwm = jaspar.read(path)
+        pssm = pwm.to_pssm(background=background, pseudocount=pseudocount)
+
+    assert isinstance(pssm, PSSM)
+    return pssm
diff --git a/src/cogent3/format/fasta.py b/src/cogent3/format/fasta.py
index 07850c373..095596bcc 100644
--- a/src/cogent3/format/fasta.py
+++ b/src/cogent3/format/fasta.py
@@ -22,7 +22,7 @@ def seqs_to_fasta(
     Parameters
     ----------
     seqs
-        seq_name tp sequence mapping
+        seq_name to sequence mapping
     block_size
         the sequence length to write to each line,
         by default 60
@@ -33,7 +33,7 @@ def seqs_to_fasta(
 
     Returns
     -------
-    The alignment in the Fasta format.
+    The sequences in the Fasta format.
     """
     order = order or seqs
     result = []


Test patch:
diff --git a/tests/test_core/test_new_alignment.py b/tests/test_core/test_new_alignment.py
index 4f51afdf4..56524d280 100644
--- a/tests/test_core/test_new_alignment.py
+++ b/tests/test_core/test_new_alignment.py
@@ -1040,9 +1040,6 @@ def test_sequence_collection_has_terminal_stop_strict():
         seq_coll.has_terminal_stop(gc=1, strict=True)
 
 
-@pytest.mark.xfail(
-    reason="todo: relies on Sequence.get_translation which requires a codon alphabet class'"
-)
 def test_sequence_collection_get_translation_trim_stop():
     data = {"seq1": "GATTCCTAG", "seq2": "GATTCCTCC"}
     seq_coll = new_aln.make_unaligned_seqs(data, moltype="dna")
@@ -1051,44 +1048,48 @@ def test_sequence_collection_get_translation_trim_stop():
     assert got.to_dict() == expect
 
 
-@pytest.mark.xfail(
-    reason="todo: relies on Sequence.get_translation which requires a codon alphabet class'"
-)
 def test_sequence_collection_get_translation_raises():
     """should raise error if self.moltype is not a nucleic acid"""
     data = {"seq1": "PAR", "seq2": "PQR"}
     seq_coll = new_aln.make_unaligned_seqs(data, moltype="protein")
-    with pytest.raises(TypeError):
+    with pytest.raises(new_alpha.AlphabetError):
         _ = seq_coll.get_translation(trim_stop=True)
 
 
-@pytest.mark.xfail(
-    reason="todo: relies on Sequence.get_translation which requires a codon alphabet class'"
-)
 @pytest.mark.parametrize(
-    "seqs", ({"seq1": "GATTTT", "seq2": "GATC??"}, {"seq1": "GAT---", "seq2": "?GATCT"})
+    "seqs",
+    (
+        {"seq1": "GATTTT", "seq2": "GATC??"},
+        {"seq1": "GAT---", "seq2": "GATCTT"},
+        {"seq1": "GAT-T-", "seq2": "GATCTT"},
+        {"seq1": "GATTTT", "seq2": "?GATCT"},
+    ),
 )
-def test_sequence_collection_get_translation2(seqs):
+def test_sequence_collection_get_translation(seqs):
     """SequenceCollection.get_translation translates each seq"""
     seq_coll = new_aln.make_unaligned_seqs(seqs, moltype="dna")
-    got = seq_coll.get_translation()
-    assert len(got) == 2
+    got = seq_coll.get_translation(incomplete_ok=True)
+    assert got.num_seqs == 2
     assert got.moltype == new_moltype.PROTEIN
 
 
-@pytest.mark.xfail(
-    reason="todo: relies on Sequence.get_translation which requires a codon alphabet class'"
-)
 def test_sequence_collection_get_translation_with_stop():
-    data = {"seq1": "GATTAG", "seq2": "?GATCT"}
+    data = {"seq1": "?GATCT", "seq2": "GATTAG"}
     seq_coll = new_aln.make_unaligned_seqs(data, moltype="dna")
-    got = seq_coll.get_translation(include_stop=True)
-    assert got.to_dict() == {"seq1": "D*", "seq2": "XS"}
+    got = seq_coll.get_translation(
+        incomplete_ok=True, include_stop=True, trim_stop=False
+    )
+    assert got.to_dict() == {"seq1": "XS", "seq2": "D*"}
+    assert got.moltype == new_moltype.PROTEIN_WITH_STOP
+
+
+def test_sequence_collection_get_translation_non_div_3():
+    data = {"seq1": "?GATCTA", "seq2": "GATTAGGG"}
+    seq_coll = new_aln.make_unaligned_seqs(data, moltype="dna")
+    got = seq_coll.get_translation(incomplete_ok=True, include_stop=True)
+    assert got.to_dict() == {"seq1": "XS", "seq2": "D*"}
 
 
-@pytest.mark.xfail(
-    reason="todo: relies on Sequence.get_translation which requires a codon alphabet class'"
-)
 @pytest.mark.parametrize(
     "data", ({"seq1": "GATTTT", "seq2": "GATC??"}, {"seq1": "GAT---", "seq2": "?GATCT"})
 )
@@ -1100,32 +1101,29 @@ def test_sequence_collection_get_translation_error(data):
         seq_coll.get_translation()
 
 
-@pytest.mark.xfail(
-    reason="todo: relies on Sequence.get_translation which requires a codon alphabet class'"
-)
 @pytest.mark.parametrize(
     "data",
     (
+        {"seq1": "GATTTT", "seq2": "GATCTT"},
+        {"seq1": "GAT---", "seq2": "GGATCT"},
+        {"seq1": "GATTTT", "seq2": "?GATCT"},
         {"seq1": "GATTTT", "seq2": "GATC??"},
-        {"seq1": "GAT---", "seq2": "?GATCT"},
+        {"seq1": "GAT-T-", "seq2": "GATCTT"},
     ),
 )
 def test_sequence_collection_get_translation_info(data):
     """SequenceCollection.get_translation preserves info attribute"""
-    seq_coll = new_aln.make_unaligned_seqs(data, moltype="dna")
-    got = seq_coll.get_translation()
+    seq_coll = new_aln.make_unaligned_seqs(data, moltype="dna", info={"key": "value"})
+    got = seq_coll.get_translation(incomplete_ok=True)
     assert got.info["key"] == "value"
 
 
-@pytest.mark.xfail(
-    reason="todo: relies on Sequence.get_translation which requires a codon alphabet class'"
-)
 def test_sequence_collection_get_translation_incomplete():
     """get translation works on incomplete codons"""
     data = {"seq1": "GATN--", "seq2": "?GATCT"}
     seq_coll = new_aln.make_unaligned_seqs(data, moltype="dna")
     got = seq_coll.get_translation(incomplete_ok=True)
-    assert got.to_dict() == {"seq1": "D?", "seq2": "XS"}
+    assert got.to_dict() == {"seq1": "DX", "seq2": "XS"}
     with pytest.raises(new_alpha.AlphabetError):
         _ = seq_coll.get_translation(incomplete_ok=False)
 
@@ -1949,7 +1947,6 @@ def test_sequence_collection_strand_symmetry():
     assert numpy.allclose(result["seq2"].observed.array, [[3, 0], [2, 1]])
 
 
-@pytest.mark.xfail(reason="todo: kath, decide on how to handle rename_seqs")
 def test_sequence_collection_rename_seqs():
     """successfully rename sequences"""
     data = {"seq1": "ACGTACGTA", "seq2": "ACCGAA---", "seq3": "ACGTACGTT"}
@@ -1957,8 +1954,6 @@ def test_sequence_collection_rename_seqs():
     new = seqs.rename_seqs(lambda x: x.upper())
     expect = {n.upper() for n in data}
     assert set(new.names) == expect
-    # renamed contains the name map as info attribute
-    assert new.info.name_map == {k.upper(): k for k in data}
 
 
 def test_sequence_collection_apply_pssm():
diff --git a/tests/test_core/test_new_sequence.py b/tests/test_core/test_new_sequence.py
index 280481a9e..f63547957 100644
--- a/tests/test_core/test_new_sequence.py
+++ b/tests/test_core/test_new_sequence.py
@@ -768,3 +768,31 @@ def test_sequence_str_bytes_array():
     assert numpy.array_equal(
         numpy.array(seq), new_moltype.DNA.alphabet.to_indices(data)
     )
+
+
+@pytest.mark.xfail(reason="remove when Sequence.__str__ complements sequence")
+@pytest.mark.parametrize("seq,rc", (("ATGTTT", False), ("AAACAT", True)))
+def test_translation(seq, rc):
+    seq = new_moltype.DNA.make_seq(seq=seq)
+    if rc:
+        seq = seq.rc()
+    get_str = str(seq)
+    assert get_str == "ATGTTT"
+    aa = seq.get_translation()
+    assert str(aa) == "MF"
+
+
+def test_get_translation_include_stop():
+    s = new_moltype.DNA.make_seq(seq="ATTTAACTT", name="s1")
+    aa = s.get_translation(include_stop=True)
+    assert str(aa) == "I*L"
+
+
+def test_get_translation_trim_stop():
+    s = new_moltype.DNA.make_seq(seq="ATTTCCTGA", name="s1")
+    aa = s.get_translation(trim_stop=True)
+    assert str(aa) == "IS"
+    # no effect on internal stops
+    s = new_moltype.DNA.make_seq(seq="ATTTAACTT", name="s1")
+    aa = s.get_translation(include_stop=True, trim_stop=True)
+    assert str(aa) == "I*L"
diff --git a/tests/test_core/test_sequence.py b/tests/test_core/test_sequence.py
index b44ce362f..f8f8d6069 100644
--- a/tests/test_core/test_sequence.py
+++ b/tests/test_core/test_sequence.py
@@ -1250,7 +1250,7 @@ class ModelSequenceTests(SequenceTests):
 
 
 @pytest.mark.parametrize("seq,rc", (("ATGTTT", False), ("AAACAT", True)))
-def test_translation(seq, rc):
+def test_translation(seq, rc):  # ported
     seq = DNA.make_seq(seq)
     if rc:
         seq = seq.rc()
@@ -1259,13 +1259,13 @@ def test_translation(seq, rc):
     assert str(aa) == "MF"
 
 
-def test_get_translation_include_stop():
+def test_get_translation_include_stop():  # ported
     s = DNA.make_seq("ATTTAACTT", name="s1")
     aa = s.get_translation(include_stop=True)
     assert str(aa) == "I*L"
 
 
-def test_get_translation_trim_stop():
+def test_get_translation_trim_stop():  # ported
     s = DNA.make_seq("ATTTCCTGA", name="s1")
     aa = s.get_translation(trim_stop=True)
     assert str(aa) == "IS"