Repository: cogent3/cogent3
Pull request #2315 (2025-05-14 22:47:02)

Misc fixes and tests on dotplots
## Summary by Sourcery

Enhance the dotplot feature to always show the alignment path for gapless inputs, enforce stricter parameter validation with clearer errors, and update tests to reflect these behaviors while consolidating test coverage and adding a changelog entry.

Bug Fixes:
- Prevent displaying an alignment path for unaligned sequence collections dotplots
- Raise a ValueError instead of AssertionError for invalid k parameter in dotplot

Enhancements:
- Always include an alignment trace on gapless alignments
- Improve error messaging and input validation for dotplot parameters
- Switch to a dedicated numpy random generator for reproducible sequence selection

Documentation:
- Add a changelog fragment summarizing dotplot fixes and enhancements

Tests:
- Revise and extend dotplot tests to cover alignment path presence, parameter errors, and annotation behavior

Chores:
- Add type annotations to alignment application defaults


Patch:
diff --git a/changelog.d/20250515_083521_Gavin.Huttley.md b/changelog.d/20250515_083521_Gavin.Huttley.md
new file mode 100644
index 000000000..600f65aed
--- /dev/null
+++ b/changelog.d/20250515_083521_Gavin.Huttley.md
@@ -0,0 +1,44 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Contributors
+
+- A bullet item for the Contributors category.
+
+-->
+
+### Enhancements
+
+- Dotplots now show the alignment path whenever Alignment.dotplot() is invoked.
+  Previously, the path was only displayed if there gaps, which could be confusing
+  since that's behaviour of SequenceCollection.dotplot().
+
+
+### Bug fixes
+
+- Alignment paths were incorrectly displayed on dotplots when invoked from the
+  dotplot method. This has been fixed.
+
+
+<!--
+### Documentation
+
+- A bullet item for the Documentation category.
+
+-->
+<!--
+### Deprecations
+
+- A bullet item for the Deprecations category.
+
+-->
+<!--
+### Discontinued
+
+- A bullet item for the Discontinued category.
+
+-->
diff --git a/src/cogent3/app/align.py b/src/cogent3/app/align.py
index bb46b7cef..73e39cbc7 100644
--- a/src/cogent3/app/align.py
+++ b/src/cogent3/app/align.py
@@ -433,10 +433,10 @@ class progressive_align:
         gc=None,
         param_vals=None,
         guide_tree=None,
-        unique_guides=False,
-        indel_length=1e-1,
-        indel_rate=1e-10,
-        distance="pdist",
+        unique_guides: bool = False,
+        indel_length: float = 1e-1,
+        indel_rate: float = 1e-10,
+        distance: str = "pdist",
         iters: int | None = None,
         approx_dists: bool = True,
     ) -> None:
diff --git a/src/cogent3/core/new_alignment.py b/src/cogent3/core/new_alignment.py
index 91ae2ec03..97888de79 100644
--- a/src/cogent3/core/new_alignment.py
+++ b/src/cogent3/core/new_alignment.py
@@ -1539,16 +1539,18 @@ class SequenceCollection:
         from cogent3.draw.dotplot import Dotplot
         from cogent3.draw.drawable import AnnotatedDrawable
 
-        if k is not None:
-            assert 0 < k < window, "k must be smaller than window size"
+        randgen = numpy.random.default_rng()
+        if k is not None and not (0 < k < window):
+            msg = f"{k=} must be positive, < {window=} and < {threshold=}"
+            raise ValueError(msg)
 
         if len(self.names) == 1:
             name1 = name2 = self.names[0]
         elif name1 is None and name2 is None:
-            name1, name2 = list(numpy.random.choice(self.names, size=2, replace=False))
+            name1, name2 = randgen.choice(self.names, size=2, replace=False).tolist()
         elif not (name1 and name2):
             names = list({*self.names, None} ^ {name1, name2})
-            name = next(iter(numpy.random.choice(names, size=1)))
+            name = next(iter(randgen.choice(names, size=1))).item()
             name1 = name1 or name
             name2 = name2 or name
 
