(Towards #2668) Implementation of keyword arguments to transformations instead of options dict.

setup.py

@@ -163,7 +163,7 @@ def get_files(directory, install_path, valid_suffixes):
         package_dir={"": "src"},
         install_requires=['pyparsing', 'fparser>=0.2.0', 'configparser',
                           'jsonschema', 'sympy', "Jinja2", 'termcolor',
-                          'graphviz'],
+                          'graphviz', 'sphinx'],
         extras_require={
             'doc': ["sphinx", "sphinxcontrib.bibtex", "sphinx-tabs",
                     "sphinx_rtd_theme", "sphinx-autodoc-typehints", "autoapi"],

src/psyclone/psyGen.py

@@ -41,10 +41,17 @@
     and generation. The classes in this method need to be specialised for a
     particular API and implementation. '''
 
+import inspect
 import os
 from collections import OrderedDict
 import abc
 
+try:
+    from sphinx.util.typing import stringify_annotation
+except ImportError:
+    # Fix for Python-3.7 where sphinx didn't yet rename this.
+    from sphinx.util.typing import stringify as stringify_annotation
+
 from psyclone.configuration import Config, LFRIC_API_NAMES, GOCEAN_API_NAMES
 from psyclone.core import AccessType
 from psyclone.errors import GenerationError, InternalError, FieldNotFoundError
@@ -2729,7 +2736,7 @@
         return type(self).__name__
 
     @abc.abstractmethod
-    def apply(self, node, options=None):
+    def apply(self, node, options=None, **kwargs):
         '''Abstract method that applies the transformation. This function
         must be implemented by each transform. As a minimum each apply
         function must take a node to which the transform is applied, and
@@ -2754,7 +2761,7 @@
 
         '''
 
-    def validate(self, node, options=None):
+    def validate(self, node, options=None, **kwargs):
         '''Method that validates that the input data is correct.
         It will raise exceptions if the input data is incorrect. This
         function needs to be implemented by each transformation.
@@ -2784,6 +2791,108 @@
         '''
         # pylint: disable=unused-argument
 
+    def get_option(self, option_name: str, **kwargs):
+        '''Test if we can just pull the option value from the name of
+        the option'''
+        valid_options = type(self).get_valid_options()
+        if option_name not in valid_options.keys():
+            raise ValueError(f"'{type(self).__name__}' failed to get option "
+                             f"'{option_name}' as it is not provided as a "
+                             f"keyword argument to the apply method.")
+        return kwargs.get(option_name, valid_options[option_name]['default'])
+
+    @classmethod
+    def get_valid_options(cls):
+        '''
+        Pulls the valid options from the apply method. It also recurses
+        upwards to the superclasses of this transformation and pulls
+        their valid options as well if they exist.
+        '''
+        valid_options = OrderedDict()
+        # Loop through the inherited classes of this class, starting with the
+        # highest superclass. For some reason super isn't working here.
+        for base_cls in cls.__mro__[::-1]:
+            base_cls_apply = base_cls.__dict__.get("apply", None)
+            if base_cls_apply is None:
+                continue
+            signature = inspect.signature(base_cls_apply)
+            # Loop over the arguments to the apply call.
+            for k, v in signature.parameters.items():
+                if k == "options":
+                    continue
+                # If the argument is a keyword argument, i.e. it has a default
+                # value then we add it to the list of options.
+                if v.default is not inspect.Parameter.empty:
+                    valid_options[k] = {}
+                    valid_options[k]['default'] = v.default
+                else:
+                    # If its not a keyword argument then we skip it.
+                    continue
+                # If there is type information on the keyword argument, then we
+                # store the information so we can generate docs and do
+                # automatic type checking of options.
+                if v.annotation is not inspect.Parameter.empty:
+                    valid_options[k]['type'] = v.annotation
+                    valid_options[k]['typename'] = stringify_annotation(
+                            v.annotation
+                    )
+                else:
+                    valid_options[k]['type'] = None
+                    valid_options[k]['typename'] = None
+        return valid_options
+
+    def validate_options(self, **kwargs):
+        '''
+        Checks the options provided to this transformation are valid. Makes
+        use of get_valid_options to work out the available options.
+
+        :raises ValueError: If an invalid option was provided.
+        :raises TypeError: If an option was provided with a value of the wrong
+                           type.
+        '''
+        valid_options = type(self).get_valid_options()
+        invalid_options = []
+        wrong_types = {}
+        for option in kwargs:
+            if option == "options":
+                continue
+            if option not in valid_options:
+                invalid_options.append(option)
+                continue
+            if valid_options[option]['type'] is not None:
+                try:
+                    if not isinstance(
+                            kwargs[option], valid_options[option]['type']):
+                        wrong_types[option] = type(kwargs[option]).__name__
+                except TypeError:
+                    # For older versions of Python, such as 3.8 they don't yet
+                    # support type checking for Generics, e.g. Union[...] so
+                    # we skip this check and it needs to be done in the
+                    # relevant function instead.
+                    pass
+
+        if len(invalid_options) > 0:
+            invalid_options_detail = []
+            for invalid in invalid_options:
+                invalid_options_detail.append(f"'{invalid}'")
+            invalid_options_list = ", ".join(invalid_options_detail)
+            raise ValueError(f"'{type(self).__name__}' received invalid "
+                             f"options [{invalid_options_list}]. Please see "
+                             f"the documentation and check the available "
+                             f"options.")
+        if len(wrong_types.keys()) > 0:
+            wrong_types_detail = []
+            for name in wrong_types.keys():
+                wrong_types_detail.append(
+                        f"'{name}' option expects type "
+                        f"'{valid_options[name]['typename']}' but received "
+                        f"'{kwargs[name]}' of type '{wrong_types[name]}'.")
+            wrong_types_list = "\n".join(wrong_types_detail)
+            raise TypeError(f"'{type(self).__name__}' received options with "
+                            f"the wrong types:\n{wrong_types_list}\n"
+                            f"Please see the documentation and check the "
+                            f"provided types.")
+
 
 class DummyTransformation(Transformation):
     '''Dummy transformation use elsewhere to keep pyreverse happy.'''

src/psyclone/psyir/transformations/loop_trans.py

@@ -62,7 +62,23 @@ class LoopTrans(Transformation, metaclass=abc.ABCMeta):
     # populated by sub-class.
     excluded_node_types = ()
 
-    def validate(self, node, options=None):
+    def apply(self, node, options=None, node_type_check: bool = True,
+              verbose: bool = False, **kwargs):
+        '''
+        :param node: target PSyIR node.
+        :type node: subclass of :py:class:`psyclone.psyir.nodes.Node`
+        :param options: a dictionary with options for transformations.
+        :type options: Optional[Dict[str, Any]]
+        :param node_type_check: If the type of nodes enclosed in the loop
+                                should be tested to avoid including
+                                unsupported nodes in the transformation.
+        :param verbose: whether to log the reason the validation failed, at
+                        the moment with a comment in the provided PSyIR node.
+        '''
+        super().apply(node, options=options, node_type_check=node_type_check,
+                      verbose=verbose, **kwargs)
+
+    def validate(self, node, options=None, **kwargs):
         '''Checks that the supplied node is a valid target for a loop
         transformation.
 
@@ -99,16 +115,19 @@ def validate(self, node, options=None):
                 f"{[type(child).__name__ for child in node.children]}.")
 
         if not options:
