fix: change behavior to warning and use pytest parametrize to test it.

Author: stevenhua0320

src/diffpy/structure/structure.py

@@ -17,6 +17,7 @@
 import copy as copymod
 
 import numpy
+import warnings
 
 from diffpy.structure.atom import Atom
 from diffpy.structure.lattice import Lattice
@@ -175,14 +176,19 @@ def add_new_atom(self, *args, **kwargs):
 
         Raises
         ------
-        ValueError
+        UserWarning
             If an atom with the same element/type and coordinates already exists.
         """
         kwargs["lattice"] = self.lattice
         atom = Atom(*args, **kwargs)
         for existing in self:
             if existing.element == atom.element and numpy.allclose(existing.xyz, atom.xyz):
-                raise ValueError(f"Duplicate atom {atom.element} already exists at {atom.xyz!r}")
+                warnings.warn(
+                    f"Duplicate atom {atom.element} already exists at {atom.xyz!r}",
+                    category=UserWarning,
+                    stacklevel=2,
+                )
+                break
         self.append(atom, copy=False)
         return
 

tests/test_structure.py

@@ -120,41 +120,7 @@ def test___copy__(self):
     #     """check Structure.getLastAtom()"""
     #     return
 
-    def test_add_new_atom(self):
-        """Check Structure.add_new_item()"""
-        # Case 1: valid atom added to an empty structure.
-        # Expect the atom list length to go from 0 to 1.
-        # Expect the atom attributes are successfully loaded.
-        s_lat = Lattice()
-        structure = Structure(lattice=s_lat)
-        length = len(structure)
-        structure.add_new_atom(atype="C", xyz=[0.1, 0.2, 0.3])
-        expected = len(structure)
-        actual = length + 1
-        assert expected == actual
-        atom_object = structure[-1]
-        assert atom_object.element == "C"
-        assert numpy.allclose(atom_object.xyz, [0.1, 0.2, 0.3])
 
-        # Case 2: valid atom added to existing atom list.
-        # Expect the atom list length to go from 1 to 2.
-        # Expect the atom attributes are successfully loaded.
-        length = len(structure)
-        structure.add_new_atom(atype="Ni", xyz=[0.8, 1.2, 0.9])
-        expected = len(structure)
-        actual = length + 1
-        assert expected == actual
-        atom_object = structure[-1]
-        assert atom_object.element == "Ni"
-        assert numpy.allclose(atom_object.xyz, [0.8, 1.2, 0.9])
-
-        # Case 3: duplicated atom added to the existing atom list.
-        # Expect the atom not to be added and gives a ValueError.
-        with pytest.raises(ValueError, match=r"Duplicate atom C already exists at array\(\[0\.1, 0\.2, 0\.3\]\)"):
-            structure.add_new_atom(atype="C", xyz=[0.1, 0.2, 0.3])
-        actual = len(structure)
-        expected = 2
-        assert expected == actual
 
     def test_addNewAtom(self):
         """Duplicate test for the deprecated addNewAtom method.
@@ -655,6 +621,56 @@ def test_pickling(self):
 # End of class TestStructure
 
 # ----------------------------------------------------------------------------
+@pytest.mark.parametrize(
+    "existing, atype, xyz, expected_len, expected_element, expected_xyz",
+    [
+        # Case 1: valid atom added to an empty structure.
+        # Expect the atom list length to go from 0 to 1.
+        # Expect the atom attributes are successfully loaded.
+        (
+            None,
+            "C",
+            [0.1, 0.2, 0.3],
+            1,
+            "C",
+            [0.1, 0.2, 0.3],
+        ),
+        # Case 2: valid atom added to existing atom list.
+        # Expect the atom list length to go from 1 to 2.
+        # Expect the atom attributes are successfully loaded.
+        (
+            [Atom("C", [0, 0, 0])],
+            "Ni",
+            [0.8, 1.2, 0.9],
+            2,
+            "Ni",
+            [0.8, 1.2, 0.9],
+        ),
+    ],
+)
+def test_add_new_atom(existing, atype, xyz, expected_len, expected_element, expected_xyz):
+    """Check Structure.add_new_item()"""
+    structure = Structure(existing, lattice=Lattice())
+    structure.add_new_atom(atype=atype, xyz=xyz)
+    actual_length = len(structure)
+    assert expected_len == actual_length
+    atom_object = structure[-1]
+    assert atom_object.element == expected_element
+    assert numpy.allclose(atom_object.xyz, expected_xyz)
+
+
+def test_add_new_atom_duplicate():
+    # Case 3: duplicated atom added to the existing atom list.
+    # Expect the atom not to be added and gives a ValueError.
+    structure = Structure(
+        [Atom("C", [0.1, 0.2, 0.3]), Atom("Ni", [0.8, 1.2, 0.9])],
+        lattice=Lattice(),
+    )
+    with pytest.warns(UserWarning):
+        structure.add_new_atom(atype="C", xyz=[0.1, 0.2, 0.3])
+    assert len(structure) == 3
+    assert structure[-1].element == "C"
+    assert numpy.allclose(structure[-1].xyz, [0.1, 0.2, 0.3])
 
 if __name__ == "__main__":
     unittest.main()