Repository: cogent3/cogent3
Pull request #2277 (2025-04-06 02:59:36)

ENH: make name_map a public property
## Summary by Sourcery

Expose name_map as a public, immutable property in SequenceCollection and Alignment classes, with improved handling of sequence renaming

New Features:
- Introduced a new ImmutableDict type to provide a read-only dictionary for name mappings
- Added a public name_map property to SequenceCollection and Alignment classes

Enhancements:
- Improved rename_seqs() method to handle multiple renames while preserving original sequence names
- Enhanced name mapping to support tracking of original and renamed sequence names

Tests:
- Added comprehensive tests for the new ImmutableDict implementation
- Added tests for name_map behavior during sequence renaming


Patch:
diff --git a/changelog.d/20250406_125550_Gavin.Huttley.md b/changelog.d/20250406_125550_Gavin.Huttley.md
new file mode 100644
index 000000000..d9d3186d7
--- /dev/null
+++ b/changelog.d/20250406_125550_Gavin.Huttley.md
@@ -0,0 +1,45 @@
+<!--
+A new scriv changelog fragment.
+
+Uncomment the section that is right (remove the HTML comment wrapper).
+-->
+
+<!--
+### Contributors
+
+- A bullet item for the Contributors category.
+
+-->
+### ENH
+
+- new type `SequenceCollection` and `Alignment` classes now have
+  a public `name_map` property. This records the mapping between
+  the sequence names in the collection and the names in the
+  underlying seqsdata container. This is required for using annotations.
+  The attribute is produced by the `rename_seqs()` method. `name_map`
+  is an immutable dictionary.
+
+<!--
+### BUG
+
+- A bullet item for the BUG category.
+
+-->
+<!--
+### DOC
+
+- A bullet item for the DOC category.
+
+-->
+<!--
+### Deprecations
+
+- A bullet item for the Deprecations category.
+
+-->
+<!--
+### Discontinued
+
+- A bullet item for the Discontinued category.
+
+-->
diff --git a/src/cogent3/core/new_alignment.py b/src/cogent3/core/new_alignment.py
index c140fb833..b6df2f2f6 100644
--- a/src/cogent3/core/new_alignment.py
+++ b/src/cogent3/core/new_alignment.py
@@ -5,6 +5,7 @@ import dataclasses
 import json
 import os
 import re
+import types
 import typing
 import warnings
 from abc import ABC, abstractmethod
