Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Introduce more intuitive method names in Data classes and reduce required input parameter for TrajectoryData.set_trajectory() method #2310

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d225427
Rename ArrayData.iterarrays() to ArrayData.get_iterarrays()
astamminger Dec 5, 2018
2501641
[TrajectoryData] Rename _get_aiida_structure() -> get_structure()
astamminger Dec 5, 2018
de30a41
[TrajectoryData] Rename _get_cif() -> get_cif()
astamminger Dec 5, 2018
f14e35e
Rename _get_cif() -> get_cit() in tcod.py
astamminger Dec 5, 2018
0ed768e
[StructureData] Rename _get_cif() -> get_cif()
astamminger Dec 5, 2018
31b6269
Add notice on renamed function to function docstrings
astamminger Dec 5, 2018
cb6371c
Adjust tests according to renamed functions
astamminger Dec 6, 2018
4f1ab4d
Make stepid and cells parameter optional
astamminger Dec 6, 2018
8f2c928
Store symbols in TrajectoryData attribute instead of array
astamminger Dec 6, 2018
5580c4e
Change passed symbols from numpy.ndarray to list in set_structurelist
astamminger Dec 6, 2018
0452968
Change TrajectorData tests to use symbols attribute rather than symbo…
astamminger Dec 6, 2018
f6061fc
Adjust pylint options for CifData
astamminger Dec 6, 2018
10316a5
Make Code.is_hidden() available as class property
astamminger Dec 6, 2018
0e20afd
Rename Code.full_text_info() method to .get_full_text_info()
astamminger Dec 6, 2018
51a0a6a
Change RemoteData.is_empty() method to class property .is_empty
astamminger Dec 6, 2018
dd37eca
Change is_alloy() and has_vacancies() methods to properties
astamminger Dec 6, 2018
e37b233
Change symbols type from numpy.array to list
astamminger Dec 6, 2018
9953408
Add deprecation warning to renamed methods
astamminger Dec 6, 2018
9bd7bd2
Make list copy compatible with python < 3.3
astamminger Dec 6, 2018
81ff185
Remove pylint disable=no-member option
astamminger Dec 6, 2018
980b34d
Rename code attribute is_hidden to hidden.
astamminger Dec 13, 2018
03ff7a5
Set allowed symbols type to be Iterable rather than list
astamminger Dec 13, 2018
467d37b
Remove superfluous check for list dimensionality
astamminger Dec 13, 2018
016a59d
Ensure symbols are stored as list
astamminger Dec 13, 2018
bd23d8f
Adjust argument ordering in set_trajeytory() docstring.
astamminger Dec 13, 2018
44b2e88
Document introduced changes in concepts/workflows
astamminger Dec 13, 2018
505222f
Fix pylint complaining about missing 'warn' member.
astamminger Dec 29, 2018
82ffebb
Merge branch 'provenance_redesign' into issue_201
astamminger Dec 29, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
118 changes: 59 additions & 59 deletions aiida/backends/tests/dataclasses.py

Large diffs are not rendered by default.

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 @@ -157,7 +157,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 @@ -253,7 +253,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 @@ -166,7 +166,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
181 changes: 123 additions & 58 deletions aiida/orm/data/array/trajectory.py

Large diffs are not rendered by default.

26 changes: 25 additions & 1 deletion aiida/orm/data/cif.py
Expand Up @@ -420,7 +420,6 @@ def parse_formula(formula):
return contents


