diff --git a/news/deprecate-addNewItem.rst b/news/deprecate-addNewItem.rst new file mode 100644 index 0000000..922d41c --- /dev/null +++ b/news/deprecate-addNewItem.rst @@ -0,0 +1,23 @@ +**Added:** + +* Added `diffpy.structure.Structure.add_new_atom` in replace of `addNewAtom` + +**Changed:** + +* + +**Deprecated:** + +* Deprecated `diffpy.structure.Structure.addNewAtom` method for removal in version 4.0.0 + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/requirements/conda.txt b/requirements/conda.txt index 0fdcecb..4c4ff50 100644 --- 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 index 0fdcecb..4c4ff50 100644 --- 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 index 814074f..b542e69 100644 --- 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 index a87f99d..665c096 100644 --- 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 index 73ea41c..9f2ecc4 100644 --- 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 index ae95d6f..84b55fc 100644 --- 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 index 24ad293..ab28ec4 100644 --- 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 index 4f24c42..08c108a 100644 --- 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 index 5c08f99..a91c431 100644 --- 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 index 3c7b725..9e84bc8 100644 --- 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 index 65d028c..648f684 100644 --- 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()