From dd0b1b9c80bfd3142949c37b4fbb4511ddbdcfd1 Mon Sep 17 00:00:00 2001 From: Sebastiaan Huber Date: Wed, 9 Jan 2019 16:33:13 +0100 Subject: [PATCH] Homogenize use of boolean properties for `Kind` and `StructureData` The `Kind` and `StructureData` were using an inconsisten mixture of methods and properties for boolean attributes, such as `has_vacancies` and `is_alloy`. To conform to `CifData` these are now all turned into properties. --- aiida/backends/tests/dataclasses.py | 56 ++++++++++----------- aiida/orm/data/array/trajectory.py | 7 +-- aiida/orm/data/structure.py | 53 +++++++------------ docs/source/examples/structure_tutorial.rst | 4 +- 4 files changed, 49 insertions(+), 71 deletions(-) diff --git a/aiida/backends/tests/dataclasses.py b/aiida/backends/tests/dataclasses.py index b2967d37e3..9e25c4174c 100644 --- a/aiida/backends/tests/dataclasses.py +++ b/aiida/backends/tests/dataclasses.py @@ -989,8 +989,8 @@ def test_sum_one_general(self): from aiida.orm.data.structure import Kind a = Kind(symbols=['Ba', 'C'], weights=[1. / 3., 2. / 3.]) - self.assertTrue(a.is_alloy()) - self.assertFalse(a.has_vacancies()) + self.assertTrue(a.is_alloy) + self.assertFalse(a.has_vacancies) def test_sum_less_one_general(self): """ @@ -999,8 +999,8 @@ def test_sum_less_one_general(self): from aiida.orm.data.structure import Kind a = Kind(symbols=['Ba', 'C'], weights=[1. / 3., 1. / 3.]) - self.assertTrue(a.is_alloy()) - self.assertTrue(a.has_vacancies()) + self.assertTrue(a.is_alloy) + self.assertTrue(a.has_vacancies) def test_no_position(self): """ @@ -1018,16 +1018,16 @@ def test_simple(self): from aiida.orm.data.structure import Kind a = Kind(symbols='Ba') - self.assertFalse(a.is_alloy()) - self.assertFalse(a.has_vacancies()) + self.assertFalse(a.is_alloy) + self.assertFalse(a.has_vacancies) b = Kind(symbols='Ba', weights=1.) - self.assertFalse(b.is_alloy()) - self.assertFalse(b.has_vacancies()) + self.assertFalse(b.is_alloy) + self.assertFalse(b.has_vacancies) c = Kind(symbols='Ba', weights=None) - self.assertFalse(c.is_alloy()) - self.assertFalse(c.has_vacancies()) + self.assertFalse(c.is_alloy) + self.assertFalse(c.has_vacancies) def test_automatic_name(self): """ @@ -1253,24 +1253,24 @@ def test_cell_ok_and_atoms(self): a.append_atom(position=(0., 0., 0.), symbols=['Ba']) a.append_atom(position=(1., 1., 1.), symbols=['Ti']) a.append_atom(position=(1.2, 1.4, 1.6), symbols=['Ti']) - self.assertFalse(a.is_alloy()) - self.assertFalse(a.has_vacancies()) + self.assertFalse(a.is_alloy) + self.assertFalse(a.has_vacancies) # There should be only two kinds! (two atoms of kind Ti should # belong to the same kind) self.assertEquals(len(a.kinds), 2) a.append_atom(position=(0.5, 1., 1.5), symbols=['O', 'C'], weights=[0.5, 0.5]) - self.assertTrue(a.is_alloy()) - self.assertFalse(a.has_vacancies()) + self.assertTrue(a.is_alloy) + self.assertFalse(a.has_vacancies) a.append_atom(position=(0.5, 1., 1.5), symbols=['O'], weights=[0.5]) - self.assertTrue(a.is_alloy()) - self.assertTrue(a.has_vacancies()) + self.assertTrue(a.is_alloy) + self.assertTrue(a.has_vacancies) a.clear_kinds() a.append_atom(position=(0.5, 1., 1.5), symbols=['O'], weights=[0.5]) - self.assertFalse(a.is_alloy()) - self.assertTrue(a.has_vacancies()) + self.assertFalse(a.is_alloy) + self.assertTrue(a.has_vacancies) def test_cell_ok_and_unknown_atoms(self): """ @@ -1288,24 +1288,24 @@ def test_cell_ok_and_unknown_atoms(self): a.append_atom(position=(0., 0., 0.), symbols=['Ba']) a.append_atom(position=(1., 1., 1.), symbols=['X']) a.append_atom(position=(1.2, 1.4, 1.6), symbols=['X']) - self.assertFalse(a.is_alloy()) - self.assertFalse(a.has_vacancies()) + self.assertFalse(a.is_alloy) + self.assertFalse(a.has_vacancies) # There should be only two kinds! (two atoms of kind X should # belong to the same kind) self.assertEquals(len(a.kinds), 2) a.append_atom(position=(0.5, 1., 1.5), symbols=['O', 'C'], weights=[0.5, 0.5]) - self.assertTrue(a.is_alloy()) - self.assertFalse(a.has_vacancies()) + self.assertTrue(a.is_alloy) + self.assertFalse(a.has_vacancies) a.append_atom(position=(0.5, 1., 1.5), symbols=['O'], weights=[0.5]) - self.assertTrue(a.is_alloy()) - self.assertTrue(a.has_vacancies()) + self.assertTrue(a.is_alloy) + self.assertTrue(a.has_vacancies) a.clear_kinds() a.append_atom(position=(0.5, 1., 1.5), symbols=['X'], weights=[0.5]) - self.assertFalse(a.is_alloy()) - self.assertTrue(a.has_vacancies()) + self.assertFalse(a.is_alloy) + self.assertTrue(a.has_vacancies) def test_kind_1(self): """ @@ -2043,8 +2043,8 @@ def test_lock(self): a.pbc = [True, True, True] _ = a.get_cell_volume() - _ = a.is_alloy() - _ = a.has_vacancies() + _ = a.is_alloy + _ = a.has_vacancies b = a.clone() # I check that clone returned an unstored copy and so can be altered diff --git a/aiida/orm/data/array/trajectory.py b/aiida/orm/data/array/trajectory.py index 1b4b0d62db..a4971b248f 100644 --- a/aiida/orm/data/array/trajectory.py +++ b/aiida/orm/data/array/trajectory.py @@ -403,7 +403,7 @@ def _prepare_xsf(self, index=None, main_file_name=""): # pylint: disable=unused return_string = "ANIMSTEPS {}\nCRYSTAL\n".format(len(indices)) # Do the checks once and for all here: structure = self.get_step_structure(index=0) - if structure.is_alloy() or structure.has_vacancies(): + if structure.is_alloy or structure.has_vacancies: raise NotImplementedError("XSF for alloys or systems with vacancies not implemented.") cells = self.get_cells() positions = self.get_positions() @@ -413,11 +413,6 @@ def _prepare_xsf(self, index=None, main_file_name=""): # pylint: disable=unused for idx in indices: return_string += "PRIMVEC {}\n".format(idx + 1) - #~ structure = self.get_step_structure(index=idx) - #~ sites = structure.sites - #~ if structure.is_alloy() or structure.has_vacancies(): - #~ raise NotImplementedError("XSF for alloys or systems with " - #~ "vacancies not implemented.") for cell_vector in cells[idx]: return_string += ' '.join(["{:18.5f}".format(i) for i in cell_vector]) return_string += "\n" diff --git a/aiida/orm/data/structure.py b/aiida/orm/data/structure.py index 2c0500592f..30c1ada369 100644 --- a/aiida/orm/data/structure.py +++ b/aiida/orm/data/structure.py @@ -997,7 +997,7 @@ def _prepare_xsf(self, main_file_name=""): """ Write the given structure to a string of format XSF (for XCrySDen). """ - if self.is_alloy() or self.has_vacancies(): + if self.is_alloy or self.has_vacancies: raise NotImplementedError("XSF for alloys or systems with " "vacancies not implemented.") @@ -1111,7 +1111,7 @@ def _prepare_xyz(self, main_file_name=""): """ Write the given structure to a string of format XYZ. """ - if self.is_alloy() or self.has_vacancies(): + if self.is_alloy or self.has_vacancies: raise NotImplementedError("XYZ for alloys or systems with " "vacancies not implemented.") @@ -1857,21 +1857,21 @@ def cell_angles(self, value): def set_cell_angles(self, value): raise NotImplementedError("Modification is not implemented yet") + @property def is_alloy(self): - """ - To understand if there are alloys in the structure. + """Return whether the structure contains any alloy kinds. :return: a boolean, True if at least one kind is an alloy """ - return any(s.is_alloy() for s in self.kinds) + return any(kind.is_alloy for kind in self.kinds) + @property def has_vacancies(self): - """ - To understand if there are vacancies in the structure. + """Return whether the structure has vacancies in the structure. :return: a boolean, True if at least one kind has a vacancy """ - return any(s.has_vacancies() for s in self.kinds) + return any(kind.has_vacancies for kind in self.kinds) def get_cell_volume(self): """ @@ -2186,23 +2186,6 @@ def get_raw(self): 'name': self.name, } - # def get_ase(self): - - # """ - # Return a ase.Atom object for this kind, setting the position to - # the origin. - # - # Note: If any site is an alloy or has vacancies, a ValueError is - # raised (from the site.get_ase() routine). - # """ - # import ase - # if self.is_alloy() or self.has_vacancies(): - # raise ValueError("Cannot convert to ASE if the site is an alloy " - # "or has vacancies.") - # aseatom = ase.Atom(position=[0.,0.,0.], symbol=self.symbols[0], - # mass=self.mass) - # return aseatom - def reset_mass(self): """ Reset the mass to the automatic calculated value. @@ -2421,21 +2404,21 @@ def set_symbols_and_weights(self, symbols, weights): self._symbols = symbols_tuple self._weights = weights_tuple + @property def is_alloy(self): - """ - To understand if kind is an alloy. + """Return whether the Kind is an alloy, i.e. contains more than one element - :return: True if the kind has more than one element (i.e., - len(self.symbols) != 1), False otherwise. + :return: boolean, True if the kind has more than one element, False otherwise. """ return len(self._symbols) != 1 + @property def has_vacancies(self): - """ - Returns True if the sum of the weights is less than one. - It uses the internal variable _sum_threshold as a threshold. + """Return whether the Kind contains vacancies, i.e. when the sum of the weights is less than one. + + .. note:: the property uses the internal variable `_sum_threshold` as a threshold. - :return: a boolean + :return: boolean, True if the sum of the weights is less than one, False otherwise """ return has_vacancies(self._weights) @@ -2530,7 +2513,7 @@ def get_ase(self, kinds): used_tags = defaultdict(list) for k in kinds: # Skip alloys and vacancies - if k.is_alloy() or k.has_vacancies(): + if k.is_alloy or k.has_vacancies: tag_list.append(None) # If the kind name is equal to the specie name, # then no tag should be set @@ -2575,7 +2558,7 @@ def get_ase(self, kinds): raise ValueError("No kind '{}' has been found in the list of kinds" "".format(self.kind_name)) - if kind.is_alloy() or kind.has_vacancies(): + if kind.is_alloy or kind.has_vacancies: raise ValueError("Cannot convert to ASE if the kind represents " "an alloy or it has vacancies.") aseatom = ase.Atom(position=self.position, diff --git a/docs/source/examples/structure_tutorial.rst b/docs/source/examples/structure_tutorial.rst index b81b81fb9e..a20010759b 100644 --- a/docs/source/examples/structure_tutorial.rst +++ b/docs/source/examples/structure_tutorial.rst @@ -80,11 +80,11 @@ The following line instead:: would create a site with 90% probability of being occupied by Calcium, and 10% of being a vacancy. -Utility methods ``s.is_alloy()`` and ``s.has_vacancies()`` can be used to +Utility methods ``s.is_alloy`` and ``s.has_vacancies`` can be used to verify, respectively, if more than one element if given in the symbols list, and if the sum of all weights is smaller than one. -.. note:: if you pass more than one symbol, the method ``s.is_alloy()`` will +.. note:: if you pass more than one symbol, the method ``s.is_alloy`` will always return ``True``, even if only one symbol has occupancy 1. and all others have occupancy zero::