# pylint: disable=abstract-method
# Note: Method 'query' is abstract in class 'Node' but is not overridden
class CifData(SinglefileData):
"""
Expand All @@ -431,6 +430,7 @@ class CifData(SinglefileData):
when setting ``ase`` or ``values``, a physical CIF file is generated
first, the values are updated from the physical CIF file.
"""
# pylint: disable=abstract-method, too-many-public-methods
_set_incompatibilities = [('ase', 'file'), ('ase', 'values'), ('file', 'values')]
_scan_types = ['standard', 'flex']
_parse_policies = ['eager', 'lazy']
Expand Down Expand Up @@ -837,6 +837,30 @@ def _get_aiida_structure(self, converter='pymatgen', store=False, **kwargs):
"""
Creates :py:class:`aiida.orm.data.structure.StructureData`.

:param converter: specify the converter. Default 'pymatgen'.
:param store: if True, intermediate calculation gets stored in the
AiiDA database for record. Default False.
:param primitive_cell: if True, primitive cell is returned,
conventional cell if False. Default False.
:param occupancy_tolerance: If total occupancy of a site is between 1 and occupancy_tolerance,
the occupancies will be scaled down to 1. (pymatgen only)
:param site_tolerance: This tolerance is used to determine if two sites are sitting in the same position,
in which case they will be combined to a single disordered site. Defaults to 1e-4. (pymatgen only)
:return: :py:class:`aiida.orm.data.structure.StructureData` 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_structure() in AiiDA v1.0', DeprecationWarning)
return self.get_structure(converter=converter, store=store, **kwargs)

def get_structure(self, converter='pymatgen', store=False, **kwargs):
"""
Creates :py:class:`aiida.orm.data.structure.StructureData`.

.. versionadded:: 1.0
Renamed from _get_aiida_structure

:param converter: specify the converter. Default 'pymatgen'.
:param store: if True, intermediate calculation gets stored in the
AiiDA database for record. Default False.
Expand Down
12 changes: 11 additions & 1 deletion aiida/orm/data/code.py
Expand Up @@ -59,7 +59,8 @@ def reveal(self):
"""
self.set_extra(self.HIDDEN_KEY, False)

def is_hidden(self):
@property
def hidden(self):
"""
Determines whether the Code is hidden or not
"""
Expand Down Expand Up @@ -514,6 +515,15 @@ def get_builder(self):
return builder

def full_text_info(self, verbose=False):
"""
Return a (multiline) string with a human-readable detailed information on this computer
"""
import warnings
from aiida.common.warnings import AiidaDeprecationWarning as DeprecationWarning # pylint: disable=redefined-builtin
warnings.warn('This method has been deprecated and will be renamed to get_full_text_info() in AiiDA v1.0', DeprecationWarning)
return self.get_full_text_info(verbose=verbose)

def get_full_text_info(self, verbose=False):
"""
Return a (multiline) string with a human-readable detailed information on this computer
"""
Expand Down
1 change: 1 addition & 0 deletions aiida/orm/data/remote.py
Expand Up @@ -43,6 +43,7 @@ def add_path(self, src_abs, dst_filename=None):

raise ModificationNotAllowed("Cannot add files or directories to a RemoteData object")

@property
def is_empty(self):
"""
Check if remote folder is empty
Expand Down
35 changes: 28 additions & 7 deletions aiida/orm/data/structure.py
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,23 @@ 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: a boolean, True if at least one kind is an alloy
"""
return any(s.is_alloy() for s in self.kinds)
return any(s.is_alloy for s in self.kinds)

@property
def has_vacancies(self):
"""
To understand if there are 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(s.has_vacancies for s in self.kinds)

def get_cell_volume(self):
"""
Expand All @@ -1885,6 +1887,23 @@ def _get_cif(self, converter='ase', store=False, **kwargs):
"""
Creates :py:class:`aiida.orm.data.cif.CifData`.

:param converter: specify the converter. Default 'ase'.
:param store: If True, intermediate calculation gets stored in the
AiiDA database for record. Default False.
:return: :py:class:`aiida.orm.data.cif.CifData` node.
"""
import warnings
from aiida.common.warnings import AiidaDeprecationWarning as DeprecationWarning # pylint: disable=redefined-builtin
warnings.warn('This method has been deprecated and will be renamed to get_cif() in AiiDA v1.0', DeprecationWarning)
return self.get_cif(converter=converter, store=store, **kwargs)

def get_cif(self, converter='ase', store=False, **kwargs):
"""
Creates :py:class:`aiida.orm.data.cif.CifData`.

.. versionadded:: 1.0
Renamed from _get_cif

