Repository: cogent3/cogent3
Pull request #1810 (2024-04-08 22:42:24)

BUG: Alignment.quick_tree() defaults to pdist calculator
[FIXED] previous default was discontinued!

[NEW] test default settings of quick_tree()


Patch:
diff --git a/changelog.d/20240329_152534_Gavin.Huttley.md b/changelog.d/20240329_152534_Gavin.Huttley.md
index 779132349..b8b15800d 100644
--- a/changelog.d/20240329_152534_Gavin.Huttley.md
+++ b/changelog.d/20240329_152534_Gavin.Huttley.md
@@ -22,12 +22,11 @@ Uncomment the section that is right (remove the HTML comment wrapper).
   length is set to None. Think of this tree as the true "evolutionary tree".
   Thanks to Von Bing Yap for suggesting this!
 
-<!--
 ### BUG
 
-- A bullet item for the BUG category.
+- Alignment.quick_tree() default settings referred to a distance calculator
+  that has been removed. Now updated to the pdist calculator.
 
--->
 <!--
 ### DOC
 
diff --git a/src/cogent3/core/alignment.py b/src/cogent3/core/alignment.py
index 16f37e0e3..e72883d40 100644
--- a/src/cogent3/core/alignment.py
+++ b/src/cogent3/core/alignment.py
@@ -3276,7 +3276,7 @@ class AlignmentI(object):
     @extend_docstring_from(distance_matrix, pre=False)
     def quick_tree(
         self,
-        calc="percent",
+        calc="pdist",
         bootstrap=None,
         drop_invalid=False,
         show_progress=False,


Test patch:
diff --git a/tests/test_core/test_alignment.py b/tests/test_core/test_alignment.py
index dae90eea2..78070ee2b 100644
--- a/tests/test_core/test_alignment.py
+++ b/tests/test_core/test_alignment.py
@@ -1685,19 +1685,6 @@ class AlignmentBaseTests(SequenceCollectionBaseTests):
         # but setting drop_invalid=False allows calc
         dists = aln.distance_matrix(calc="paralinear", drop_invalid=True)
 
-    def test_quick_tree(self):
-        """quick tree method returns tree"""
-        aln = self.Class(self.brca1_data, moltype=DNA)
-        aln = aln.take_seqs(aln.names[:5])[:100]
-        tree = aln.quick_tree(calc="hamming", show_progress=False)
-        # bootstrap
-        tree = aln.quick_tree(calc="hamming", bootstrap=2, show_progress=False)
-        self.assertEqual(set(tree.get_tip_names()), set(aln.names))
-        for edge in tree.preorder():
-            if edge.is_root():
-                continue
-            self.assertIsInstance(edge.params["support"], float)
-
     def test_get_gapped_seq(self):
         """Alignment.get_gapped_seq should return seq, with gaps"""
         aln = self.Class({"seq1": "--TTT?", "seq2": "GATC??"})
@@ -3619,3 +3606,24 @@ def test_array_align_error_with_mixed_length():
     data = dict(s1="ACGG", s2="A-G")
     with pytest.raises(ValueError, match=".* not all the same length.*"):
         make_aligned_seqs(data=data)
+
+
+@pytest.mark.parametrize("cls", (Alignment, ArrayAlignment))
+@pytest.mark.parametrize("calc", ("hamming", None))
+def test_quick_tree(cls, calc, brca1_data):
+    """quick tree method returns tree"""
+    aln = cls(brca1_data, moltype=DNA)[:100]
+    aln = aln.take_seqs(["Human", "Rhesus", "HowlerMon", "Galago", "Mouse"])
+    kwargs = dict(show_progress=False)
+    if calc:
+        kwargs["calc"] = calc
+
+    # bootstrap
+    tree = aln.quick_tree(bootstrap=2, **kwargs)
+    assert set(tree.get_tip_names()) == set(aln.names)
+    types = {
+        type(float(edge.params["support"]))
+        for edge in tree.preorder()
+        if not edge.is_root()
+    }
+    assert types == {float}