-            options = {}
-        if not isinstance(options, dict):
-            raise TransformationError(
-                f"Transformation validate method 'options' argument must be a "
-                f"dictionary but found '{type(options).__name__}'.")
-
-        verbose = options.get("verbose", False)
+            self.validate_options(**kwargs)
+            verbose = self.get_option("verbose", **kwargs)
+            node_type_check = self.get_option("node_type_check", **kwargs)
+        else:
+            if not isinstance(options, dict):
+                raise TransformationError(
+                    f"Transformation validate method 'options' argument must "
+                    f"be a dictionary but found '{type(options).__name__}'.")
+            verbose = options.get("verbose", False)
+            node_type_check = options.get("node-type-check", True)
 
         # Check that the proposed region contains only supported node types
-        if options.get("node-type-check", True):
+        if node_type_check:
             # Stop at any instance of Kern to avoid going into the
             # actual kernels, e.g. in Nemo inlined kernels
             # pylint: disable=cell-var-from-loop

src/psyclone/psyir/transformations/omp_loop_trans.py

@@ -226,7 +226,9 @@ def _directive(self, children, collapse=None):
             node.reprod = self._reprod
         return node
 
-    def apply(self, node, options=None):
+    def apply(self, node, options=None,
+              reprod: bool = Config.get().reproducible_reductions,
+              **kwargs):
         '''Apply the OMPLoopTrans transformation to the specified PSyIR Loop.
 
         :param node: the supplied node to which we will apply the \
@@ -241,9 +243,13 @@ def apply(self, node, options=None):
 
         '''
         if not options:
-            options = {}
-        self._reprod = options.get("reprod",
-                                   Config.get().reproducible_reductions)
+            self.validate_options(
+                    reprod=reprod, **kwargs
+            )
+            self._reprod = reprod
+        else:
+            self._reprod = options.get("reprod",
+                                       Config.get().reproducible_reductions)
 
         if self._reprod:
             # When reprod is True, the variables th_idx and nthreads are
@@ -264,4 +270,4 @@ def apply(self, node, options=None):
                     "nthreads", tag="omp_num_threads",
                     symbol_type=DataSymbol, datatype=INTEGER_TYPE)
 
-        super().apply(node, options)
+        super().apply(node, options, **kwargs)

src/psyclone/psyir/transformations/omp_task_trans.py

@@ -69,7 +69,7 @@ def name(self):
         '''
         return "OMPTaskTrans"
 
-    def validate(self, node, options=None):
+    def validate(self, node, options=None, **kwargs):
         '''
         Validity checks for input arguments.
 
