Repository: cogent3/cogent3

Issue:
ENH: add a SequenceCollection.count_ambiguous_per_seq method
This is really valuable for identifying low quality sequences. It should also be added to the `drop_bad_seqs()` method.

This should be implemented with `cogent3.core.new_sequence.Sequence.count_ambiguous()` and `cogent3.core.new_alignment.SequenceCollection.count_ambiguous_per_seq()`

Discussion:

Solution patch:
diff --git a/src/cogent3/__init__.py b/src/cogent3/__init__.py
index d55f3ec7e..d68965a42 100644
--- a/src/cogent3/__init__.py
+++ b/src/cogent3/__init__.py
@@ -30,9 +30,9 @@ from cogent3.core.moltype import (
     get_moltype,  # noqa
 )
 from cogent3.core.tree import PhyloNode, TreeBuilder, TreeError, TreeNode
-from cogent3.evolve.fast_distance import (  # noqa
-    available_distances,
-    get_distance_calculator,
+from cogent3.evolve.fast_distance import (
+    available_distances,  # noqa
+    get_distance_calculator,  # noqa
 )
 from cogent3.evolve.models import available_models, get_model  # noqa
 from cogent3.parse.cogent3_json import load_from_json
@@ -40,6 +40,7 @@ from cogent3.parse.newick import parse_string as newick_parse_string
 from cogent3.parse.sequence import get_parser, is_genbank
 from cogent3.parse.table import load_delimited
 from cogent3.parse.tree_xml import parse_string as tree_xml_parse_string
+from cogent3.util import warning as c3warn
 from cogent3.util.io import get_format_suffixes, open_
 from cogent3.util.progress_display import display_wrap
 from cogent3.util.table import Table as _Table
@@ -108,7 +109,10 @@ def make_seq(
     else:
         moltype = get_moltype(moltype)
     seq = moltype.make_seq(
-        seq=seq, name=name, annotation_offset=annotation_offset, **kw
+        seq=seq,
+        name=name,
+        annotation_offset=annotation_offset,
+        **kw,
     )
     if annotation_db:
         seq.annotation_db = annotation_db
@@ -116,7 +120,13 @@ def make_seq(
 
 
 def _make_seq_container(
-    klass, data, moltype=None, label_to_name=None, info=None, source=None, **kw
+    klass,
+    data,
+    moltype=None,
+    label_to_name=None,
+    info=None,
+    source=None,
+    **kw,
 ):
     """utility function for creating the different sequence collection/alignment instances"""
     if moltype is not None:
@@ -131,12 +141,22 @@ def _make_seq_container(
     info["source"] = str(source)
 
     return klass(
-        data=data, moltype=moltype, label_to_name=label_to_name, info=info, **kw
+        data=data,
+        moltype=moltype,
+        label_to_name=label_to_name,
+        info=info,
+        **kw,
     )
 
 
 def make_unaligned_seqs(
-    data, moltype=None, label_to_name=None, info=None, source=None, new_type=False, **kw
+    data,
+    moltype=None,
+    label_to_name=None,
+    info=None,
+    source=None,
+    new_type=False,
+    **kw,
 ):
     """Initialize an unaligned collection of sequences.
 
@@ -286,6 +306,7 @@ def _load_files_to_unaligned_seqs(
 
 def _load_seqs(file_format, filename, fmt, parser_kw):
     """utility function for loading sequences"""
+    filename = pathlib.Path(filename).expanduser()
     fmt = fmt or file_format
     if not fmt:
         msg = "could not determined file format, set using the format argument"
@@ -299,7 +320,9 @@ T = Optional[_anno_db.SupportsFeatures]
 
 
 def _load_genbank_seq(
-    filename: os.PathLike, parser_kw: dict, just_seq: bool = False
+    filename: os.PathLike,
+    parser_kw: dict,
+    just_seq: bool = False,
 ) -> tuple[str, str, T]:
     """utility function for loading sequences"""
     from cogent3.parse.genbank import iter_genbank_records
@@ -313,7 +336,8 @@ def _load_genbank_seq(
         None
         if just_seq
         else _anno_db.GenbankAnnotationDb(
-            data=features.pop("features", None), seqid=name
+            data=features.pop("features", None),
+            seqid=name,
         )
     )
     return name, seq, db
@@ -378,7 +402,9 @@ def load_seq(
 
     if is_genbank(format or file_format):
         name, seq, db = _load_genbank_seq(
-            filename, parser_kw or {}, just_seq=annotation_path is not None
+            filename,
+            parser_kw or {},
+            just_seq=annotation_path is not None,
         )
     else:
         db = None
@@ -487,6 +513,7 @@ def load_aligned_seqs(
     label_to_name=None,
     parser_kw=None,
     info=None,
+    new_type: bool = False,
     **kw,
 ):
     """
@@ -506,6 +533,11 @@ def load_aligned_seqs(
         function for converting original name into another name.
     parser_kw : dict
         optional arguments for the parser
+    new_type
+        if True, the returned Alignment will be of the new type,
+        (cogent3.core.new_alignment.Alignment). The default will be
+        changed to True in 2024.12. Support for the old style will be removed
+        as of 2025.6.
     kw
         passed to make_aligned_seqs
 
@@ -525,6 +557,7 @@ def load_aligned_seqs(
         moltype=moltype,
         source=filename,
         info=info,
+        new_type=new_type,
         **kw,
     )
 
@@ -675,7 +708,7 @@ def load_table(
     """
     if not any(isinstance(filename, t) for t in (str, pathlib.PurePath)):
         raise TypeError(
-            "filename must be string or Path, perhaps you want make_table()"
+            "filename must be string or Path, perhaps you want make_table()",
         )
 
     sep = sep or kwargs.pop("delimiter", None)
@@ -703,7 +736,10 @@ def load_table(
             sep = sep or "\t"
 
         header, rows, loaded_title, legend = load_delimited(
-            filename, sep=sep, limit=limit, **kwargs
+            filename,
+            sep=sep,
+            limit=limit,
+            **kwargs,
         )
         if skip_inconsistent:
             num_fields = len(header)
@@ -735,12 +771,16 @@ def load_table(
     )
 
 
+@c3warn.deprecated_args(
+    version="2024.12",
+    reason="argument has no effect",
+    discontinued="name_nodes",
+)
 def make_tree(
     treestring=None,
     tip_names=None,
     format=None,
     underscore_unmunge=False,
-    name_nodes=False,
 ):
     """Initialises a tree.
 
@@ -756,8 +796,6 @@ def make_tree(
     underscore_unmunge : bool
         replace underscores with spaces in all names read, i.e. "sp_name"
         becomes "sp name"
-    name_nodes: bool
-        whether to name unnamed nodes
 
     Notes
     -----
@@ -787,15 +825,13 @@ def make_tree(
     if not tree.name_loaded:
         tree.name = "root"
 
-    # ensure all nodes have names if name_nodes is True
-    if name_nodes:
-        tree.name_unnamed_nodes()
-
     return tree
 
 
 def load_tree(
-    filename: Union[str, pathlib.Path], format=None, underscore_unmunge=False
+    filename: Union[str, pathlib.Path],
+    format=None,
+    underscore_unmunge=False,
 ):
     """Constructor for tree.
 
diff --git a/src/cogent3/app/align.py b/src/cogent3/app/align.py
index 35d379900..0ba0df9e7 100644
--- a/src/cogent3/app/align.py
+++ b/src/cogent3/app/align.py
@@ -828,7 +828,7 @@ def cogent3_score(aln: AlignedSeqsType) -> float:
     if aln.num_seqs == 1 or len(aln) == 0:
         msg = "zero length" if len(aln) == 0 else "single sequence"
         return NotCompleted(
-            "FAL",
+            "FAIL",
             "cogent3_score",
             f"cannot compute alignment quality because {msg}",
             source=aln.info,
@@ -838,7 +838,7 @@ def cogent3_score(aln: AlignedSeqsType) -> float:
     return align_params.get(
         "lnL",
         NotCompleted(
-            "FAL", "cogent3_score", "no alignment quality score", source=aln.info
+            "FAIL", "cogent3_score", "no alignment quality score", source=aln.info
         ),
     )
 
diff --git a/src/cogent3/app/sample.py b/src/cogent3/app/sample.py
index 5ba807e4f..bd7a81869 100644
--- a/src/cogent3/app/sample.py
+++ b/src/cogent3/app/sample.py
@@ -16,6 +16,7 @@ from .typing import AlignedSeqsType, SeqsCollectionType, SerialisableType
 # set.
 
 MolTypes = Union[str, MolType]
+OptInt = Optional[int]
 
 
 def intersection(groups):
@@ -892,6 +893,7 @@ class omit_bad_seqs:
         self,
         quantile: Optional[float] = None,
         gap_fraction: int = 1,
+        ambig_fraction: OptInt = None,  # refactor: set default to 1 when support for old style aln is dropped
         moltype: MolTypes = "dna",
     ):
         """
@@ -905,6 +907,10 @@ class omit_bad_seqs:
         gap_fraction
             sequences whose proportion of gaps is >= this value are excluded, the
             default excludes sequences that are just gaps.
+        ambig_fraction
+            sequences whose proportion of ambiguous characters is >= this value
+            are excluded. Only supported for new style alignments. By default,
+            not applied.
         moltype
             molecular type, can be string or instance
 
@@ -954,8 +960,10 @@ class omit_bad_seqs:
         assert (
             moltype.label.lower() in "dna rna protein protein_with_stop"
         ), "moltype must be one of DNA, RNA or PROTEIN"
+        # refactor: design, this should raise a MolTypeError
         self._quantile = quantile
         self._gap_fraction = gap_fraction
+        self._ambig_fraction = ambig_fraction
         self._moltype = moltype
 
     T = Union[SerialisableType, AlignedSeqsType]
@@ -967,6 +975,24 @@ class omit_bad_seqs:
         gaps_per_seq = aln.count_gaps_per_seq()
         length = len(aln)
         keep = [n for n, c in gaps_per_seq.items() if c / length < self._gap_fraction]
+
+        from cogent3.core import new_alignment
+
+        if self._ambig_fraction is not None:
+            if isinstance(aln, new_alignment.Alignment):
+                ambigs_per_seq = aln.count_ambiguous_per_seq()
+                keep = [
+                    n for n in keep if ambigs_per_seq[n] / length < self._ambig_fraction
+                ]
+            else:
+                msg = (
+                    "ambig_fraction cannot be applied to old style alignments, set "
+                    "new_type=True in make_aligned_seqs to create an Alignment which "
+                    "supports this feature"
+                )
+
+                return NotCompleted("FAIL", self, msg, aln.info.source)
+
         result = aln.take_seqs(keep)
         if self._quantile is not None:
             result = result.omit_bad_seqs(quantile=self._quantile)
diff --git a/src/cogent3/core/new_alignment.py b/src/cogent3/core/new_alignment.py
index 4df43ebc1..7cf562157 100644
--- a/src/cogent3/core/new_alignment.py
+++ b/src/cogent3/core/new_alignment.py
@@ -43,7 +43,7 @@ from cogent3.format.phylip import alignment_to_phylip
 from cogent3.maths.stats.number import CategoryCounter
 from cogent3.util import progress_display as UI
 from cogent3.util import warning as c3warn
-from cogent3.util.deserialise import deserialise_object, register_deserialiser
+from cogent3.util.deserialise import register_deserialiser
 from cogent3.util.dict_array import DictArray, DictArrayTemplate
 from cogent3.util.io import atomic_write, get_format_suffixes
 from cogent3.util.misc import (
@@ -1592,6 +1592,17 @@ class SequenceCollection:
         )
         return per_seq.motif_totals()
 
+    def count_ambiguous_per_seq(self) -> DictArray:
+        """Counts of ambiguous characters per sequence."""
+
+        darr = DictArrayTemplate(self.names)
+        counts = numpy.array(
+            [self.seqs[name].count_ambiguous() for name in self.names],
+            dtype=numpy.uint32,
+        )
+
+        return darr.wrap(counts)
+
     def get_motif_probs(
         self,
         alphabet: new_alphabet.AlphabetABC = None,
@@ -4496,7 +4507,7 @@ class Alignment(SequenceCollection):
         unique: bool = False,
         include_ambiguity: bool = True,
         drawable: bool = False,
-    ):
+    ) -> DictArray:
         """return counts of gaps per sequence as a DictArray
 
         Parameters
@@ -4551,6 +4562,15 @@ class Alignment(SequenceCollection):
 
         return result
 
+    def count_ambiguous_per_seq(self) -> DictArray:
+        """Return the counts of ambiguous characters per sequence as a DictArray."""
+
+        gap_index = self.moltype.most_degen_alphabet().gap_index
+        ambigs_pos = self.array_seqs > gap_index
+        ambigs = ambigs_pos.sum(axis=1)
+
+        return DictArray.from_array_names(ambigs, self.names)
+
     def variable_positions(
         self,
         include_gap_motif: bool = True,
@@ -6166,7 +6186,13 @@ class Alignment(SequenceCollection):
             alphabet=self.moltype.most_degen_alphabet(),
         )
         kwargs["seqs_data"] = new_seqs_data
-        kwargs["slice_record"] = None
+        # we have modified the underlying data with the plus attributes only,
+        # so, if reversed, we need to retain that information in the slice_record
+        kwargs["slice_record"] = (
+            new_sequence.SliceRecord(parent_len=new_seqs_data.align_len, step=-1)
+            if self._slice_record.is_reversed
+            else None
+        )
         return self.__class__(
             **kwargs,
         )
diff --git a/src/cogent3/core/new_sequence.py b/src/cogent3/core/new_sequence.py
index b859978f9..326aeae0a 100644
--- a/src/cogent3/core/new_sequence.py
+++ b/src/cogent3/core/new_sequence.py
@@ -363,6 +363,12 @@ class Sequence:
 
         return counts
 
+    def count_ambiguous(self) -> int:
+        """Returns the number of ambiguous characters in the sequence."""
+        data = numpy.array(self)
+        gap_index = self.moltype.most_degen_alphabet().gap_index
+        return numpy.sum(data > gap_index)
+
     def __lt__(self, other):
         """compares based on the sequence string."""
         return str(self) < str(other)
@@ -2804,11 +2810,12 @@ class SeqView(SeqViewABC):
                 slice_record=self.slice_record.copy(),
                 parent_len=self.parent_len,
             )
-        # the plus start and stop are applied, but the step is not applied
-        sr = SliceRecord(parent_len=len(self), step=self.slice_record.step)
+        # we slice parent with the plus attributes only, so, if reversed, we need to
+        # retain that information in the slice_record
+        sr = SliceRecord(parent_len=len(self), step=-1) if self.is_reversed else None
         return self.__class__(
             parent=self.parent[
-                self.slice_record.plus_start : self.slice_record.plus_stop
+                self.slice_record.plus_start : self.slice_record.plus_stop : self.slice_record.plus_step
             ],
             parent_len=len(self),
             seqid=self.seqid,
diff --git a/src/cogent3/core/tree.py b/src/cogent3/core/tree.py
index c3c88fb90..bfd2742e5 100644
--- a/src/cogent3/core/tree.py
+++ b/src/cogent3/core/tree.py
@@ -1695,6 +1695,11 @@ class TreeNode:
         """return dict of {<child name>: <parent name>, ...}"""
         return {e.name: e.parent.name for e in self.postorder(include_self=False)}
 
+    @staticmethod
+    def parse_token(token: str) -> tuple[str | None, dict]:
+        # TreeNode token contains no attributes
+        return token, {}
+
 
 class PhyloNode(TreeNode):
     def __init__(self, *args, **kwargs):
@@ -1704,13 +1709,6 @@ class PhyloNode(TreeNode):
         kwargs["params"] = params
         super().__init__(*args, **kwargs)
 
-        if self.children:
-            # self is an internal tree node
-            name, support = split_name_and_support(self.name)
-            self.name = name
-            if support is not None:
-                self.params["support"] = support
-
     @property
     def length(self) -> Union[float, None]:
         return self.params.get("length", None)
@@ -2192,6 +2190,11 @@ class PhyloNode(TreeNode):
                     tip_b[0] += child_b.length or 0.0
                 n.MaxDistTips = [tip_a, tip_b]
 
+    @staticmethod
+    def parse_token(token: str) -> tuple[str | None, dict]:
+        name, attrs = split_name_and_support(token)
+        return (name, {"support": attrs}) if attrs else (name, {})
+
 
 def split_name_and_support(name_field: str | None) -> tuple[str | None, float | None]:
     """Handle cases in the Newick format where an internal node name field
@@ -2270,6 +2273,11 @@ class TreeBuilder(object):
         """Callback for newick parser"""
         if children is None:
             children = []
+        # split name and support for internal nodes
+        elif children != []:
+            name, pars = self.TreeNodeClass.parse_token(name)
+            params |= pars
+
         node = self.TreeNodeClass(
             children=list(children),
             name=self._unique_name(name),


Test patch:
diff --git a/tests/test_app/test_sample.py b/tests/test_app/test_sample.py
index eb1ca31dc..f0768b699 100644
--- a/tests/test_app/test_sample.py
+++ b/tests/test_app/test_sample.py
@@ -293,40 +293,6 @@ class TranslateTests(TestCase):
         for name, seq in got.to_dict().items():
             self.assertIn(seq, expect[name])
 
-    def test_omit_bad_seqs(self):
-        """correctly omit bad sequences from an alignment"""
-        data = {
-            "s1": "---ACC---TT-",
-            "s2": "---ACC---TT-",
-            "s3": "---ACC---TT-",
-            "s4": "--AACCG-GTT-",
-            "s5": "--AACCGGGTTT",
-            "s6": "AGAACCGGGTT-",
-            "s7": "------------",
-        }
-        aln = make_aligned_seqs(data=data, moltype=DNA)
-        # default just eliminates strict gap sequences
-        dropbad = sample.omit_bad_seqs()
-        got = dropbad(aln)
-        expect = data.copy()
-        del expect["s7"]
-        self.assertEqual(got.to_dict(), expect)
-        # providing a more stringent gap_frac
-        dropbad = sample.omit_bad_seqs(gap_fraction=0.5)
-        got = dropbad(aln)
-        expect = data.copy()
-        for n in ("s1", "s2", "s3", "s7"):
-            del expect[n]
-        self.assertEqual(got.to_dict(), expect)
-
-        # setting quantile drops additional sequences
-        dropbad = sample.omit_bad_seqs(quantile=6 / 7)
-        got = dropbad(aln)
-        expect = data.copy()
-        for n in ("s6", "s7"):
-            del expect[n]
-        self.assertEqual(got.to_dict(), expect)
-
     def test_omit_duplicated(self):
         """correctly drop duplicated sequences"""
         # strict omit_duplicated
@@ -565,3 +531,92 @@ def test_concat_empty(data):
     ccat = sample.concat()
     got = ccat(data)
     assert isinstance(got, NotCompleted)
+
+
+@pytest.fixture
+def bad_gap_data():
+    return {
+        "s1": "---ACC---TT-",
+        "s2": "---ACC---TT-",
+        "s3": "---ACC---TT-",
+        "s4": "--AACCG-GTT-",
+        "s5": "--AACCGGGTTT",
+        "s6": "AGAACCGGGTT-",
+        "s7": "------------",
+    }
+
+
+@pytest.mark.parametrize(
+    "gap_fraction, quantile, expected_keys",
+    [
+        (
+            1,
+            None,
+            {"s1", "s2", "s3", "s4", "s5", "s6"},
+        ),  # default just eliminates strict gap sequences
+        (0.5, None, {"s4", "s5", "s6"}),  # providing a more stringent gap_frac
+        (
+            1,
+            6 / 7,
+            {"s1", "s2", "s3", "s4", "s5"},
+        ),  # setting quantile drops additional sequences
+    ],
+)
+def test_omit_bad_seqs(bad_gap_data, gap_fraction, quantile, expected_keys):
+    """correctly omit bad sequences from an alignment"""
+
+    dropbad = sample.omit_bad_seqs(gap_fraction=gap_fraction, quantile=quantile)
+    aln = make_aligned_seqs(bad_gap_data, moltype=DNA)
+    got = dropbad(aln)
+    expected = {k: bad_gap_data[k] for k in expected_keys}
+    assert got.to_dict() == expected
+
+
+@pytest.fixture
+def bad_ambig_gap_data():
+    return {
+        "s1": "---ACC---TT-",
+        "s2": "---ACC---TT-",
+        "s3": "SSSACCSSSTTS",
+        "s4": "SSAACCGSGTTS",
+        "s5": "--AACCGGGTTT",
+        "s6": "AGAACCGGGTT-",
+        "s7": "SSSSSSSSSSSS",
+    }
+
+
+def test_omit_bad_seqs_ambigs(bad_ambig_gap_data):
+    """correctly omit sequences from a new style alignment via fraction of ambiguous data"""
+    from cogent3.core import new_alignment
+
+    aln = new_alignment.make_aligned_seqs(bad_ambig_gap_data, moltype="dna")
+
+    # drop sequences with all ambiguous data
+    dropbad = sample.omit_bad_seqs(ambig_fraction=1)
+    got = dropbad(aln)
+    assert set(got.to_dict().keys()) == {"s1", "s2", "s3", "s4", "s5", "s6"}
+
+    # drop sequences with more than 50% ambiguous data
+    dropbad = sample.omit_bad_seqs(ambig_fraction=0.5)
+    got = dropbad(aln)
+    assert set(got.to_dict().keys()) == {"s1", "s2", "s4", "s5", "s6"}
+
+    # drop sequences with any ambiguous data
+    dropbad = sample.omit_bad_seqs(ambig_fraction=0.01)
+    got = dropbad(aln)
+    assert set(got.to_dict().keys()) == {"s1", "s2", "s5", "s6"}
+
+    # drop sequences with more than 50% ambiguous data and 50% gaps
+    dropbad = sample.omit_bad_seqs(gap_fraction=0.5, ambig_fraction=0.5)
+    got = dropbad(aln)
+    assert set(got.to_dict().keys()) == {"s4", "s5", "s6"}
+
+
+def test_omit_bad_seqs_ambigs_old_aln(bad_ambig_gap_data):
+    # ambig_fraction should be ignored if using old style alignment
+
+    aln = make_aligned_seqs(bad_ambig_gap_data, moltype=DNA)
+    dropbad = sample.omit_bad_seqs(gap_fraction=0.5, ambig_fraction=0.5)
+
+    got = dropbad(aln)
+    assert isinstance(got, NotCompleted)
diff --git a/tests/test_core/test_new_alignment.py b/tests/test_core/test_new_alignment.py
index e8bd69342..a5e56ba34 100644
--- a/tests/test_core/test_new_alignment.py
+++ b/tests/test_core/test_new_alignment.py
@@ -1839,6 +1839,22 @@ def test_sequence_collection_counts():
     assert all(got[k] == v for k, v in expect.items())
 
 
+@pytest.mark.parametrize(
+    "mk_cls", [new_alignment.make_unaligned_seqs, new_alignment.make_aligned_seqs]
+)
+def test_sequence_collection_count_ambiguous_per_seq(mk_cls):
+    data = {
+        "a": "AGGGT",
+        "b": "AGGN?",
+        "c": "?????",
+        "d": "-----",
+    }
+    coll = mk_cls(data, moltype="dna")
+    got = coll.count_ambiguous_per_seq()
+    expect = {"a": 0, "b": 2, "c": 5, "d": 0}
+    assert got == expect
+
+
 def test_sequence_collection_make_feature(seqs):
     data = {"seqid": "seq1", "biotype": "xyz", "name": "abc", "spans": [(1, 2)]}
     feat = seqs.make_feature(feature=data)
@@ -4999,6 +5015,14 @@ def test_alignment_copy(simple_aln):
     assert str(copied.seqs["A"]) == str(renamed.seqs["A"])
 
 
+def test_alignment_copy_rc(simple_aln):
+    rc = simple_aln.rc()
+    got = rc.copy()
+    assert got.to_dict() == rc.to_dict()
+    assert got._is_reversed == rc._is_reversed
+    assert got._slice_record is rc._slice_record
+
+
 def test_alignment_deepcopy(simple_aln):
     got = simple_aln.deepcopy()
     # all data structures should be different IDs
@@ -5021,6 +5045,14 @@ def test_alignment_deepcopy(simple_aln):
     assert str(copied.seqs["A"]) == str(renamed.seqs["A"])
 
 
+def test_alignment_deepcopy_rc(simple_aln):
+    rc = simple_aln.rc()
+    got = rc.deepcopy()
+    assert got.to_dict() == rc.to_dict()
+    assert got._is_reversed == rc._is_reversed
+    assert got._slice_record is not simple_aln._slice_record
+
+
 @pytest.mark.parametrize(
     "mk_cls",
     [
@@ -5203,3 +5235,25 @@ def test_deserialise_alignment():
     assert aln.names == ["new_seq1", "new_seq2"]
     assert str(aln.get_seq("new_seq1")) == "ATCG"
     assert str(aln.get_seq("new_seq2")) == "TAGC"
+
+
+@pytest.fixture
+def home_seqs(DATA_DIR) -> str:
+    """makes a temporary directory with file"""
+    import tempfile
+
+    HOME = pathlib.Path("~").expanduser()
+    fn = "brca1.fasta"
+    contents = (DATA_DIR / fn).read_text()
+    with tempfile.TemporaryDirectory(dir=HOME) as dn:
+        dn = pathlib.Path(dn)
+        outpath = HOME / dn.name / fn
+        outpath.expanduser().write_text(contents)
+        yield f"~/{dn.name}/{fn}"
+
+
+@pytest.mark.parametrize("mk_cls", [load_aligned_seqs, load_unaligned_seqs])
+def test_load_with_pathlib(mk_cls, home_seqs):
+    path = home_seqs
+    got = mk_cls(path, moltype="dna")
+    assert "Human" in got.names
diff --git a/tests/test_core/test_new_sequence.py b/tests/test_core/test_new_sequence.py
index 7a6883a35..db008245f 100644
--- a/tests/test_core/test_new_sequence.py
+++ b/tests/test_core/test_new_sequence.py
@@ -831,6 +831,20 @@ def test_counts():
     assert dict(got) == expect
 
 
+@pytest.mark.parametrize(
+    "data, expect",
+    [
+        ("CCTTNN", 2),
+        ("???MMM", 6),
+        ("TGCAGG", 0),
+        ("", 0),
+    ],
+)
+def test_count_ambiguous(data, expect):
+    seq = new_moltype.DNA.make_seq(seq=data)
+    assert seq.count_ambiguous() == expect
+
+
 def test_strand_symmetry():
     """correctly compute test of strand symmetry"""
     from cogent3 import get_moltype
@@ -2673,6 +2687,25 @@ def test_seqview_attrs_non_modulo(rev):
     assert sr.plus_stop == 0
 
 
+@pytest.mark.parametrize("start", (0, 1, 10, -1, -10))
+@pytest.mark.parametrize("stop", (0, 10, 8, 1, -1, -11))
+@pytest.mark.parametrize("step", (1, 2, -1, -2))
+def test_plus_attrs_slicing(start, stop, step):
+    """slicing with plus attributes then reversing should match native python slicing"""
+    data = "0123456789"
+    sr = new_sequence.SliceRecord(start=start, stop=stop, step=step, parent_len=10)
+
+    plus_start, plus_stop, plus_step = sr.plus_start, sr.plus_stop, sr.plus_step
+    got = (
+        data[plus_start:plus_stop:plus_step][::-1]
+        if step < 0
+        else data[plus_start:plus_stop:plus_step]
+    )
+    expect = data[start:stop:step]
+    print(start, stop, step, plus_start, plus_stop, plus_step)
+    assert got == expect
+
+
 @pytest.mark.parametrize("rev", (False, True))
 def test_seqview_attrs_zero_slice(rev):
     sr = new_sequence.SliceRecord(start=0, stop=0, step=1, parent_len=0)
diff --git a/tests/test_core/test_tree.py b/tests/test_core/test_tree.py
index d70e61cf6..55db57503 100644
--- a/tests/test_core/test_tree.py
+++ b/tests/test_core/test_tree.py
@@ -2369,35 +2369,29 @@ def test_split_name_and_support_invalid_support(invalid):
 def test_phylonode_support():
     tip_names = [str(i) for i in range(1, 13)]
     tree = make_tree(
-        treestring="(1,(((2,3),4)/53,(5,((6,(7,(8,9))def/25),(10,11)abc))),12);"
+        treestring="(1,(((2,3)53,4)/53,(5,((6,(7,(8,9))def/25),(10,11)abc))),12);"
     )
     assert tree.get_tip_names() == tip_names
+
+    node_names = set(tree.get_node_names())
+    # check that no node name is an empty string or None
+    assert all(node_names)
+
     # parent of 4 is node with only support value
     just_support = tree.get_node_matching_name("4").parent
     assert just_support.params["support"] == 53.0
+
+    # parent of 2 has the same support as parent of 4
+    same_support = tree.get_node_matching_name("2").parent
+    assert same_support.params["support"] == just_support.params["support"]
+
     # parent of 10 has a node name only
     just_name = tree.get_node_matching_name("10").parent
     assert just_name.name == "abc"
     assert "support" not in just_name.params
+
     # the node with name "def/25" correctly resoloved into node
     # name "def" and support 25.0
     name_and_support = tree.get_node_matching_name("def")
     assert name_and_support.name == "def"  # bit redundant given selection process
     assert name_and_support.params["support"] == 25.0
-
-
-def test_phylonode_support_name_nodes_false():
-    # test that internal node names are None with name_nodes=False
-    tree = make_tree("((1,2)5,(3,4)6);", name_nodes=False)
-    internal_nodes = [node.name for node in tree.iter_nontips()]
-    assert all(node is None for node in internal_nodes)
-
-
-def test_phylonode_support_name_nodes_true():
-    # test that all nodes have unique names with name_nodes=True
-    tree = make_tree("((1,2)5,(3,4)6);", name_nodes=True)
-    node_names = set(tree.get_node_names())
-    # check that no node name is an empty string or None
-    assert all(node_names)
-    # check that the total number of unique node names is 7
-    assert len(node_names) == 7