Merge pull request #157 from stevenhua0320/deprecate-addNewItem

sbillinge · web-flow · commit ca9116a7fa13 · 2026-02-19T20:55:49.000-08:00
build: deprecated addNewAtom
diff --git a/news/deprecate-addNewItem.rst b/news/deprecate-addNewItem.rst
@@ -0,0 +1,23 @@
+**Added:**
+
+* Added `diffpy.structure.Structure.add_new_atom` in replace of `addNewAtom`
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* Deprecated `diffpy.structure.Structure.addNewAtom` method for removal in version 4.0.0
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>
diff --git a/requirements/conda.txt b/requirements/conda.txt
@@ -1,2 +1,3 @@
 numpy
 pycifrw
+diffpy.utils
diff --git a/requirements/pip.txt b/requirements/pip.txt
@@ -1,2 +1,3 @@
 numpy
 pycifrw
+diffpy.utils
diff --git a/src/diffpy/structure/parsers/p_cif.py b/src/diffpy/structure/parsers/p_cif.py
@@ -497,7 +497,7 @@ def _parse_atom_site_label(self, block):
             if curlabel == "?":
                 continue
             self.labelindex[curlabel] = len(self.stru)
-            self.stru.addNewAtom()
+            self.stru.add_new_atom()
             a = self.stru.getLastAtom()
             for fset, val in zip(prop_setters, values):
                 fset(a, val)
diff --git a/src/diffpy/structure/parsers/p_discus.py b/src/diffpy/structure/parsers/p_discus.py
@@ -264,7 +264,7 @@ def _parse_atom(self, words):
         element = words[0][0:1].upper() + words[0][1:].lower()
         xyz = [float(w) for w in words[1:4]]
         Biso = float(words[4])
-        self.stru.addNewAtom(element, xyz)
+        self.stru.add_new_atom(element, xyz)
         a = self.stru.getLastAtom()
         a.Bisoequiv = Biso
         return
diff --git a/src/diffpy/structure/parsers/p_pdb.py b/src/diffpy/structure/parsers/p_pdb.py
@@ -197,7 +197,7 @@ def parseLines(self, lines):
                         # get element from the first 2 characters of name
                         element = line[12:14].strip()
                         element = element[0].upper() + element[1:].lower()
-                    stru.addNewAtom(element, occupancy=occupancy, label=name)
+                    stru.add_new_atom(element, occupancy=occupancy, label=name)
                     last_atom = stru.getLastAtom()
                     last_atom.xyz_cartn = rc
                     last_atom.Uisoequiv = uiso
diff --git a/src/diffpy/structure/parsers/p_pdffit.py b/src/diffpy/structure/parsers/p_pdffit.py
@@ -132,7 +132,7 @@ def parseLines(self, lines):
                 element = wl1[0][0].upper() + wl1[0][1:].lower()
                 xyz = [float(w) for w in wl1[1:4]]
                 occ = float(wl1[4])
-                stru.addNewAtom(element, xyz=xyz, occupancy=occ)
+                stru.add_new_atom(element, xyz=xyz, occupancy=occ)
                 a = stru.getLastAtom()
                 p_nl += 1
                 wl2 = next(ilines).split()
diff --git a/src/diffpy/structure/parsers/p_rawxyz.py b/src/diffpy/structure/parsers/p_rawxyz.py
@@ -103,7 +103,7 @@ def parseLines(self, lines):
                 xyz = [float(f) for f in fields[x_idx : x_idx + 3]]
                 if len(xyz) == 2:
                     xyz.append(0.0)
-                stru.addNewAtom(element, xyz=xyz)
+                stru.add_new_atom(element, xyz=xyz)
         except ValueError:
             emsg = "%d: invalid number" % p_nl
             exc_type, exc_value, exc_traceback = sys.exc_info()
diff --git a/src/diffpy/structure/parsers/p_xcfg.py b/src/diffpy/structure/parsers/p_xcfg.py
@@ -269,7 +269,7 @@ def parseLines(self, lines):
                 elif len(words) == xcfg_entry_count and p_element is not None:
                     fields = [float(w) for w in words]
                     xyz = [xcfg_A * xi for xi in fields[:3]]