@@ -169,7 +169,7 @@ def _inline_kernels(self, node):
                 continue
             intrans.apply(call)
 
-    def apply(self, node, options=None):
+    def apply(self, node, options=None, **kwargs):
         '''Apply the OMPTaskTrans to the specified node in a Schedule.
 
         Can only be applied to a Loop.

src/psyclone/psyir/transformations/parallel_loop_trans.py

@@ -40,6 +40,7 @@
 
 import abc
 from collections.abc import Iterable
+from typing import Union, List
 
 from psyclone import psyGen
 from psyclone.core import Signature
@@ -52,7 +53,10 @@
 from psyclone.psyir.transformations.transformation_error import \
     TransformationError
 
+from psyclone.utils import transformation_documentation_wrapper
 
+
+@transformation_documentation_wrapper
 class ParallelLoopTrans(LoopTrans, metaclass=abc.ABCMeta):
     '''
     Adds an abstract directive (it needs to be specified by sub-classing this
@@ -118,7 +122,7 @@ def _attempt_privatisation(node, symbol_name, dry_run=False):
 
         return True
 
-    def validate(self, node, options=None):
+    def validate(self, node, options=None, **kwargs):
         '''
         Perform validation checks before applying the transformation
 
@@ -156,16 +160,28 @@ def validate(self, node, options=None):
         '''
         # Check that the supplied node is a Loop and does not contain any
         # unsupported nodes.
-        super().validate(node, options=options)
-
+        super().validate(node, options=options, **kwargs)
         if not options:
-            options = {}
-        verbose = options.get("verbose", False)
-        collapse = options.get("collapse", False)
-        force = options.get("force", False)
-        ignore_dependencies_for = options.get("ignore_dependencies_for", [])
-        sequential = options.get("sequential", False)
-        privatise_arrays = options.get("privatise_arrays", False)
+            self.validate_options(**kwargs)
+            verbose = self.get_option("verbose", **kwargs)
+            collapse = self.get_option("collapse", **kwargs)
+            force = self.get_option("force", **kwargs)
+            ignore_dependencies_for = self.get_option(
+                    "ignore_dependencies_for", **kwargs
+            )
+            if ignore_dependencies_for is None:
+                ignore_dependencies_for = []
+            sequential = self.get_option("sequential", **kwargs)
+            privatise_arrays = self.get_option("privatise_arrays", **kwargs)
+        else:
+            verbose = options.get("verbose", False)
+            collapse = options.get("collapse", False)
+            force = options.get("force", False)
+            ignore_dependencies_for = options.get(
+                    "ignore_dependencies_for", []
+            )
+            sequential = options.get("sequential", False)
+            privatise_arrays = options.get("privatise_arrays", False)
 
         # Check we are not a sequential loop
         if (not sequential and isinstance(node, PSyLoop) and
@@ -272,7 +288,11 @@ def validate(self, node, options=None):
                         node.append_preceding_comment(f"PSyclone: {messages}")
                 raise TransformationError(messages)
 
-    def apply(self, node, options=None):
+    def apply(self, node, options=None, verbose: bool = False,
+              collapse: Union[int, bool] = False, force: bool = False,
+              ignore_dependencies_for: Union[None, List[str]] = None,
+              privatise_arrays: bool = False, sequential: bool = False,
+              **kwargs):
         '''
         Apply the Loop transformation to the specified node in a
         Schedule. This node must be a Loop since this transformation
@@ -311,14 +331,31 @@ def apply(self, node, options=None):
 
         '''
         if not options:
-            options = {}
-        self.validate(node, options=options)
-
-        verbose = options.get("verbose", False)
-        collapse = options.get("collapse", False)
-        ignore_dep_analysis = options.get("force", False)
-        list_of_names = options.get("ignore_dependencies_for", [])
-        privatise_arrays = options.get("privatise_arrays", False)
+            self.validate_options(
+                    verbose=verbose, collapse=collapse,
+                    ignore_dependencies_for=ignore_dependencies_for,
+                    privatise_arrays=privatise_arrays,
+                    sequential=sequential, **kwargs
+            )
+            # Rename the input options that are renamed in this apply method.
+            ignore_dep_analysis = force
+            if ignore_dependencies_for is None:
+                list_of_names = []
+            else:
+                list_of_names = ignore_dependencies_for
+        else:
+            verbose = options.get("verbose", False)
+            collapse = options.get("collapse", False)
+            ignore_dep_analysis = options.get("force", False)
+            list_of_names = options.get("ignore_dependencies_for", [])
+            privatise_arrays = options.get("privatise_arrays", False)
+
+        self.validate(node, options=options, verbose=verbose,
+                      collapse=collapse,
+                      ignore_dependencies_for=ignore_dependencies_for,
+                      privatise_arrays=privatise_arrays,
+                      sequential=sequential, **kwargs)
+
         list_of_signatures = [Signature(name) for name in list_of_names]
         dtools = DependencyTools()