Fix array comparison in core.Structure.merge_sites, also allow int property to be merged instead of float alone, mode only allow full name

src/pymatgen/core/structure.py

@@ -4717,43 +4717,56 @@ def scale_lattice(self, volume: float) -> Self:
         return self
 
     def merge_sites(self, tol: float = 0.01, mode: Literal["sum", "delete", "average"] = "sum") -> Self:
-        """Merges sites (adding occupancies) within tol of each other.
-        Removes site properties.
+        """Merges sites (by adding occupancies) within tolerance and optionally removes
+            site properties in "sum/delete" modes.
 
         Args:
             tol (float): Tolerance for distance to merge sites.
-            mode ("sum" | "delete" | "average"): "delete" means duplicate sites are
-                deleted. "sum" means the occupancies are summed for the sites.
-                "average" means that the site is deleted but the properties are averaged
-                Only first letter is considered.
+            mode ("sum" | "delete" | "average"): Only first letter is considered at this moment.
+                - "delete" means duplicate sites are deleted.
+                - "sum" means the occupancies are summed for the sites.
+                - "average" means that the site is deleted but the properties are averaged.
 
         Returns:
-            Structure: self with merged sites.
+            Structure: Structure with merged sites.
         """
-        dist_mat = self.distance_matrix
+        # TODO: change the code the allow full name after 2025-12-01
+        # TODO2: add a test for mode value, currently it only checks if first letter is "s/a"
+        if mode.lower() not in {"sum", "delete", "average"} and mode.lower()[0] in {"s", "d", "a"}:
+            warnings.warn(
+                "mode would only allow full name sum/delete/average after 2025-12-01", DeprecationWarning, stacklevel=2
+            )
+
+        if mode.lower()[0] not in {"s", "d", "a"}:
+            raise ValueError(f"Illegal {mode=}, should start with a/d/s.")
+
+        dist_mat: NDArray = self.distance_matrix
         np.fill_diagonal(dist_mat, 0)
         clusters = fcluster(linkage(squareform((dist_mat + dist_mat.T) / 2)), tol, "distance")
-        sites = []
+
+        sites: list[PeriodicSite] = []
         for cluster in np.unique(clusters):
             inds = np.where(clusters == cluster)[0]
             species = self[inds[0]].species
             coords = self[inds[0]].frac_coords
             props = self[inds[0]].properties
+
             for n, i in enumerate(inds[1:]):
                 sp = self[i].species
                 if mode.lower()[0] == "s":
                     species += sp
                 offset = self[i].frac_coords - coords
                 coords += ((offset - np.round(offset)) / (n + 2)).astype(coords.dtype)
                 for key in props:
-                    if props[key] is not None and self[i].properties[key] != props[key]:
+                    if props[key] is not None and not np.array_equal(self[i].properties[key], props[key]):
                         if mode.lower()[0] == "a" and isinstance(props[key], float):
                             # update a running total
                             props[key] = props[key] * (n + 1) / (n + 2) + self[i].properties[key] / (n + 2)
                         else:
                             props[key] = None
                             warnings.warn(
-                                f"Sites with different site property {key} are merged. So property is set to none"
+                                f"Sites with different site property {key} are merged. So property is set to none",
+                                stacklevel=2,
                             )
             sites.append(PeriodicSite(species, coords, self.lattice, properties=props))
 

tests/core/test_structure.py

@@ -1633,11 +1633,15 @@ def test_merge_sites(self):
             [0.5, 0.5, 1.501],
         ]
         struct = Structure(Lattice.cubic(1), species, coords)
-        struct.merge_sites(mode="s")
+        struct.merge_sites(mode="sum")
         assert struct[0].specie.symbol == "Ag"
         assert struct[1].species == Composition({"Cl": 0.35, "F": 0.25})
         assert_allclose(struct[1].frac_coords, [0.5, 0.5, 0.5005])
 
+        # Test illegal mode
+        with pytest.raises(ValueError, match="Illegal mode='illegal', should start with a/d/s"):
+            struct.merge_sites(mode="illegal")
+
         # Test for TaS2 with spacegroup 166 in 160 setting.
         lattice = Lattice.hexagonal(3.374351, 20.308941)
         species = ["Ta", "S", "S"]
@@ -1648,7 +1652,7 @@ def test_merge_sites(self):
         ]
         tas2 = Structure.from_spacegroup(160, lattice, species, coords)
         assert len(tas2) == 13
-        tas2.merge_sites(mode="d")
+        tas2.merge_sites(mode="delete")
         assert len(tas2) == 9
 
         lattice = Lattice.hexagonal(3.587776, 19.622793)
@@ -1661,7 +1665,7 @@ def test_merge_sites(self):
         ]
         navs2 = Structure.from_spacegroup(160, lattice, species, coords)
         assert len(navs2) == 18
-        navs2.merge_sites(mode="d")
+        navs2.merge_sites(mode="delete")
         assert len(navs2) == 12
 
         # Test that we can average the site properties that are floats
@@ -1676,7 +1680,7 @@ def test_merge_sites(self):
         site_props = {"prop1": [3.0, 5.0, 7.0, 11.0]}
         navs2 = Structure.from_spacegroup(160, lattice, species, coords, site_properties=site_props)
         navs2.insert(0, "Na", coords[0], properties={"prop1": 100.0})
-        navs2.merge_sites(mode="a")
+        navs2.merge_sites(mode="average")
         assert len(navs2) == 12
         assert 51.5 in [itr.properties["prop1"] for itr in navs2]
 

tests/util/test_misc.py

@@ -46,12 +46,6 @@ def test_nested_arrays(self):
 
     def test_diff_dtype(self):
         """Make sure it also works for other data types as value."""
-
-        @dataclass
-        class CustomClass:
-            name: str
-            value: int
-
         # Test with bool values
         dict1 = {"a": True}
         dict2 = {"a": True}
@@ -69,13 +63,31 @@ class CustomClass:
         assert not is_np_dict_equal(dict4, dict6)
 
         # Test with a custom data class
+        @dataclass
+        class CustomClass:
+            name: str
+            value: int
+
         dict7 = {"a": CustomClass(name="test", value=1)}
         dict8 = {"a": CustomClass(name="test", value=1)}
         assert is_np_dict_equal(dict7, dict8)
 
         dict9 = {"a": CustomClass(name="test", value=2)}
         assert not is_np_dict_equal(dict7, dict9)
 
+        # Test __eq__ method being used
+        @dataclass
+        class NewCustomClass:
+            name: str
+            value: int
+
+            def __eq__(self, other):
+                return True
+
+        dict7_1 = {"a": NewCustomClass(name="test", value=1)}
+        dict8_1 = {"a": NewCustomClass(name="hello", value=2)}
+        assert is_np_dict_equal(dict7_1, dict8_1)
+
         # Test with None
         dict10 = {"a": None}
         dict11 = {"a": None}