fix: several small bugs found in a static-analysis sweep

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):

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

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,

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

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

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:

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."

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()

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):

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):

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()

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()