Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions news/deprecate-assignUniqueLabels.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
**Added:**

* No News Added: deprecate assignUniqueLabels method
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this no news. Don't we need to list what we deprecate and what we add?


**Changed:**

* <news item>

**Deprecated:**

* <news item>

**Removed:**

* <news item>

**Fixed:**

* <news item>

**Security:**

* <news item>
1 change: 1 addition & 0 deletions requirements/conda.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
numpy
pycifrw
diffpy.utils
1 change: 1 addition & 0 deletions requirements/pip.txt
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
numpy
pycifrw
diffpy.utils
22 changes: 21 additions & 1 deletion src/diffpy/structure/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
assignUniqueLabels_deprecation_msg = build_deprecation_message(
base,
"assignUniqueLabels",
"assign_unique_labels",
removal_version,
)


class Structure(list):
"""Define group of atoms in a specified lattice. Structure --> group
Expand Down Expand Up @@ -163,7 +173,7 @@ def getLastAtom(self):
last_atom = self[-1]
return last_atom

def assignUniqueLabels(self):
def assign_unique_labels(self):
"""Set a unique label string for each `Atom` in this structure.

The label strings are formatted as "%(baresymbol)s%(index)i",
Expand All @@ -181,6 +191,16 @@ def assignUniqueLabels(self):
islabeled.add(a)
return

@deprecated(assignUniqueLabels_deprecation_msg)
def assignUniqueLabels(self):
"""This function has been deprecated and will be removed in
version 4.0.0.

Please use diffpy.structure.Structure.assign_unique_labels
instead.
"""
return self.assign_unique_labels()

def distance(self, aid0, aid1):
"""Calculate distance between 2 `Atoms`, no periodic boundary
conditions.
Expand Down
21 changes: 16 additions & 5 deletions tests/test_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,18 +121,29 @@ def test___copy__(self):
# return

def test_assignUniqueLabels(self):
"""Check Structure.assignUniqueLabels()"""
"""Duplicate test with the deprecated assignUniqueLabels method.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to do this. This test will start failing when we remove the function so these will be easy to find and remove.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbillinge So, what we need to contain here? should we maintain the original docstring here?


Remove this test in version 4.0.0
"""
self.assertEqual("", "".join([a.label for a in self.stru]))
self.stru.assignUniqueLabels()
self.assertEqual("C1", self.stru[0].label)
self.assertEqual("C2", self.stru[1].label)
return

def test_assign_unique_labels(self):
"""Check Structure.assignUniqueLabels()"""
self.assertEqual("", "".join([a.label for a in self.stru]))
self.stru.assign_unique_labels()
self.assertEqual("C1", self.stru[0].label)
self.assertEqual("C2", self.stru[1].label)
return

def test_distance(self):
"""Check Structure.distance()"""
from math import sqrt

self.stru.assignUniqueLabels()
self.stru.assign_unique_labels()
self.assertRaises(IndexError, self.stru.distance, 333, "C1")
self.assertRaises(IndexError, self.stru.distance, "C", "C1")
self.assertAlmostEqual(sqrt(2.0), self.stru.distance(0, 1), self.places)
Expand All @@ -143,7 +154,7 @@ def test_distance(self):
def test_angle(self):
"""Check Structure.angle()"""
cdse = Structure(filename=self.cdsefile)
cdse.assignUniqueLabels()
cdse.assign_unique_labels()
self.assertEqual(109, round(cdse.angle(0, 2, 1)))
self.assertEqual(109, round(cdse.angle("Cd1", "Se1", "Cd2")))
return
Expand Down Expand Up @@ -236,7 +247,7 @@ def test___getitem__(self):
cdse013.pop(2)
self.assertEqual(cdse013, cdse[:2, 3].tolist())
self.assertRaises(IndexError, cdse.__getitem__, "Cd1")
cdse.assignUniqueLabels()
cdse.assign_unique_labels()
self.assertTrue(cdse[0] is cdse["Cd1"])
cdse[0].label = "Hohenzollern"
self.assertRaises(IndexError, cdse.__getitem__, "Cd1")
Expand Down Expand Up @@ -466,7 +477,7 @@ def test_label(self):
"""Check Structure.label."""
cdse = Structure(filename=self.cdsefile)
self.assertEqual(4 * [""], cdse.label.tolist())
cdse.assignUniqueLabels()
cdse.assign_unique_labels()
self.assertEqual("Cd1 Cd2 Se1 Se2".split(), cdse.label.tolist())
cdse.label = cdse.label.lower()
self.assertEqual("cd1 cd2 se1 se2".split(), cdse.label.tolist())
Expand Down
Loading