@@ -661,7 +662,8 @@ class SequenceCollection:
         """
         self._seqs_data = seqs_data
         self.moltype = moltype
-        self._name_map = name_map or {name: name for name in seqs_data.names}
+        name_map = name_map or {name: name for name in seqs_data.names}
+        self._name_map = types.MappingProxyType(name_map)
         self._is_reversed = is_reversed
         if not isinstance(info, InfoClass):
             info = InfoClass(info) if info else InfoClass()
@@ -703,7 +705,7 @@ class SequenceCollection:
         return {
             "seqs_data": self._seqs_data,
             "moltype": self.moltype,
-            "name_map": self._name_map.copy(),
+            "name_map": dict(self._name_map),
             "info": self.info.copy(),
             "annotation_db": self.annotation_db,
             "source": self.source,
@@ -718,6 +720,26 @@ class SequenceCollection:
     def names(self) -> list:
         return list(self._name_map.keys())
 
+    @property
+    def name_map(self) -> types.MappingProxyType:
+        """returns mapping of seq names to parent seq names
+
+        Notes
+        -----
+        The underlying SeqsData may have different names for the same
+        sequences. This object maps the names of sequences in self to
+        the names of sequences in SeqsData.
+        MappingProxyType is an immutable mapping, so it cannot be
+        changed. Use self.rename_seqs() to do that.
+        """
+        return self._name_map
+
+    @name_map.setter
+    def name_map(self, value: OptDict) -> None:  # noqa: ARG002
+        """name_map can only be set at initialisation"""
+        msg = "name_map cannot be set after initialisation"
+        raise TypeError(msg)
+
     @property
     def num_seqs(self) -> int:
         return len(self.names)
@@ -929,11 +951,31 @@ class SequenceCollection:
             annotation_db=self.annotation_db,
         )
 
-    def rename_seqs(self, renamer: Callable[[str], str]):
-        """Returns new collection with renamed sequences."""
-        new_name_map = {
-            renamer(name): old_name for name, old_name in self._name_map.items()
-        }
+    def rename_seqs(self, renamer: Callable[[str], str]) -> typing_extensions.Self:
+        """Returns new collection with renamed sequences.
+
+        Parameters
+        ----------
+        renamer
+            callable that takes a name string and returns a string
+
+        Raises
+        ------
+        ValueError if renamer produces duplicate names.
+
+        Notes
+        -----
+        The resulting object stores the mapping of new to old names in
+        self.name_map.
+        """
+        new_name_map = {}
+        for name, parent_name in self._name_map.items():
+            new_name = renamer(name)
+            # we retain the parent_name when it differs from the name,
+            # this can happen after multiple renames on the same collection
+            parent_name = parent_name if name != parent_name else name  # noqa: PLW2901
+            new_name_map[new_name] = parent_name
+
         if len(new_name_map) != len(self._name_map):
             msg = f"non-unique names produced by {renamer=}"
             raise ValueError(msg)
@@ -4280,7 +4322,7 @@ class Alignment(SequenceCollection):
         return {
             "seqs_data": self._seqs_data,
             "moltype": self.moltype,
-            "name_map": self._name_map.copy(),
+            "name_map": dict(self._name_map),
             "info": self.info.copy(),
             "annotation_db": self.annotation_db,
             "slice_record": self._slice_record,


Test patch:
diff --git a/tests/test_core/test_new_alignment.py b/tests/test_core/test_new_alignment.py
index 2e5758a90..9660f8595 100644
--- a/tests/test_core/test_new_alignment.py
+++ b/tests/test_core/test_new_alignment.py
@@ -2457,6 +2457,53 @@ def test_sequence_collection_rename_seqs(mk_cls):
     assert set(new._seqs_data.names) == set(data)
 
 
+@pytest.mark.parametrize(
+    "mk_cls",
+    [new_alignment.make_unaligned_seqs, new_alignment.make_aligned_seqs],
+)
+def test_sequence_collection_rename_seqs_name_map(mk_cls):
+    """successfully rename sequences"""
+    data = {"seq1": "ACGTACGTA", "seq2": "ACCGAA---", "seq3": "ACGTACGTT"}
+    seqs = mk_cls(data, moltype="dna")
+    new = seqs.rename_seqs(lambda x: x.upper())
+    expect = {k.upper(): k for k in data}
+    assert dict(new.name_map) == expect
+
+
+@pytest.mark.parametrize(
+    "mk_cls",
+    [new_alignment.make_unaligned_seqs, new_alignment.make_aligned_seqs],
+)
+def test_sequence_collection_immutable_name_map(mk_cls):
+    """name map attribute is immutable"""
+    data = {"seq1": "ACGTACGTA", "seq2": "ACCGAA---", "seq3": "ACGTACGTT"}
+    seqs = mk_cls(data, moltype="dna")
+    with pytest.raises(TypeError):
+        seqs.name_map = {k.upper(): k for k in data}
+
+
+@pytest.mark.parametrize(
+    "mk_cls",
+    [new_alignment.make_unaligned_seqs, new_alignment.make_aligned_seqs],
+)
+def test_sequence_collection_multiple_rename_seqs_name_map(mk_cls):
+    """parent seq names should remain the same after renames"""
+    data = {"Seq1": "ACGTACGTA", "Seq2": "ACCGAA---", "Seq3": "ACGTACGTT"}
+    seqs = mk_cls(data, moltype="dna")
+    # original name map is the same as the original names
+    expect = {k: k for k in data}
+    assert seqs.name_map == expect
+
+    # subsequently only the keys change in name_map
+    new = seqs.rename_seqs(lambda x: x.upper())
+    expect = {k.upper(): k for k in data}
+    assert new.name_map == expect
+    # and again only the keys change in name_map
+    new = new.rename_seqs(lambda x: x.lower())
+    expect = {k.lower(): k for k in data}
+    assert new.name_map == expect
+
+
 @pytest.mark.parametrize(
     "mk_cls",
     [new_alignment.make_unaligned_seqs, new_alignment.make_aligned_seqs],
@@ -2608,7 +2655,7 @@ def test_sequence_collection_to_rich_dict():
         "version": __version__,
         "init_args": {
             "moltype": seqs.moltype.label,
-            "name_map": seqs._name_map,
+            "name_map": seqs.name_map,
             "info": seqs.info,
             "source": "unknown",
         },
@@ -2626,7 +2673,7 @@ def test_sequence_collection_to_rich_dict_reversed_seqs():
         "seqs": reversed_seqs.to_dict(),
         "init_args": {
             "moltype": seqs.moltype.label,
-            "name_map": seqs._name_map,
+            "name_map": seqs.name_map,
             "info": seqs.info,
             "source": "unknown",
         },
@@ -5266,7 +5313,7 @@ def test_alignment_apply_scaled_gaps_codon2aa_invalid_moltype(codon_and_aa_alns)
 def test_alignment_copy(simple_aln):
     got = simple_aln.copy()
     # mutable data structures should be different IDs
-    assert got._name_map is not simple_aln._name_map
+    assert got.name_map is not simple_aln.name_map
     assert got.info is not simple_aln.info
     assert got.annotation_db is not simple_aln.annotation_db
     # immutable data structures should be the same object
@@ -5295,7 +5342,7 @@ def test_alignment_copy_rc(simple_aln):
 def test_alignment_deepcopy(simple_aln):
     got = simple_aln.deepcopy()
     # all data structures should be different IDs
-    assert got._name_map is not simple_aln._name_map
+    assert got.name_map is not simple_aln.name_map
     assert got.info is not simple_aln.info
     assert got.annotation_db is not simple_aln.annotation_db
     assert got._slice_record is not simple_aln._slice_record
@@ -5430,7 +5477,7 @@ def test_alignment_to_rich_dict_round_trip_renamed(mk_cls):
     renamed_aln = aln.rename_seqs(renamer=lambda x: x.upper())
     rd = renamed_aln.to_rich_dict()
     got = deserialise_object(rd)
-    assert got._name_map == {"SEQ1": "seq1", "SEQ2": "seq2"}
+    assert dict(got.name_map) == {"SEQ1": "seq1", "SEQ2": "seq2"}
     assert got.to_dict() == renamed_aln.to_dict()
     assert got is not renamed_aln
 
diff --git a/tests/test_core/test_new_seq_annotation.py b/tests/test_core/test_new_seq_annotation.py
index f95a51d28..545700809 100644
--- a/tests/test_core/test_new_seq_annotation.py
+++ b/tests/test_core/test_new_seq_annotation.py
@@ -193,7 +193,6 @@ def test_seq_with_masked_annotations():
     shadow = seq.with_masked_annotations(biotypes="CDS", shadow=True)
     expect = "?" * start + raw_seq[start:stop] + "?" * (len(raw_seq) - stop)
     assert str(shadow) == expect
-    print(shadow)
 
 
 @pytest.mark.parametrize("shadow", [True, False])