diff --git a/src/dendropy/calculate/phylogeneticdistance.py b/src/dendropy/calculate/phylogeneticdistance.py index dc7c6273c..0fa166578 100644 --- a/src/dendropy/calculate/phylogeneticdistance.py +++ b/src/dendropy/calculate/phylogeneticdistance.py @@ -1362,7 +1362,7 @@ def _calculate_standardized_effect_size(self, for rep_idx in range(num_randomization_replicates): null_model_matrix.shuffle_taxa( is_shuffle_phylogenetic_distances=is_shuffle_phylogenetic_distances, - is_shuffle_phylogenetic_path_steps=is_shuffle_phylogenetic_distances, + is_shuffle_phylogenetic_path_steps=is_shuffle_phylogenetic_path_steps, is_shuffle_mrca=False, rng=rng) for comparison_regime_idx, comparison_regime in enumerate(comparison_regimes): diff --git a/src/dendropy/calculate/treesum.py b/src/dendropy/calculate/treesum.py index c00841c00..8eb8a1e3c 100644 --- a/src/dendropy/calculate/treesum.py +++ b/src/dendropy/calculate/treesum.py @@ -242,8 +242,8 @@ def summarize_node_ages_on_tree(self, """ if summarization_fn is None: summarization_fn = lambda x: float(sum(x))/len(x) - if is_bipartitions_updated: - tree.encode_splits() + if not is_bipartitions_updated: + tree.encode_bipartitions() #'height', #'height_median', #'height_95hpd', @@ -263,8 +263,12 @@ def summarize_node_ages_on_tree(self, nd.age = nd.parent_node.age else: nd.age = 0 - ## force parent nodes to be at least as old as their oldest child - if collapse_negative_edges: + if collapse_negative_edges: + ## force parent nodes to be at least as old as their oldest + ## child; this requires a postorder pass, since the preorder + ## pass above visits parents before the children whose ages + ## it needs to compare against + for nd in tree.postorder_node_iter(): for child in nd.child_nodes(): if child.age > nd.age: nd.age = child.age diff --git a/src/dendropy/datamodel/charmatrixmodel.py b/src/dendropy/datamodel/charmatrixmodel.py index 4a4be328b..8be381821 100644 --- a/src/dendropy/datamodel/charmatrixmodel.py +++ b/src/dendropy/datamodel/charmatrixmodel.py @@ -628,7 +628,7 @@ def concatenate(cls, char_matrices): v1 = len(cm[0]) for t, s in cm.items(): if len(s) != v1: - raise ValueError("Unequal length sequences in character matrix %d".format(cidx+1)) + raise ValueError("Unequal length sequences in character matrix {}".format(cidx+1)) concatenated_chars.extend_matrix(cm) if cm.label is None: new_label = "locus%03d" % cidx @@ -637,7 +637,7 @@ def concatenate(cls, char_matrices): cs_label = new_label i = 2 while cs_label in concatenated_chars.character_subsets: - label = "%s_%03d" % (new_label, i) + cs_label = "%s_%03d" % (new_label, i) i += 1 character_indices = range(pos_start, pos_start + cm.vector_size) pos_start += cm.vector_size @@ -1759,7 +1759,7 @@ def append_taxon_sequence(self, taxon, state_symbols): symbol = value else: symbol = str(value) - self[taxon].append(self.default_symbol_state_map[symbol]) + self[taxon].append(self.default_state_alphabet.full_symbol_state_map[symbol]) def remap_to_state_alphabet_by_symbol(self, state_alphabet, diff --git a/src/dendropy/datamodel/taxonmodel.py b/src/dendropy/datamodel/taxonmodel.py index e459a6c1b..1fad2cc29 100644 --- a/src/dendropy/datamodel/taxonmodel.py +++ b/src/dendropy/datamodel/taxonmodel.py @@ -1660,7 +1660,7 @@ def __init__(self, label=None): string, then the ``label`` attribute of ``self`` is set to this value. If a |Taxon| object, then the ``label`` attribute of ``self`` is set to the same value as the ``label`` attribute the other - |Taxon| object and all annotations/metadata are copied. + |Taxon| object and all annotations/comments/metadata are copied. """ if isinstance(label, Taxon): other_taxon = label @@ -1674,7 +1674,7 @@ def __init__(self, label=None): else: basemodel.DataObject.__init__(self, label=label) self._lower_cased_label = None - self.comments = [] + self.comments = [] def _get_label(self): return self._label diff --git a/src/dendropy/datamodel/treemodel/_node.py b/src/dendropy/datamodel/treemodel/_node.py index 22336b829..427f73df9 100644 --- a/src/dendropy/datamodel/treemodel/_node.py +++ b/src/dendropy/datamodel/treemodel/_node.py @@ -1379,6 +1379,8 @@ def distance_from_root(self): elif self._parent_node and self.edge.length == None: # what do we do here: parent node exists, but my # length does not? + if self._parent_node.edge.length == None: + return 0.0 return float(self._parent_node.edge.length) elif not self._parent_node and self.edge.length == None: # no parent node, and no edge length diff --git a/src/dendropy/model/discrete.py b/src/dendropy/model/discrete.py index a90d3a83a..1920c06b1 100644 --- a/src/dendropy/model/discrete.py +++ b/src/dendropy/model/discrete.py @@ -473,7 +473,7 @@ def simulate_discrete_chars( mutation_rate : float Mutation *modifier* rate (should be 1.0 if branch lengths on tree reflect true expected number of changes). - root_states`` : list + root_states : list Vector of root states (length must equal ``seq_len``). char_matrix : |DnaCharacterMatrix| If given, new sequences for taxa on ``tree_model`` leaf_nodes will be @@ -496,7 +496,7 @@ def simulate_discrete_chars( tree = seq_evolver.evolve_states( tree=tree_model, seq_len=seq_len, - root_states=None, + root_states=root_states, rng=rng) tree.migrate_taxon_namespace(tree_model.taxon_namespace) if char_matrix is None: diff --git a/src/dendropy/utility/container.py b/src/dendropy/utility/container.py index 876ef7249..582924fc0 100644 --- a/src/dendropy/utility/container.py +++ b/src/dendropy/utility/container.py @@ -283,7 +283,7 @@ def __deepcopy__(self, memo): def normalize_key_and_assign_value(self, key, value): "*Almost* like __setitem__(), but returns value of normalized key to calling code." normalized_key = self.normalize_key(key) - dict.__setitem__(self, normalized_key, value) + collections.OrderedDict.__setitem__(self, normalized_key, value) return normalized_key def normalize_key(self, key): @@ -301,7 +301,7 @@ def __getitem__(self, key): def __delitem__(self, key): "Remove item with normalized key." key = self.normalize_key(key) - dict.__delitem__(self, key) + collections.OrderedDict.__delitem__(self, key) def __contains__(self, key): "Returns true if has normalized key." diff --git a/tests/unittests/test_container_normalized_bitmask_dict.py b/tests/unittests/test_container_normalized_bitmask_dict.py index 8cc777c8f..32e08a72e 100644 --- a/tests/unittests/test_container_normalized_bitmask_dict.py +++ b/tests/unittests/test_container_normalized_bitmask_dict.py @@ -45,8 +45,14 @@ def runTest(self): self.assertIn(s[1][0], d) self.assertEqual(d[s[0][0]], d[s[1][0]]) - for k, v in d.items(): - pass + # Regression test: NormalizedBitmaskDict.__setitem__() used to call + # dict.__setitem__() directly, bypassing OrderedDict's internal + # bookkeeping. This left len()/__repr__() correct but silently + # broke iteration (.items()/.keys()/.values()/__iter__()), which + # would all report zero entries despite the dict being non-empty. + seen_keys = set(k for k, v in d.items()) + self.assertEqual(len(seen_keys), len(splits)) + self.assertEqual(len(d), len(splits)) del d[splits[0][0][0]] del d[splits[1][1][0]] @@ -55,5 +61,11 @@ def runTest(self): self.assertNotIn(splits[1][0][0], d) self.assertNotIn(splits[1][1][0], d) + # Regression test: __delitem__() had the same dict-vs-OrderedDict + # bypass problem; confirm iteration still reflects the deletions. + seen_keys_after_delete = set(k for k, v in d.items()) + self.assertEqual(len(seen_keys_after_delete), len(splits) - 2) + self.assertEqual(len(d), len(splits) - 2) + if __name__ == "__main__": unittest.main() diff --git a/tests/unittests/test_datamodel_charmatrix.py b/tests/unittests/test_datamodel_charmatrix.py index 6fd141324..dac7805a7 100644 --- a/tests/unittests/test_datamodel_charmatrix.py +++ b/tests/unittests/test_datamodel_charmatrix.py @@ -439,6 +439,57 @@ def test_extend_matrix(self): self.verify_sequence_equal(c1[tns[1]], [2, 2, 2, 3, 3, 3]) self.verify_sequence_equal(c1[tns[2]], [4, 4, 4]) +class CharacterMatrixConcatenateTest(dendropytest.ExtendedTestCase): + + def test_unequal_length_sequence_error_message(self): + # Regression test: the error message used the '%d' printf-style + # placeholder but applied '.format()' to the string, so the + # placeholder was never substituted and the matrix index was + # missing from the raised message. + tns = get_taxon_namespace(2) + cm1 = charmatrixmodel.CharacterMatrix(taxon_namespace=tns) + cm1[tns[0]] = [1, 1, 1, 1] + cm1[tns[1]] = [1, 1] + with self.assertRaises(ValueError) as ctx: + charmatrixmodel.CharacterMatrix.concatenate([cm1]) + self.assertIn("1", str(ctx.exception)) + self.assertNotIn("%d", str(ctx.exception)) + + def test_concatenate_does_not_hang_on_duplicate_labels(self): + # Regression test: when two matrices being concatenated share the + # same (or no) label, the de-duplication loop computed a new + # candidate label into 'label' but checked/looped on 'cs_label', + # which never changed -- an infinite loop on any label collision. + tns = get_taxon_namespace(2) + cm1 = charmatrixmodel.CharacterMatrix(taxon_namespace=tns) + cm1.label = "dupe" + cm1[tns[0]] = [1, 1] + cm1[tns[1]] = [1, 1] + cm2 = charmatrixmodel.CharacterMatrix(taxon_namespace=tns) + cm2.label = "dupe" + cm2[tns[0]] = [2, 2] + cm2[tns[1]] = [2, 2] + concatenated = charmatrixmodel.CharacterMatrix.concatenate([cm1, cm2]) + self.assertEqual(len(concatenated.character_subsets), 2) + labels = set(concatenated.character_subsets.keys()) + self.assertIn("dupe", labels) + self.assertEqual(len(labels), 2) + +class CharacterMatrixAppendTaxonSequenceTest(dendropytest.ExtendedTestCase): + + def test_append_taxon_sequence_with_symbols(self): + # Regression test: 'default_symbol_state_map' does not exist on + # DiscreteCharacterMatrix (or any subclass); any non-empty call + # raised an AttributeError. + tns = get_taxon_namespace(1) + cm = dendropy.DnaCharacterMatrix(taxon_namespace=tns) + taxon = tns[0] + cm.append_taxon_sequence(taxon, "ACGT") + self.assertEqual(len(cm[taxon]), 4) + self.assertEqual( + [s.symbol for s in cm[taxon]], + ["A", "C", "G", "T"]) + class CharacterMatrixTaxonManagement(dendropytest.ExtendedTestCase): def test_assign_taxon_namespace(self): diff --git a/tests/unittests/test_datamodel_taxon.py b/tests/unittests/test_datamodel_taxon.py index c9d97be05..46f15838f 100644 --- a/tests/unittests/test_datamodel_taxon.py +++ b/tests/unittests/test_datamodel_taxon.py @@ -130,6 +130,17 @@ def test_construct_from_another_with_complex_annotations(self): self.assertEqual(t2.annotations[1].value, "z") t1.label = "a" + def test_construct_from_another_preserves_comments(self): + # Regression test: Taxon.__init__() unconditionally reset + # 'self.comments = []' after the if/else branch that (for the + # Taxon-from-Taxon case) had just deep-copied 'comments' from the + # source Taxon, silently discarding it. + t1 = Taxon("a") + t1.comments.append("hello world") + for t2 in (Taxon(t1), copy.deepcopy(t1), t1.clone(2)): + self.assertIsNot(t1, t2) + self.assertEqual(t2.comments, ["hello world"]) + def test_simple_copy(self): t1 = Taxon("a") with self.assertRaises(TypeError): diff --git a/tests/unittests/test_datamodel_tree_node_fundamentals.py b/tests/unittests/test_datamodel_tree_node_fundamentals.py index 568551fa8..da1a0dd8a 100644 --- a/tests/unittests/test_datamodel_tree_node_fundamentals.py +++ b/tests/unittests/test_datamodel_tree_node_fundamentals.py @@ -322,5 +322,21 @@ def test_edge_head_node_setting(self): self.assertIs(node.edge, edge2) self.assertIs(node.edge.head_node, node) +class TestNodeDistanceFromRoot(unittest.TestCase): + + def test_none_edge_length_with_parent_also_none(self): + # Regression test: when a node's own edge length is None and it + # has a parent, distance_from_root() falls back to + # float(parent.edge.length); if the parent's edge length is also + # None, this raised a TypeError instead of degrading gracefully + # (as the sibling all-None-length branch a few lines down does, + # which returns 0.0). + parent = dendropy.Node(label="parent") + child = dendropy.Node(label="child") + parent.set_child_nodes([child]) + parent.edge.length = None + child.edge.length = None + self.assertEqual(child.distance_from_root(), 0.0) + if __name__ == "__main__": unittest.main() diff --git a/tests/unittests/test_discrete.py b/tests/unittests/test_discrete.py index 8fcaa1c14..bfbeb4f53 100644 --- a/tests/unittests/test_discrete.py +++ b/tests/unittests/test_discrete.py @@ -23,6 +23,7 @@ import random import unittest +import dendropy from dendropy.model import discrete from dendropy.simulate import treesim @@ -56,5 +57,28 @@ def test_rng_param(self): ) ) + def test_simulate_discrete_chars_honors_root_states(self): + """ + Regression test: simulate_discrete_chars() accepts a 'root_states' + parameter but was passing 'root_states=None' on to the underlying + evolver, silently discarding the caller's requested root sequence. + """ + tree = dendropy.Tree.get(data="(A:1,B:1):0;", schema="newick") + seq_model = discrete.Jc69() + alphabet_states = list(seq_model.state_alphabet) + seq_len = 8 + root_states = [alphabet_states[i % 4] for i in range(seq_len)] + discrete.simulate_discrete_chars( + seq_len=seq_len, + tree_model=tree, + seq_model=seq_model, + root_states=root_states, + retain_sequences_on_tree=True, + rng=random.Random(1), + ) + root_node = tree.seed_node + root_sequence = root_node.sequences[-1] + self.assertEqual(list(root_sequence), root_states) + if __name__ == "__main__": unittest.main() diff --git a/tests/unittests/test_tree_summarization_and_consensus.py b/tests/unittests/test_tree_summarization_and_consensus.py index ec57dcb0e..86d6b0c09 100644 --- a/tests/unittests/test_tree_summarization_and_consensus.py +++ b/tests/unittests/test_tree_summarization_and_consensus.py @@ -28,6 +28,7 @@ import itertools from dendropy.calculate import treecompare from dendropy.calculate import statistics +from dendropy.calculate import treesum import os import sys sys.path.insert(0, os.path.dirname(__file__)) @@ -218,6 +219,121 @@ def testMeanNodeAgeSummarizationOnMCCT(self): obs_edge = target_tree.bipartition_edge_map[exp_bipartition] self.assertAlmostEqual(obs_edge.head_node.age, exp_edge.head_node.age) +class TestSummarizeNodeAgesOnTree(unittest.TestCase): + """ + Regression tests for TreeSummarizer.summarize_node_ages_on_tree(). + """ + + def setUp(self): + self.taxon_namespace = dendropy.TaxonNamespace(["A", "B", "C"]) + support_newicks = [ + "(A:1,(B:1,C:1):1):0;", + "(A:2,(B:1,C:2):1):0;", + "(A:1,(B:2,C:1):2):0;", + ] + self.support_trees = dendropy.TreeList(taxon_namespace=self.taxon_namespace) + for newick in support_newicks: + tree = dendropy.Tree.get( + data=newick, + schema="newick", + taxon_namespace=self.taxon_namespace) + tree.encode_bipartitions() + self.support_trees.append(tree) + self.split_distribution = dendropy.SplitDistribution( + taxon_namespace=self.taxon_namespace) + for tree in self.support_trees: + self.split_distribution.count_splits_on_tree(tree) + + def _get_target_tree(self): + target_tree = dendropy.Tree.get( + data="(A:1,(B:1,C:1):1):0;", + schema="newick", + taxon_namespace=self.taxon_namespace) + target_tree.encode_bipartitions() + return target_tree + + def test_collapse_negative_edges_does_not_crash(self): + # 'collapse_negative_edges=True' used to compare a not-yet-visited + # child's 'age' (still None, since ages are assigned in preorder) + # against its parent's, raising a TypeError. + target_tree = self._get_target_tree() + summarizer = treesum.TreeSummarizer() + result_tree = summarizer.summarize_node_ages_on_tree( + tree=target_tree, + split_distribution=self.split_distribution, + set_edge_lengths=True, + collapse_negative_edges=True, + is_bipartitions_updated=True, + ) + for nd in result_tree.preorder_node_iter(): + if nd.parent_node is not None: + self.assertGreaterEqual(nd.parent_node.age, nd.age) + + def test_collapse_negative_edges_raises_parent_to_oldest_child(self): + # 'collapse_negative_edges=True' must actually *collapse* negative + # edges, not merely avoid crashing: when the summarized age of an + # internal node comes out older than its parent's summarized age + # (a negative edge), the parent's age is raised to its oldest + # child's age. Here the (B,C) split is summarized deep while the + # root split is summarized shallow, so without collapsing the root + # ends up younger than its (B,C) child. + support_newicks = [ + "((A:0.1,B:0.1):0.1,C:0.2);", # topology ((A,B),C); root age 0.2, no (B,C) split + "((A:0.1,B:0.1):0.1,C:0.2);", # again, to keep the root/full-split age shallow + "(A:1.0,(B:0.9,C:0.9):0.1);", # topology (A,(B,C)); (B,C) split age 0.9 + ] + split_distribution = dendropy.SplitDistribution( + taxon_namespace=self.taxon_namespace, + ignore_node_ages=False) + for newick in support_newicks: + tree = dendropy.Tree.get( + data=newick, + schema="newick", + taxon_namespace=self.taxon_namespace, + rooting="force-rooted") + tree.encode_bipartitions() + tree.calc_node_ages() + split_distribution.count_splits_on_tree( + tree, is_bipartitions_updated=True) + + def summarize(collapse_negative_edges): + target_tree = dendropy.Tree.get( + data="(A:1,(B:1,C:1):1);", + schema="newick", + taxon_namespace=self.taxon_namespace, + rooting="force-rooted") + target_tree.encode_bipartitions() + treesum.TreeSummarizer().summarize_node_ages_on_tree( + tree=target_tree, + split_distribution=split_distribution, + set_edge_lengths=False, + collapse_negative_edges=collapse_negative_edges, + is_bipartitions_updated=True, + ) + ages = {} + for nd in target_tree.preorder_node_iter(): + key = frozenset( + leaf.taxon.label for leaf in nd.leaf_iter()) + ages[key] = nd.age + return ages + + root_key = frozenset(["A", "B", "C"]) + internal_key = frozenset(["B", "C"]) + + # Sanity check on the constructed fixture: without collapsing, the + # (B,C) child is summarized older than the root, i.e. a negative edge. + uncollapsed = summarize(collapse_negative_edges=False) + self.assertGreater( + uncollapsed[internal_key], uncollapsed[root_key], + "fixture should produce a negative edge (child older than " + "parent) when negative edges are not collapsed") + + # With collapsing, the parent's age is raised to its oldest child's + # age, so no node is older than its parent. + collapsed = summarize(collapse_negative_edges=True) + self.assertEqual(collapsed[root_key], uncollapsed[internal_key]) + self.assertGreaterEqual(collapsed[root_key], collapsed[internal_key]) + class TestTopologyCounter(dendropytest.ExtendedTestCase): def get_regime(self,