Skip to content

Commit

Permalink
Introduce more intuitive method names in Data classes and reduce requ…
Browse files Browse the repository at this point in the history
…ired input parameter for TrajectoryData.set_trajectory() method (aiidateam#2310)

* Rename ArrayData.iterarrays() to ArrayData.get_iterarrays()

* [TrajectoryData] Rename _get_aiida_structure() -> get_structure()

* [TrajectoryData] Rename _get_cif() -> get_cif()

* Rename _get_cif() -> get_cit() in tcod.py

* [StructureData] Rename _get_cif() -> get_cif()

* Make stepid and cells parameter optional
  Change TrajectoryData to make passing stepids and cells optional. While
  nothing will be stored for cells if not given a consecutive sequence will be
  automatically assigned to stepids if missing from the inputs.

* Store symbols in TrajectoryData attribute instead of array
  As proposed in issue aiidateam#201 symbols are now stored in a
  TrajectoryData attribute rather than an array to simplify the
  query for these symbols

* Change passed symbols from numpy.ndarray to list in set_structurelist

* Change TrajectoryData tests to use symbols attribute rather than symbols array

* Make Code.is_hidden() available as class property

* Rename Code.full_text_info() method to .get_full_text_info()

* Change RemoteData.is_empty() method to class property .is_empty

* Change is_alloy() and has_vacancies() methods to properties

  Make class methods .is_alloy() and .has_vacancies() for StrutureData
  and Kind classes accessible as class properties .is_alloy and
  .has_vacancies

* Change symbols type from numpy.array to list

* Add deprecation warning to renamed methods

* Rename code attribute is_hidden to hidden.

* Set allowed symbols type to be Iterable rather than list

* Ensure symbols are stored as list

* Adjust argument ordering in set_trajectory() docstring.

* Document introduced changes in concepts/workflows
  • Loading branch information
astamminger authored and elsapassaro committed Jan 25, 2019
1 parent 9b0b8a3 commit 85c6cc6
Show file tree
Hide file tree
Showing 16 changed files with 245 additions and 101 deletions.
4 changes: 2 additions & 2 deletions aiida/backends/tests/cmdline/commands/test_code.py
Expand Up @@ -146,13 +146,13 @@ def test_hide_one(self):
result = self.cli_runner.invoke(hide, [str(self.code.pk)])
self.assertIsNone(result.exception, result.output)

self.assertTrue(self.code.is_hidden())
self.assertTrue(self.code.hidden)

def test_reveal_one(self):
result = self.cli_runner.invoke(reveal, [str(self.code.pk)])
self.assertIsNone(result.exception, result.output)

self.assertFalse(self.code.is_hidden())
self.assertFalse(self.code.hidden)

def test_relabel_code(self):
result = self.cli_runner.invoke(relabel, [str(self.code.pk), 'new_code'])
Expand Down
2 changes: 1 addition & 1 deletion aiida/backends/tests/cmdline/commands/test_data.py
Expand Up @@ -518,7 +518,7 @@ def create_trajectory_data():
0.,
3.,
]]])
symbols = numpy.array(['H', 'O', 'C'])
symbols = ['H', 'O', 'C']
positions = numpy.array([[[0., 0., 0.], [0.5, 0.5, 0.5], [1.5, 1.5, 1.5]], [[0., 0., 0.], [0.5, 0.5, 0.5],
[1.5, 1.5, 1.5]]])
velocities = numpy.array([[[0., 0., 0.], [0., 0., 0.], [0., 0., 0.]], [[0.5, 0.5, 0.5], [0.5, 0.5, 0.5],
Expand Down
2 changes: 1 addition & 1 deletion aiida/backends/tests/cmdline/commands/test_node.py
Expand Up @@ -518,7 +518,7 @@ def create_trajectory_data():
0.,
3.,
]]])
symbols = numpy.array(['H', 'O', 'C'])
symbols = ['H', 'O', 'C']
positions = numpy.array([[[0., 0., 0.], [0.5, 0.5, 0.5], [1.5, 1.5, 1.5]], [[0., 0., 0.], [0.5, 0.5, 0.5],
[1.5, 1.5, 1.5]]])
velocities = numpy.array([[[0., 0., 0.], [0., 0., 0.], [0., 0., 0.]], [[0.5, 0.5, 0.5], [0.5, 0.5, 0.5],
Expand Down
62 changes: 31 additions & 31 deletions aiida/backends/tests/dataclasses.py
Expand Up @@ -256,7 +256,7 @@ def test_change_cifdata_file(self):

@unittest.skipIf(not has_ase(), "Unable to import ase")
@unittest.skipIf(not has_pycifrw(), "Unable to import PyCifRW")
def test_get_aiida_structure(self):
def test_get_structure(self):
import tempfile

from aiida.orm.data.cif import CifData
Expand Down Expand Up @@ -286,9 +286,9 @@ def test_get_aiida_structure(self):
a = CifData(file=tmpf.name)

with self.assertRaises(ValueError):
a._get_aiida_structure(converter='none')
a.get_structure(converter='none')

c = a._get_aiida_structure()
c = a.get_structure()

self.assertEquals(c.get_kind_names(), ['C', 'O'])

Expand Down Expand Up @@ -330,13 +330,13 @@ def test_ase_primitive_and_conventional_cells_ase(self):
tmpf.flush()
c = CifData(file=tmpf.name)

ase = c._get_aiida_structure(converter='ase', primitive_cell=False).get_ase()
ase = c.get_structure(converter='ase', primitive_cell=False).get_ase()
self.assertEquals(ase.get_number_of_atoms(), 15)

ase = c._get_aiida_structure(converter='ase').get_ase()
ase = c.get_structure(converter='ase').get_ase()
self.assertEquals(ase.get_number_of_atoms(), 15)

ase = c._get_aiida_structure(converter='ase', primitive_cell=True, subtrans_included=False).get_ase()
ase = c.get_structure(converter='ase', primitive_cell=True, subtrans_included=False).get_ase()
self.assertEquals(ase.get_number_of_atoms(), 5)

@unittest.skipIf(not has_ase(), "Unable to import ase")
Expand Down Expand Up @@ -389,13 +389,13 @@ def test_ase_primitive_and_conventional_cells_pymatgen(self):
tmpf.flush()
c = CifData(file=tmpf.name)

ase = c._get_aiida_structure(converter='pymatgen', primitive_cell=False).get_ase()
ase = c.get_structure(converter='pymatgen', primitive_cell=False).get_ase()
self.assertEquals(ase.get_number_of_atoms(), 15)

ase = c._get_aiida_structure(converter='pymatgen').get_ase()
ase = c.get_structure(converter='pymatgen').get_ase()
self.assertEquals(ase.get_number_of_atoms(), 15)

ase = c._get_aiida_structure(converter='pymatgen', primitive_cell=True).get_ase()
ase = c.get_structure(converter='pymatgen', primitive_cell=True).get_ase()
self.assertEquals(ase.get_number_of_atoms(), 5)

@unittest.skipIf(not has_pycifrw(), "Unable to import PyCifRW")
Expand Down Expand Up @@ -1899,7 +1899,7 @@ def test_get_cif(self):
a.append_atom(position=(0.5, 0.5, 0.5), symbols=['Ba'])
a.append_atom(position=(1., 1., 1.), symbols=['Ti'])

c = a._get_cif()
c = a.get_cif()
lines = c._prepare_cif()[0].decode('utf-8').split('\n')
non_comments = []
for line in lines:
Expand Down Expand Up @@ -2891,7 +2891,7 @@ def test_creation(self):

def test_iteration(self):
"""
Check the functionality of the iterarrays() iterator
Check the functionality of the get_iterarrays() iterator
"""
from aiida.orm.data.array import ArrayData
import numpy
Expand All @@ -2907,7 +2907,7 @@ def test_iteration(self):
third = numpy.random.rand(6, 6)
n.set_array('third', third)

for name, array in n.iterarrays():
for name, array in n.get_iterarrays():
if name == 'first':
self.assertAlmostEquals(abs(first - array).max(), 0.)
if name == 'second':
Expand Down Expand Up @@ -2959,7 +2959,7 @@ def test_creation(self):
0.,
3.,
]]])
symbols = numpy.array(['H', 'O', 'C'])
symbols = ['H', 'O', 'C']
positions = numpy.array([[[0., 0., 0.], [0.5, 0.5, 0.5], [1.5, 1.5, 1.5]], [[0., 0., 0.], [0.5, 0.5, 0.5],
[1.5, 1.5, 1.5]]])
velocities = numpy.array([[[0., 0., 0.], [0., 0., 0.], [0., 0., 0.]], [[0.5, 0.5, 0.5], [0.5, 0.5, 0.5],
Expand All @@ -2975,7 +2975,7 @@ def test_creation(self):
self.assertAlmostEqual(abs(stepids - n.get_stepids()).sum(), 0.)
self.assertAlmostEqual(abs(times - n.get_times()).sum(), 0.)
self.assertAlmostEqual(abs(cells - n.get_cells()).sum(), 0.)
self.assertEqual(symbols.tolist(), n.get_symbols().tolist())
self.assertEqual(symbols, n.symbols)
self.assertAlmostEqual(abs(positions - n.get_positions()).sum(), 0.)
self.assertAlmostEqual(abs(velocities - n.get_velocities()).sum(), 0.)

Expand All @@ -2984,7 +2984,7 @@ def test_creation(self):
self.assertEqual(data[0], stepids[1])
self.assertAlmostEqual(data[1], times[1])
self.assertAlmostEqual(abs(cells[1] - data[2]).sum(), 0.)
self.assertEqual(symbols.tolist(), data[3].tolist())
self.assertEqual(symbols, data[3])
self.assertAlmostEqual(abs(data[4] - positions[1]).sum(), 0.)
self.assertAlmostEqual(abs(data[5] - velocities[1]).sum(), 0.)

Expand All @@ -3003,7 +3003,7 @@ def test_creation(self):
self.assertAlmostEqual(abs(stepids - n.get_stepids()).sum(), 0.)
self.assertIsNone(n.get_times())
self.assertAlmostEqual(abs(cells - n.get_cells()).sum(), 0.)
self.assertEqual(symbols.tolist(), n.get_symbols().tolist())
self.assertEqual(symbols, n.symbols)
self.assertAlmostEqual(abs(positions - n.get_positions()).sum(), 0.)
self.assertIsNone(n.get_velocities())

Expand All @@ -3016,7 +3016,7 @@ def test_creation(self):
self.assertAlmostEqual(abs(stepids - n.get_stepids()).sum(), 0.)
self.assertIsNone(n.get_times())
self.assertAlmostEqual(abs(cells - n.get_cells()).sum(), 0.)
self.assertEqual(symbols.tolist(), n.get_symbols().tolist())
self.assertEqual(symbols, n.symbols)
self.assertAlmostEqual(abs(positions - n.get_positions()).sum(), 0.)
self.assertIsNone(n.get_velocities())

Expand All @@ -3029,7 +3029,7 @@ def test_creation(self):
self.assertAlmostEqual(abs(stepids - n.get_stepids()).sum(), 0.)
self.assertAlmostEqual(abs(times - n.get_times()).sum(), 0.)
self.assertAlmostEqual(abs(cells - n.get_cells()).sum(), 0.)
self.assertEqual(symbols.tolist(), n.get_symbols().tolist())
self.assertEqual(symbols, n.symbols)
self.assertAlmostEqual(abs(positions - n.get_positions()).sum(), 0.)
self.assertIsNone(n.get_velocities())

Expand All @@ -3042,7 +3042,7 @@ def test_creation(self):
self.assertAlmostEqual(abs(stepids - n.get_stepids()).sum(), 0.)
self.assertAlmostEqual(abs(times - n.get_times()).sum(), 0.)
self.assertAlmostEqual(abs(cells - n.get_cells()).sum(), 0.)
self.assertEqual(symbols.tolist(), n.get_symbols().tolist())
self.assertEqual(symbols, n.symbols)
self.assertAlmostEqual(abs(positions - n.get_positions()).sum(), 0.)
self.assertIsNone(n.get_velocities())

Expand All @@ -3055,7 +3055,7 @@ def test_creation(self):
self.assertAlmostEqual(abs(stepids - n.get_stepids()).sum(), 0.)
self.assertAlmostEqual(abs(times - n.get_times()).sum(), 0.)
self.assertAlmostEqual(abs(cells - n.get_cells()).sum(), 0.)
self.assertEqual(symbols.tolist(), n.get_symbols().tolist())
self.assertEqual(symbols, n.symbols)
self.assertAlmostEqual(abs(positions - n.get_positions()).sum(), 0.)
self.assertIsNone(n.get_velocities())

Expand All @@ -3064,7 +3064,7 @@ def test_creation(self):
self.assertEqual(data[0], stepids[1])
self.assertAlmostEqual(data[1], times[1])
self.assertAlmostEqual(abs(cells[1] - data[2]).sum(), 0.)
self.assertEqual(symbols.tolist(), data[3].tolist())
self.assertEqual(symbols, data[3])
self.assertAlmostEqual(abs(data[4] - positions[1]).sum(), 0.)
self.assertIsNone(data[5])

Expand All @@ -3083,7 +3083,7 @@ def test_creation(self):
self.assertAlmostEqual(abs(stepids - n.get_stepids()).sum(), 0.)
self.assertAlmostEqual(abs(times - n.get_times()).sum(), 0.)
self.assertAlmostEqual(abs(cells - n.get_cells()).sum(), 0.)
self.assertEqual(symbols.tolist(), n.get_symbols().tolist())
self.assertEqual(symbols, n.symbols)
self.assertAlmostEqual(abs(positions - n.get_positions()).sum(), 0.)
self.assertIsNone(n.get_velocities())

Expand All @@ -3092,7 +3092,7 @@ def test_creation(self):
self.assertEqual(data[0], stepids[1])
self.assertAlmostEqual(data[1], times[1])
self.assertAlmostEqual(abs(cells[1] - data[2]).sum(), 0.)
self.assertEqual(symbols.tolist(), data[3].tolist())
self.assertEqual(symbols, data[3])
self.assertAlmostEqual(abs(data[4] - positions[1]).sum(), 0.)
self.assertIsNone(data[5])

Expand Down Expand Up @@ -3141,7 +3141,7 @@ def test_conversion_to_structure(self):
0.,
3.,
]]])
symbols = numpy.array(['H', 'O', 'C'])
symbols = ['H', 'O', 'C']
positions = numpy.array([[[0., 0., 0.], [0.5, 0.5, 0.5], [1.5, 1.5, 1.5]], [[0., 0., 0.], [0.5, 0.5, 0.5],
[1.5, 1.5, 1.5]]])
velocities = numpy.array([[[0., 0., 0.], [0., 0., 0.], [0., 0., 0.]], [[0.5, 0.5, 0.5], [0.5, 0.5, 0.5],
Expand All @@ -3152,15 +3152,15 @@ def test_conversion_to_structure(self):
stepids=stepids, cells=cells, symbols=symbols, positions=positions, times=times, velocities=velocities)

from_step = n.get_step_structure(1)
from_get_aiida_structure = n._get_aiida_structure(index=1)
from_get_structure = n.get_structure(index=1)

for struc in [from_step, from_get_aiida_structure]:
for struc in [from_step, from_get_structure]:
self.assertEqual(len(struc.sites), 3) # 3 sites
self.assertAlmostEqual(abs(numpy.array(struc.cell) - cells[1]).sum(), 0)
newpos = numpy.array([s.position for s in struc.sites])
self.assertAlmostEqual(abs(newpos - positions[1]).sum(), 0)
newkinds = [s.kind_name for s in struc.sites]
self.assertEqual(newkinds, symbols.tolist())
self.assertEqual(newkinds, symbols)

# Weird assignments (nobody should ever do this, but it is possible in
# principle and we want to check
Expand Down Expand Up @@ -3195,7 +3195,7 @@ def test_conversion_to_structure(self):
self.assertAlmostEqual(abs(newpos - positions[1]).sum(), 0)
newkinds = [s.kind_name for s in struc.sites]
# Kinds are in the same order as given in the custm_kinds list
self.assertEqual(newkinds, symbols.tolist())
self.assertEqual(newkinds, symbols)
newatomtypes = [struc.get_kind(s.kind_name).symbols[0] for s in struc.sites]
# Atoms remain in the same order as given in the positions list
self.assertEqual(newatomtypes, ['He', 'Os', 'Cu'])
Expand Down Expand Up @@ -3247,7 +3247,7 @@ def test_conversion_from_structurelist(self):

td = TrajectoryData(structurelist=structurelist)
self.assertEqual(td.get_cells().tolist(), cells)
self.assertEqual(td.get_symbols().tolist(), symbols[0])
self.assertEqual(td.symbols, symbols[0])
self.assertEqual(td.get_positions().tolist(), positions)

symbols = [['H', 'O', 'C'], ['H', 'O', 'P']]
Expand Down Expand Up @@ -3301,7 +3301,7 @@ def test_export_to_file(self):
0.,
3.,
]]])
symbols = numpy.array(['H', 'O', 'C'])
symbols = ['H', 'O', 'C']
positions = numpy.array([[[0., 0., 0.], [0.5, 0.5, 0.5], [1.5, 1.5, 1.5]], [[0., 0., 0.], [0.5, 0.5, 0.5],
[1.5, 1.5, 1.5]]])
velocities = numpy.array([[[0., 0., 0.], [0., 0., 0.], [0., 0., 0.]], [[0.5, 0.5, 0.5], [0.5, 0.5, 0.5],
Expand Down Expand Up @@ -3346,7 +3346,7 @@ def test_export_to_file(self):

class TestKpointsData(AiidaTestCase):
"""
Tests the TrajectoryData objects.
Tests the KpointsData objects.
"""

def test_set_kpoints_path_legacy(self):
Expand Down
4 changes: 2 additions & 2 deletions aiida/backends/tests/orm/data/remote.py
Expand Up @@ -58,6 +58,6 @@ def tearDown(self):

def test_clean(self):
"""Try cleaning a RemoteData node."""
self.assertFalse(self.remote.is_empty())
self.assertFalse(self.remote.is_empty)
self.remote._clean()
self.assertTrue(self.remote.is_empty())
self.assertTrue(self.remote.is_empty)
4 changes: 2 additions & 2 deletions aiida/backends/tests/tcodexporter.py
Expand Up @@ -156,7 +156,7 @@ def test_cif_structure_roundtrip(self):
tmpf.flush()
a = CifData(file=tmpf.name)

c = a._get_aiida_structure()
c = a.get_structure()
c.store()
pd = ParameterData()

Expand Down Expand Up @@ -249,7 +249,7 @@ def test_inline_export(self):
tmpf.flush()
a = CifData(file=tmpf.name)

s = a._get_aiida_structure(store=True)
s = a.get_structure(store=True)
val = export_values(s)
script = val.first_block()['_tcod_file_contents'][1]
function = '_get_aiida_structure_pymatgen_inline'
Expand Down
2 changes: 1 addition & 1 deletion aiida/cmdline/commands/cmd_code.py
Expand Up @@ -165,7 +165,7 @@ def code_duplicate(ctx, code, non_interactive, **kwargs):
@with_dbenv()
def show(code, verbose):
"""Display detailed information for the given CODE."""
click.echo(tabulate.tabulate(code.full_text_info(verbose)))
click.echo(tabulate.tabulate(code.get_full_text_info(verbose)))


@verdi_code.command()
Expand Down
14 changes: 14 additions & 0 deletions aiida/orm/data/array/__init__.py
Expand Up @@ -95,6 +95,20 @@ def iterarrays(self):
Iterator that returns tuples (name, array) for each array stored in the
node.
"""
import warnings
from aiida.common.warnings import AiidaDeprecationWarning as DeprecationWarning # pylint: disable=redefined-builtin
warnings.warn( # pylint: disable=no-member
'This method has been deprecated and will be renamed to get_iterarrays() in AiiDA v1.0', DeprecationWarning)
return self.get_iterarrays()

def get_iterarrays(self):
"""
Iterator that returns tuples (name, array) for each array stored in the
node.
.. versionadded:: 1.0
Renamed from iterarrays
"""
for name in self.get_arraynames():
yield (name, self.get_array(name))

Expand Down

0 comments on commit 85c6cc6

Please sign in to comment.