fix(core): add data type validation when setting topology attributes …

package/AUTHORS

@@ -286,6 +286,7 @@ Chronological list of authors
   - Aryaman Chaudhri
   - Francesco Siciliani
   - Akshit Boora
+  - Dhrub Chatterjee
 
 
 External code

package/CHANGELOG

@@ -23,6 +23,10 @@ The rules for this file:
  * 2.11.0
 
 Fixes
+ * Setting topology attributes (such as atom names, residue IDs, charges)
+   now validates that assigned values match the attribute's expected data type
+   and raises a clear TypeError rather than failing with confusing internal
+   errors or silently coercing invalid types (Issue #2764)
  * Fixed PBC handling in triclinic boxes in `_triclinic_pbc()` (used by
    `distances.apply_PBC()`) and `lib.nsgrid.NSGrid` (used by `capped_distance`
    and `self_capped_distance`):

package/MDAnalysis/core/topologyattrs.py

@@ -85,6 +85,92 @@
 from .. import _TOPOLOGY_ATTRS, _TOPOLOGY_TRANSPLANTS, _TOPOLOGY_ATTRNAMES
 
 
+def _check_dtype(attr, group, values):
+    """Check that values match the expected dtype of the TopologyAttr."""
+    is_component = isinstance(group, ComponentBase)
+    is_iter = iterable(values)
+    attr_name = attr.singular if is_component else attr.attrname
+
+    if (
+        isinstance(attr, _StringInternerMixin)
+        or getattr(attr, "dtype", None) == "U1"
+    ):
+        kind = "str"
+        type_desc = "a string"
+        plural_desc = "strings"
+    else:
+        dtype = getattr(attr, "dtype", None)
+        if dtype is None:
+            return
+        if np.issubdtype(dtype, np.integer):
+            kind = "int"
+            type_desc = "an integer"
+            plural_desc = "integers"
+        elif np.issubdtype(dtype, np.floating):
+            kind = "float"
+            type_desc = "a number"
+            plural_desc = "numbers"
+        elif np.issubdtype(dtype, np.bool_):
+            kind = "bool"
+            type_desc = "a boolean"
+            plural_desc = "booleans"
+        else:
+            return
+
+    if is_iter:
+        if isinstance(values, np.ndarray) and values.dtype != object:
+            if kind == "str":
+                valid = np.issubdtype(values.dtype, np.str_)
+            elif kind == "int":
+                valid = np.issubdtype(
+                    values.dtype, np.integer
+                ) and not np.issubdtype(values.dtype, np.bool_)
+            elif kind == "float":
+                valid = np.issubdtype(
+                    values.dtype, np.number
+                ) and not np.issubdtype(values.dtype, np.bool_)
+            else:  # bool
+                valid = np.issubdtype(values.dtype, np.bool_)
+        else:
+            if kind == "str":
+                valid = all(isinstance(v, str) for v in values)
+            elif kind == "int":
+                valid = all(
+                    isinstance(v, (int, np.integer))
+                    and not isinstance(v, (bool, np.bool_))
+                    for v in values
+                )
+            elif kind == "float":
+                valid = all(
+                    isinstance(v, (int, float, np.number))
+                    and not isinstance(v, (bool, np.bool_))
+                    for v in values
+                )
+            else:  # bool
+                valid = all(isinstance(v, (bool, np.bool_)) for v in values)
+
+        if not valid:
+            raise TypeError(
+                f"All values for {attr_name} must be {plural_desc}"
+            )
+    else:
+        if kind == "str":
+            valid = isinstance(values, str)
+        elif kind == "int":
+            valid = isinstance(values, (int, np.integer)) and not isinstance(
+                values, (bool, np.bool_)
+            )
+        elif kind == "float":
+            valid = isinstance(
+                values, (int, float, np.number)
+            ) and not isinstance(values, (bool, np.bool_))
+        else:  # bool
+            valid = isinstance(values, (bool, np.bool_))
+
+        if not valid:
+            raise TypeError(f"{attr_name} must be {type_desc}")
+
+
 def _check_length(func):
     """Wrapper which checks the length of inputs to set_X
 
@@ -151,6 +237,7 @@ def wrapper(attr, group, values):
                         lenvalues=val_len,
                     )
                 )
+        _check_dtype(attr, group, values)
         # if everything went OK, continue with the function
         return func(attr, group, values)
 
@@ -1947,7 +2034,7 @@ def _gyration(recenteredpos, masses):
                 recenteredpos = atomgroup.positions - com
             eig_vals = _gyration(recenteredpos, masses)
         else:
-            (atom_masks, compound_masks, n_compounds) = (
+            atom_masks, compound_masks, n_compounds = (
                 atomgroup._split_by_compound_indices(compound)
             )
 
@@ -2408,7 +2495,7 @@ def dipole_vector(
                 "ij,ij->j", recenteredpos, charges[:, np.newaxis]
             )
         else:
-            (atom_masks, compound_masks, n_compounds) = (
+            atom_masks, compound_masks, n_compounds = (
                 atomgroup._split_by_compound_indices(compound)
             )
 
@@ -2619,7 +2706,7 @@ def __quadrupole_tensor(recenteredpos, charges):
                 recenteredpos = atomgroup.positions - ref
             quad_tensor = __quadrupole_tensor(recenteredpos, charges)
         else:
-            (atom_masks, compound_masks, n_compounds) = (
+            atom_masks, compound_masks, n_compounds = (
                 atomgroup._split_by_compound_indices(compound)
             )
 

testsuite/MDAnalysisTests/core/test_topologyattrs.py

@@ -20,9 +20,8 @@
 # MDAnalysis: A Toolkit for the Analysis of Molecular Dynamics Simulations.
 # J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787
 #
-"""Tests for MDAnalysis.core.topologyattrs objects.
+"""Tests for MDAnalysis.core.topologyattrs objects."""
 
-"""
 import numpy as np
 
 from numpy.testing import (
@@ -125,8 +124,8 @@ def test_set_atoms_singular(self, attr):
     def test_set_atoms_plural(self, attr):
         # set len 2 Group to len 2 values
         dg = DummyGroup([3, 7])
-        attr.set_atoms(dg, np.array([23, 504]))
-        assert_equal(attr.get_atoms(dg), np.array([23, 504]))
+        attr.set_atoms(dg, self.values[:2])
+        assert_equal(attr.get_atoms(dg), self.values[:2])
 
     def test_set_atoms_VE(self, attr):
         # set len 2 Group to wrong length values
@@ -305,10 +304,8 @@ def test_set_residues_singular(self, attr):
         )
 
     def test_set_residues_plural(self, attr):
-        attr.set_residues(DummyGroup([3, 0, 1]), np.array([23, 504, 2]))
-        assert_almost_equal(
-            attr.get_residues(DummyGroup([3, 0, 1])), np.array([23, 504, 2])
-        )
+        attr.set_residues(DummyGroup([3, 0, 1]), self.values[:3])
+        assert_equal(attr.get_residues(DummyGroup([3, 0, 1])), self.values[:3])
 
     def test_set_residues_VE(self, attr):
         dg = DummyGroup([3, 0, 1])
@@ -786,3 +783,153 @@ def test_coc_unwrap(self, u, compound):
         u.atoms.wrap
         coc = u.atoms[:2].center_of_charge(compound=compound, unwrap=True)
         assert_equal(coc.flatten(), [0, -0.5, 0])
+
+
+class TestDtypeChecks:
+    @pytest.fixture
+    def u(self):
+        universe = mda.Universe.empty(
+            n_atoms=6,
+            n_residues=3,
+            n_segments=2,
+            atom_resindex=[0, 0, 1, 1, 2, 2],
+            residue_segindex=[0, 0, 1],
+        )
+        universe.add_TopologyAttr("names", ["C", "CA", "N", "O", "CB", "CG"])
+        universe.add_TopologyAttr("resnames", ["ALA", "GLU", "VAL"])
+        universe.add_TopologyAttr("segids", ["SEG1", "SEG2"])
+        universe.add_TopologyAttr("resids", [1, 2, 3])
+        universe.add_TopologyAttr("charges", [0.0, 0.0, 0.0, 0.0, 0.0, 0.0])
+        universe.add_TopologyAttr(
+            "masses", [12.0, 12.0, 14.0, 16.0, 12.0, 12.0]
+        )
+        universe.add_TopologyAttr(
+            "aromaticities", [False, False, False, False, False, False]
+        )
+        return universe
+
+    @pytest.mark.parametrize("invalid", [1, 2.5, True, None])
+    def test_string_attr_atom_invalid(self, u, invalid):
+        with pytest.raises(TypeError, match="must be a string"):
+            u.atoms[0].name = invalid
+
+    @pytest.mark.parametrize(
+        "invalid", [[1, 2, 3], ["C", 1, "O"], np.array([1, 2, 3]), 1]
+    )
+    def test_string_attr_group_invalid(self, u, invalid):
+        with pytest.raises(TypeError, match="must be (a string|strings)"):
+            u.atoms[:3].names = invalid
+
+    @pytest.mark.parametrize("invalid", [2.5, "1", True, None])
+    def test_int_attr_residue_invalid(self, u, invalid):
+        with pytest.raises(TypeError, match="must be an integer"):
+            u.residues[0].resid = invalid
+
+    @pytest.mark.parametrize(
+        "invalid",
+        [[1.5, 2.5], np.array([1.5, 2.5]), [1, "2"], [True, False], 2.5],
+    )
+    def test_int_attr_group_invalid(self, u, invalid):
+        with pytest.raises(TypeError, match="must be (an integer|integers)"):
+            u.residues[:2].resids = invalid
+
+    @pytest.mark.parametrize("invalid", ["positive", True, None])
+    def test_float_attr_atom_invalid(self, u, invalid):
+        with pytest.raises(TypeError, match="must be a number"):
+            u.atoms[0].charge = invalid
+
+    @pytest.mark.parametrize(
+        "invalid",
+        [
+            ["a", "b", "c"],
+            [True, False, True],
+            np.array([True, False, True]),
+            "positive",
+        ],
+    )
+    def test_float_attr_group_invalid(self, u, invalid):
+        with pytest.raises(TypeError, match="must be (a number|numbers)"):
+            u.atoms[:3].charges = invalid
+
+    @pytest.mark.parametrize("invalid", [1, 0, "true", None])
+    def test_bool_attr_atom_invalid(self, u, invalid):
+        with pytest.raises(TypeError, match="must be a boolean"):
+            u.atoms[0].aromaticity = invalid
+
+    @pytest.mark.parametrize(
+        "invalid",
+        [[1, 0, 1], ["a", "b", "c"], np.array([1, 0, 1]), 1],
+    )
+    def test_bool_attr_group_invalid(self, u, invalid):
+        with pytest.raises(TypeError, match="must be (a boolean|booleans)"):
+            u.atoms[:3].aromaticities = invalid
+
+    def test_valid_assignments(self, u):
+        u.atoms[0].name = "N"
+        assert u.atoms[0].name == "N"
+
+        u.atoms[:3].names = ["C1", "C2", "C3"]
+        assert list(u.atoms[:3].names) == ["C1", "C2", "C3"]
+
+        u.atoms[:3].names = np.array(["C1", "C2", "C3"])
+        assert list(u.atoms[:3].names) == ["C1", "C2", "C3"]
+
+        u.atoms.names = "CA"
+        assert all(name == "CA" for name in u.atoms.names)
+
+        u.residues[0].resid = 10
+        assert u.residues[0].resid == 10
+
+        u.residues[:2].resids = [100, 200]
+        assert list(u.residues[:2].resids) == [100, 200]
+
+        u.residues[:2].resids = np.array([100, 200])
+        assert list(u.residues[:2].resids) == [100, 200]
+
+        u.residues.resids = 1
+        assert all(r == 1 for r in u.residues.resids)
+
+        # ints should be allowed for float attributes
+        u.atoms[0].charge = 1
+        assert u.atoms[0].charge == 1.0
+
+        u.atoms[:2].charges = [-0.5, 0.5]
+        assert_equal(u.atoms[:2].charges, [-0.5, 0.5])
+
+        u.atoms[:2].charges = np.array([-0.5, 0.5])
+        assert_equal(u.atoms[:2].charges, [-0.5, 0.5])
+
+        u.atoms.charges = 0.0
+        assert all(c == 0.0 for c in u.atoms.charges)
+
+        u.atoms[0].aromaticity = True
+        assert u.atoms[0].aromaticity == True
+
+        u.atoms[:3].aromaticities = [True, False, True]
+        assert list(u.atoms[:3].aromaticities) == [True, False, True]
+
+        u.atoms[:3].aromaticities = np.array([True, False, True])
+        assert list(u.atoms[:3].aromaticities) == [True, False, True]
+
+        u.atoms.aromaticities = True
+        assert all(a == True for a in u.atoms.aromaticities)
+
+    def test_unsupported_or_none_dtype_bypass(self, u):
+        from MDAnalysis.core.topologyattrs import _check_dtype, TopologyAttr
+
+        class DNone(TopologyAttr):
+            attrname = "dn"
+            singular = "d"
+
+            def __init__(self):
+                self.dtype = None
+
+        class DObj(TopologyAttr):
+            attrname = "do"
+            singular = "d"
+
+            def __init__(self):
+                self.dtype = object
+
+        _check_dtype(DNone(), u.atoms, [1, 2])
+        _check_dtype(DObj(), u.atoms, [1, 2])