chore: deprecated addNewAtom

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>

requirements/conda.txt

@@ -1,2 +1,3 @@
 numpy
 pycifrw
+diffpy.utils

requirements/pip.txt

@@ -1,2 +1,3 @@
 numpy
 pycifrw
+diffpy.utils

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)

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

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

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()

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()

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,

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

src/diffpy/structure/structure.py

@@ -21,9 +21,19 @@
 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 +155,35 @@ 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
+        ------
+        ValueError
+            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):
+                raise ValueError(f"Duplicate atom {atom.element} already exists at {atom.xyz!r}")
+        self.append(atom, copy=False)
         return
 
     def getLastAtom(self):

tests/test_structure.py

@@ -120,6 +120,59 @@ 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.
+
+        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]))