Repository: cogent3/cogent3 (python)

Issue:
aln.iupac_consensus doesn't work as expected
**Describe the bug**
aln.iupac_consensus produces a string with only "???" instead of a sequence string with IUPAC codes.

**To Reproduce**
Steps to reproduce the behavior:
```
aln = load_aligned_seqs("my_file.fasta", moltype="dna")
iupac_consensus = aln.iupac_consensus(alphabet=None)
print(iupac_consensus)
```
This produces only "????????" instead of a sequence with IUPAC codes

**Expected behavior**
Consenus sequences with IUPAC codes







Pull request description:
ENH: iupac_consensus allows ignoring gaps, fixes #1278
[NEW] allow_gaps argument controls whether gaps included in deriving
    consensus sequence.

Patch:
diff --git a/src/cogent3/app/align.py b/src/cogent3/app/align.py
index 2117919ed..d85ec7a70 100644
--- a/src/cogent3/app/align.py
+++ b/src/cogent3/app/align.py
@@ -396,7 +396,7 @@ class align_to_ref:
                 continue
 
             aln = global_pairwise(ref_seq, seq, **kwargs).to_type(array_align=False)
-            pwise.append(((seq.name, aln)))
+            pwise.append((seq.name, aln))
 
         return pairwise_to_multiple(pwise, ref_seq, self._moltype, info=seqs.info)
 
