Skip to content

Commit

Permalink
Homogenize use of boolean properties for Kind and StructureData
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sphuber committed Jan 9, 2019
1 parent e078852 commit dd0b1b9
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 71 deletions.
56 changes: 28 additions & 28 deletions aiida/backends/tests/dataclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand All @@ -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):
"""
Expand All @@ -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):
"""
Expand Down Expand Up @@ -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):
"""
Expand All @@ -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):
"""
Expand Down Expand Up @@ -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
Expand Down
7 changes: 1 addition & 6 deletions aiida/orm/data/array/trajectory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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"
Expand Down
53 changes: 18 additions & 35 deletions aiida/orm/data/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.")

Expand Down Expand Up @@ -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.")

Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions docs/source/examples/structure_tutorial.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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::
Expand Down

0 comments on commit dd0b1b9

Please sign in to comment.