@@ -1565,7 +1567,7 @@ class SequenceCollection:
         dotplot = Dotplot(
             seq1,
             seq2,
-            False,
+            isinstance(self, Alignment),
             window=window,
             threshold=threshold,
             k=k,
diff --git a/src/cogent3/draw/dotplot.py b/src/cogent3/draw/dotplot.py
index 93d40f4a4..9d2cf050b 100644
--- a/src/cogent3/draw/dotplot.py
+++ b/src/cogent3/draw/dotplot.py
@@ -73,13 +73,18 @@ def _convert_input(seq, moltype):
 def get_align_coords(
     map1: location.IndelMap,
     map2: location.IndelMap,
-    aligned=False,
+    aligned: bool = False,
 ) -> MatchedSeqPaths:
     """sequence coordinates of aligned segments"""
     from cogent3.align.pycompare import segment
 
     if aligned:
         assert len(map1) == len(map2), "aligned sequences not equal length"
+        if map1.num_gaps == map2.num_gaps == 0:
+            end = len(map1) - 1
+            paths = MatchedSeqPaths("Alignment")
+            paths.append(segment(0, end), segment(0, end))
+            return paths
 
     # we reduce the maps to only gaps that are unique to each
 
@@ -120,7 +125,7 @@ class Dotplot(Drawable):
         self,
         seq1,
         seq2,
-        is_aligned,
+        is_aligned: bool,
         moltype="text",
         window=20,
         threshold=None,
@@ -162,11 +167,14 @@ class Dotplot(Drawable):
             title for the plot
         """
 
-        # we ensure sequences have gaps parsed and the calculate aspect ratio
+        # we ensure sequences have gaps parsed and then calculate aspect ratio
         moltype = seq1.moltype if hasattr(seq1, "moltype") else get_moltype(moltype)
         map1, map2, seq1, seq2 = _prep_seqs(moltype, seq1, seq2, is_aligned)
 
         len1, len2 = len(seq1), len(seq2)
+        if len1 == 0 or len2 == 0:
+            zlen = seq1.name if not len1 else seq2.name
+            raise ValueError(f"cannot display zero-length sequence {zlen!r}")
         height = width * len2 / len1
 
         if seq1.name == seq2.name is None:
@@ -184,7 +192,9 @@ class Dotplot(Drawable):
 
         self.seq1 = seq1
         self.seq2 = seq2
-        self._aligned_coords = get_align_coords(map1, map2, aligned=is_aligned)
+        self._aligned_coords = (
+            get_align_coords(map1, map2, aligned=is_aligned) if is_aligned else None
+        )
 
         self.title = title
         self._window = window


Test patch:
diff --git a/tests/test_draw/test_dotplot.py b/tests/test_draw/test_dotplot.py
index 2360db9f8..34d0f7e8f 100644
--- a/tests/test_draw/test_dotplot.py
+++ b/tests/test_draw/test_dotplot.py
@@ -77,26 +77,6 @@ class TestUtilFunctions(TestCase):
         assert len(traces) == 3
         assert traces[1].name == "- strand"
 
-    def test_align_without_gaps(self):
-        """dotplot has alignment coordinates if no gaps"""
-        aln = make_aligned_seqs(
-            data={"seq1": "ACGG", "seq2": "CGCA", "seq3": "CCG-"},
-            moltype="dna",
-        )
-        aln_plot = aln.dotplot("seq1")
-        assert aln_plot._aligned_coords is not None
-
-    def test_dotplot_seqcoll(self):
-        """dotplot sequence collection, gaps are removed"""
-        seqs = make_unaligned_seqs(
-            {"seq1": "ACGG", "seq2": "CGCA", "seq3": "CCG-"},
-            moltype="dna",
-        )
-        dp = seqs.dotplot("seq1", "seq3")
-        assert dp._aligned_coords is not None
-        assert len(dp.seq1) == 4
-        assert len(dp.seq2) == 3
-
     def test_dotplot_single(self):
         """dotplot with single sequence should not fail"""
         seqs = make_unaligned_seqs({"seq1": "CACACCACTGCAGTCGGATAGACC"}, moltype="dna")
@@ -198,3 +178,25 @@ def test_get_align_coords_common_gaps():
     path = get_align_coords(m1, m2)
     expect = [2, 4, None, 5, 5], [0, 2, None, 5, 5]
     assert path.get_coords() == expect
+
+
+def test_align_without_gaps():
+    """dotplot has alignment coordinates if no gaps"""
+    aln = make_aligned_seqs(
+        data={"seq1": "ACGG", "seq2": "ACGG"}, moltype="dna", new_type=True
+    )
+    dp = aln.dotplot(window=2, threshold=1)
+    # trigger building the figure
+    dp.figure
+    assert any(trace["name"] == "Alignment" for trace in dp.traces)
+
+
+def test_dotplot_seqcoll():
+    """dotplot sequence collection, gaps are removed"""
+    seqs = make_unaligned_seqs(
+        {"seq1": "ACGG", "seq2": "CGCA", "seq3": "CCG-"}, moltype="dna", new_type=True
+    )
+    dp = seqs.dotplot("seq1", "seq3")
+    assert all(trace["name"] != "Alignment" for trace in dp.traces)
+    assert len(dp.seq1) == 4
+    assert len(dp.seq2) == 3
diff --git a/tests/test_draw/test_new_draw_integration.py b/tests/test_draw/test_new_draw_integration.py
index 3e7505da7..b3c41dbe7 100644
--- a/tests/test_draw/test_new_draw_integration.py
+++ b/tests/test_draw/test_new_draw_integration.py
@@ -126,7 +126,7 @@ def test_dotplot_base_cases(dotplot_seqs, aligned):
     assert str(plot.seq1) == str(plot.seq2)
 
     # k larger than window should raise an error
-    with pytest.raises(AssertionError):
+    with pytest.raises(ValueError):
         seqs.dotplot(window=5, k=11)
 
     # names not in the collection should raise an error
@@ -150,6 +150,19 @@ def test_dotplot_aligned_path():
     assert len(trace["x"])
 
 
+def test_dotplot_unaligned_no_aligned_path():
+    dotplot_seqs = {
+        "Human": "TTAATGAAGTAGGTTCCAGTACTAATGAAGTGGGCTCCAGTATTAATGAAATAGGTTCCAGTGATGAAAACATTCAAGCA",
+        "Mouse": "TTCATGAAGTATGTTCCACT------------------------------------------GGTGACTCCTTCCCAGGA",
+    }
+    seqs = new_alignment.make_unaligned_seqs(dotplot_seqs, moltype="dna")
+    plot = seqs.dotplot()
+    # trigger construction of figure dict
+    _ = plot.figure
+    # make sure the aligned path trace does NOT exists
+    assert all(tr["name"] != "Alignment" for tr in plot.traces)
+
+
 @pytest.mark.parametrize("with_annotations", [True, False])
 @pytest.mark.parametrize(
     "mk_cls",
@@ -160,11 +173,26 @@ def test_dotplot_annotated(annotated_seq, with_annotations, mk_cls):
         annotated_seq.replace_annotation_db(None)  # this drops all annotations
 
     coll = mk_cls({"c_elegans": annotated_seq}, moltype="dna")
-    dp = coll.dotplot()
-    if with_annotations:
-        assert len(dp.figure.data) > 2
-    else:
-        assert len(dp.figure.data) == 2
+    aligned = mk_cls == new_alignment.make_aligned_seqs
+    fig = coll.dotplot().figure
+    base = {"Alignment", "+ strand"} if aligned else {"+ strand"}
+    expect = base | {"gene"} if with_annotations else base
+    got = {tr["name"] for tr in fig.data}
+    assert got == expect
+
+
+@pytest.mark.parametrize(
+    "mk_cls",
+    [new_alignment.make_unaligned_seqs, new_alignment.make_aligned_seqs],
+)
+def test_dotplot_just_gaps(mk_cls):
+    eqs = {
+        "Human": "ACTAATGAAGTGGGCTCCAGTATTAATGA",
+        "Mouse": "-----------------------------",
+    }
+    coll = mk_cls(eqs, moltype="dna")
+    with pytest.raises(ValueError):
+        coll.dotplot()
 
 
 def test_dotplot_regression():