diff --git a/src/cogent3/app/composable.py b/src/cogent3/app/composable.py
index 690d02641..7b3315f29 100644
--- a/src/cogent3/app/composable.py
+++ b/src/cogent3/app/composable.py
@@ -1420,9 +1420,7 @@ def _apply_to(
     for result in self.as_completed(
         inputs, parallel=parallel, par_kw=par_kw, show_progress=show_progress
     ):
-        member = self.main(
-            data=result.obj, identifier=id_from_source(result.source)
-        )  # writers must return DataMember
+        member = self.main(data=result.obj, identifier=id_from_source(result.source))
         md5 = member.md5
         logger.log_message(str(member), label="output")
         if md5:
diff --git a/src/cogent3/app/data_store_new.py b/src/cogent3/app/data_store_new.py
index e54e3429a..1466381d9 100644
--- a/src/cogent3/app/data_store_new.py
+++ b/src/cogent3/app/data_store_new.py
@@ -351,7 +351,7 @@ def summary_not_completeds(
     for record in types:
         messages, sources = list(zip(*types[record]))
         messages = reprlib.repr(", ".join(m.splitlines()[-1] for m in set(messages)))
-        sources = reprlib.repr(", ".join(s.splitlines()[-1] for s in sources))
+        sources = reprlib.repr(", ".join(s.splitlines()[-1] for s in sources if s))
         row = list(record) + [
             messages,
             len(types[record]),
diff --git a/src/cogent3/app/io_new.py b/src/cogent3/app/io_new.py
index 20c9a053d..0d24680c6 100644
--- a/src/cogent3/app/io_new.py
+++ b/src/cogent3/app/io_new.py
@@ -450,7 +450,13 @@ class load_db:
 
     def main(self, identifier: IdentifierType) -> SerialisableType:
         """returns deserialised object"""
-        data = identifier.read()
+        try:
+            data = identifier.read()
+        except AttributeError:
+            raise AttributeError(
+                f"{identifier} failed because its of type {type(identifier)}"
+            )
+
         # do we need to inject identifier attribute?
         result = self.deserialiser(data)
         if hasattr(result, "info"):
@@ -557,7 +563,7 @@ class write_tabular:
         return self.data_store.write(unique_id=identifier, data=output)
 
 
-@define_app(app_type=WRITER)
+@define_app(app_type=WRITER, skip_not_completed=False)
 class write_db:
     """Write serialised objects to a database instance."""
 
diff --git a/src/cogent3/core/alignment.py b/src/cogent3/core/alignment.py
index a8af55d6f..473ba9909 100644
--- a/src/cogent3/core/alignment.py
+++ b/src/cogent3/core/alignment.py
@@ -236,6 +236,9 @@ def assign_sequential_names(ignored, num_seqs, base_name="seq", start_at=0):
 
 def coerce_to_string(s):
     """Converts an arbitrary sequence into a string."""
+    if isinstance(s, (set, frozenset)):
+        return "".join(c for c in s)
+
     if isinstance(s, str):  # if it's a string, OK as is
         return s
     if isinstance(s, Aligned):  # if it's an Aligned object, convert to string
@@ -2349,13 +2352,16 @@ class AlignmentI(object):
         """Returns new Alignment containing cols where f(col) is True."""
         return self.take_positions(self.get_position_indices(f, negate=negate))
 
-    def iupac_consensus(self, alphabet=None):
+    def iupac_consensus(self, alphabet=None, allow_gaps=True):
         """Returns string containing IUPAC consensus sequence of the alignment."""
         if alphabet is None:
             alphabet = self.moltype
+
+        exclude = set() if allow_gaps else set(alphabet.gaps)
         consensus = []
         degen = alphabet.degenerate_from_seq
         for col in self.positions:
+            col = set(col) - exclude
             consensus.append(degen(coerce_to_string(col)))
         return coerce_to_string(consensus)
 
@@ -3880,15 +3886,20 @@ class ArrayAlignment(AlignmentI, _SequenceCollectionBase):
 
         return f"{len(self.names)} x {self.seq_len} alignment: {seqs}"
 
-    def iupac_consensus(self, alphabet=None):
+    def iupac_consensus(self, alphabet=None, allow_gaps=True):
         """Returns string containing IUPAC consensus sequence of the alignment."""
         if alphabet is None:
             alphabet = self.moltype
+
+        exclude = set() if allow_gaps else set(alphabet.gaps)
         consensus = []
         degen = alphabet.degenerate_from_seq
         for col in self.positions:
-            col = alphabet.make_array_seq(col, alphabet=alphabet.alphabets.degen_gapped)
-            consensus.append(degen(str(col)))
+            col = set(
+                alphabet.make_array_seq(col, alphabet=alphabet.alphabets.degen_gapped)
+            )
+            col -= exclude
+            consensus.append(degen(coerce_to_string(col)))
         return coerce_to_string(consensus)
 
     def sample(
diff --git a/src/cogent3/core/tree.py b/src/cogent3/core/tree.py
index e6bd07a5f..fbbd601de 100644
--- a/src/cogent3/core/tree.py
+++ b/src/cogent3/core/tree.py
@@ -100,14 +100,12 @@ class TreeNode(object):
         """Returns new TreeNode object."""
         self.name = name
         self.name_loaded = name_loaded
-        if params is None:
-            params = {}
-        self.params = params
+        self.params = params or {}
         self.children = []
         if children is not None:
             self.extend(children)
         self._parent = parent
-        if (parent is not None) and not (self in parent.children):
+        if parent is not None and self not in parent.children:
             parent.append(self)
 
     # built-in methods and list interface support
@@ -143,10 +141,7 @@ class TreeNode(object):
 
     def compare_name(self, other):
         """Compares TreeNode by name"""
-        if self is other:
-            return True
-
-        return self.name == other.name
+        return True if self is other else self.name == other.name
 
     def compare_by_names(self, other):
         """Equality test for trees by name"""


Test patch:
diff --git a/tests/test_core/test_alignment.py b/tests/test_core/test_alignment.py
index 57182da4c..b615738b7 100644
--- a/tests/test_core/test_alignment.py
+++ b/tests/test_core/test_alignment.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+import itertools
 import json
 import os
 import pathlib
@@ -9,11 +9,18 @@ from tempfile import TemporaryDirectory, mktemp
 from unittest import TestCase, main
 
 import numpy
+import pytest
 
 from numpy import array, log2, nan, transpose
 from numpy.testing import assert_allclose, assert_equal
 
-from cogent3 import load_aligned_seqs, load_unaligned_seqs, make_seq, open_
+from cogent3 import (
+    load_aligned_seqs,
+    load_unaligned_seqs,
+    make_aligned_seqs,
+    make_seq,
+    open_,
+)
 from cogent3.core.alignment import (
     Aligned,
     Alignment,
@@ -3370,6 +3377,25 @@ class IntegrationTests(TestCase):
         self.assertEqual(self.r1.name, "x")
 
 
+@pytest.mark.parametrize(
+    "moltype,array_align",
+    tuple(itertools.product(["rna", "dna", "protein"], [True, False])),
+)
+def test_upac_consensus_allow_gaps(moltype, array_align):
+    aln = make_aligned_seqs(
+        data={"s1": "ACGG", "s2": "ACGG", "s3": "-CGG"},
+        moltype=moltype,
+        array_align=array_align,
+    )
+    # default behaviour
+    iupac = aln.iupac_consensus()
+    assert iupac == "?CGG"
+
+    # allow_gaps
+    iupac = aln.iupac_consensus(allow_gaps=False)
+    assert iupac == "ACGG"
+
+
 # run tests if invoked from command line
 if __name__ == "__main__":
     main()