:param converter: specify the converter. Default 'ase'.
:param store: If True, intermediate calculation gets stored in the
AiiDA database for record. Default False.
Expand Down Expand Up @@ -2196,7 +2215,7 @@ def get_raw(self):
# raised (from the site.get_ase() routine).
# """
# import ase
# if self.is_alloy() or self.has_vacancies():
# 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],
Expand Down Expand Up @@ -2421,6 +2440,7 @@ 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.
Expand All @@ -2430,6 +2450,7 @@ def is_alloy(self):
"""
return len(self._symbols) != 1

@property
def has_vacancies(self):
"""
Returns True if the sum of the weights is less than one.
Expand Down Expand Up @@ -2530,7 +2551,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 +2596,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
6 changes: 3 additions & 3 deletions aiida/tools/dbexporters/tcod.py
Expand Up @@ -981,7 +981,7 @@ def export_cifnode(what, parameters=None, trajectory_index=None,
"""
The main exporter function. Exports given coordinate-containing \*Data
node to :py:class:`aiida.orm.data.cif.CifData` node, ready to be
exported to TCOD. All \*Data types, having method ``_get_cif()``, are
exported to TCOD. All \*Data types, having method ``get_cif()``, are
supported in addition to :py:class:`aiida.orm.data.cif.CifData`.

:param what: data node to be exported.
Expand Down Expand Up @@ -1038,11 +1038,11 @@ def export_cifnode(what, parameters=None, trajectory_index=None,

# Convert node to CifData (if required)

if not isinstance(node, CifData) and getattr(node, '_get_cif'):
if not isinstance(node, CifData) and getattr(node, 'get_cif'):
function_args = {'store': store}
if trajectory_index is not None:
function_args['index'] = trajectory_index
node = node._get_cif(**function_args)
node = node.get_cif(**function_args)

if not isinstance(node,CifData):
raise NotImplementedError("Exporter does not know how to "
Expand Down
2 changes: 1 addition & 1 deletion aiida/tools/dbimporters/baseclasses.py
Expand Up @@ -321,7 +321,7 @@ def get_aiida_structure(self, converter="pymatgen", store=False, **kwargs):
"""
cif = self.get_cif_node(store=store, parse_policy='lazy')

return cif._get_aiida_structure(converter=converter, store=store, **kwargs)
return cif.get_structure(converter=converter, store=store, **kwargs)

def get_parsed_cif(self):
"""
Expand Down
12 changes: 12 additions & 0 deletions docs/source/concepts/workflows.rst
Expand Up @@ -913,3 +913,15 @@ However, these workchains can be updated with just a few minor updates that we w
* The import ``aiida.work.workfunction.workfunction`` has been moved to ``aiida.work.process_function.workfunction``.
* The ``input_group`` has been deprecated and been replaced by namespaces. See the section on :ref:`port namespaces<ports_portnamespaces>` on how to use them.
* The use of a ``.`` (period) in output keys is not supported in ``Process.out`` because that is now reserved to indicate namespaces.
* The method ``ArrayData.iterarrayas()`` has been renamed to ``ArrayData.get_iterarrays()``.
* The method ``TrajectoryData._get_cif()`` has been renamed to ``TrajectoryData.get_cif()``.
* The method ``TrajectoryData._get_aiida_structure()`` has been renamed to ``TrajectoryData.get_structure()``.
* The method ``StructureData._get_cif()`` has been renamed to ``StructureData.get_cif()``.
* The method ``Code.full_text_info()`` has been renamed to ``Code.get_full_text_info()``.
* The method ``Code.is_hidden()`` has been changed and is now accessed through the ``Code.hidden`` property.
* The method ``RemoteData.is_empty()`` has been changes and is now accessed through the ``RemoteData.is_empty``.
* The method ``.is_alloy()`` for classes ``StructureData`` and ``Kind`` is now accessed through the ``.is_alloy`` property.
* The method ``.has_vacancies()`` for classes ``StructureData`` and ``Kind`` is now accessed through the ``.has_vacancies`` property.
* The arguments ``stepids`` and ``cells`` of the :meth:`TrajectoryData.set_trajectory()<aiida.orm.data.array.trajectory.TrajectoryData.set_trajectory>` method are made optional
which has implications on the ordering of the arguments passed to this method.
* The list of atomic symbols for trajectories is no longer stored as array data but is now accessible through the ``TrajectoryData.symbols`` attribute.