-                    stru.addNewAtom(p_element, xyz=xyz)
+                    stru.add_new_atom(p_element, xyz=xyz)
                     a = stru[-1]
                     _assign_auxiliaries(
                         a,
diff --git a/src/diffpy/structure/parsers/p_xyz.py b/src/diffpy/structure/parsers/p_xyz.py
@@ -109,7 +109,7 @@ def parseLines(self, lines):
                 element = fields[0]
                 element = element[0].upper() + element[1:].lower()
                 xyz = [float(f) for f in fields[1:4]]
-                stru.addNewAtom(element, xyz=xyz)
+                stru.add_new_atom(element, xyz=xyz)
         except ValueError:
             exc_type, exc_value, exc_traceback = sys.exc_info()
             emsg = "%d: invalid number format" % p_nl
diff --git a/src/diffpy/structure/structure.py b/src/diffpy/structure/structure.py
@@ -15,15 +15,26 @@
 """This module defines class `Structure`."""
 
 import copy as copymod
+import warnings
 
 import numpy
 
 from diffpy.structure.atom import Atom
 from diffpy.structure.lattice import Lattice
 from diffpy.structure.utils import _linkAtomAttribute, atomBareSymbol, isiterable
+from diffpy.utils._deprecator import build_deprecation_message, deprecated
 
 # ----------------------------------------------------------------------------
 
+base = "diffpy.structure.Structure"
+removal_version = "4.0.0"
+addNewAtom_deprecation_msg = build_deprecation_message(
+    base,
+    "addNewAtom",
+    "add_new_atom",
+    removal_version,
+)
+
 
 class Structure(list):
     """Define group of atoms in a specified lattice. Structure --> group
@@ -145,17 +156,40 @@ def __str__(self):
         s_atoms = "\n".join([str(a) for a in self])
         return s_lattice + "\n" + s_atoms
 
+    @deprecated(addNewAtom_deprecation_msg)
     def addNewAtom(self, *args, **kwargs):
+        """This function has been deprecated and will be removed in
+        version 4.0.0.
+
+        Please use diffpy.structure.Structure.add_new_atom instead.
+        """
+        self.add_new_atom(*args, **kwargs)
+        return
+
+    def add_new_atom(self, *args, **kwargs):
         """Add new `Atom` instance to the end of this `Structure`.
 
         Parameters
         ----------
         *args, **kwargs :
             See `Atom` class constructor.
+
+        Raises
+        ------
+        UserWarning
+            If an atom with the same element/type and coordinates already exists.
         """
         kwargs["lattice"] = self.lattice
-        a = Atom(*args, **kwargs)
-        self.append(a, copy=False)
+        atom = Atom(*args, **kwargs)
+        for existing in self:
+            if existing.element == atom.element and numpy.allclose(existing.xyz, atom.xyz):
+                warnings.warn(
+                    f"Duplicate atom {atom.element} already exists at {atom.xyz!r}",
+                    category=UserWarning,
+                    stacklevel=2,
+                )
+                break
+        self.append(atom, copy=False)
         return
 
     def getLastAtom(self):
diff --git a/tests/test_structure.py b/tests/test_structure.py
@@ -120,6 +120,23 @@ def test___copy__(self):
     #     """check Structure.getLastAtom()"""
     #     return
 
+    def test_addNewAtom(self):
+        """Duplicate test for the deprecated addNewAtom method.
+
+        Remove this test in version 4.0.0
+        """
+        s_lat = Lattice()
+        structure = Structure(lattice=s_lat)
+
+        length = len(structure)
+        structure.addNewAtom(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])
+
     def test_assignUniqueLabels(self):
         """Check Structure.assignUniqueLabels()"""
         self.assertEqual("", "".join([a.label for a in self.stru]))
@@ -601,7 +618,59 @@ 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 to be added and gives a UserWarning.
+    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()

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
`1`	`1`	`numpy`
`2`	`2`	`pycifrw`
	`3`	`+diffpy.utils`