Introduce more intuitive method names in Data classes and reduce requ…

…ired input parameter for TrajectoryData.set_trajectory() method (#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 #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

aiida/backends/tests/cmdline/commands/test_code.py

@@ -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'])

aiida/backends/tests/cmdline/commands/test_data.py

@@ -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],

aiida/backends/tests/cmdline/commands/test_node.py

@@ -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],

aiida/backends/tests/orm/data/remote.py

@@ -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)

aiida/backends/tests/tcodexporter.py

@@ -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()
 
@@ -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'

aiida/cmdline/commands/cmd_code.py

@@ -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()

aiida/orm/data/array/__init__.py

@@ -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))
 

aiida/orm/data/cif.py

@@ -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):
     """
@@ -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']
@@ -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.

aiida/orm/data/code.py

@@ -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
         """
@@ -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
         """

aiida/orm/data/remote.py

@@ -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

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,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):
         """
@@ -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.
@@ -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],
@@ -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.
@@ -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.
@@ -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
@@ -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,

aiida/tools/dbexporters/tcod.py

@@ -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.
@@ -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 "

aiida/tools/dbimporters/baseclasses.py

@@ -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):
         """

docs/source/concepts/workflows.rst

@@ -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.