(towards #2668) Add kwargs to all remaining transformations by sergisiso · Pull Request #3612 · stfc/PSyclone

sergisiso · 2026-09-21T17:11:51Z

No description provided.

…kwarg_trans

codecov · 2026-09-21T17:23:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (e93a73c) to head (ca6d36e).

Additional details and impacted files

@@            Coverage Diff             @@
##            master     #3612    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          403       403            
  Lines        56790     56892   +102     
==========================================
+ Hits         56790     56892   +102

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sergisiso · 2026-09-22T06:38:42Z

@LonelyCat124 This is ready for review

LonelyCat124

I didn't look at the tests yet and had a fairly quick look through. It mostly looks ok (did it definitely find them all? Other than the Fuse ones iI'm currently working on), but its definitely missed a few of the things and in a couple of places it does some very non-obvious things.

It would be nice to cleanup the Nodes that take options as an input, but might be beyond the scope of this PR.

LonelyCat124 · 2026-09-22T12:43:33Z

        :param options: a dictionary with options for transformations.
-        :type options: Optional[Dict[str, Any]]
-        :param bool options["create_driver"]: whether or not to create a \
+        :param options["create_driver"]: whether or not to create a \


rm any options["..."] throughout the changed files.

I removed them all now

LonelyCat124 · 2026-09-22T12:43:57Z

        :param options: a dictionary with options for transformations.
-        :type options: Optional[Dict[str, Any]]
-        :param str options["prefix"]: a prefix to use for the PSyData module \
+        :param options["prefix"]: a prefix to use for the PSyData module \


is prefix inherited or it missed it?

Inherited from PSyDataTrans

LonelyCat124 · 2026-09-22T12:44:08Z

            "driver-MODULE-REGION.f90" where MODULE and REGION will be the \
            corresponding values for this region. Defaults to False.
-        :param (str,str) options["region_name"]: an optional name to \
+        :param options["region_name"]: an optional name to \


is region name inherited or missed?

Inherited from PSyDataTrans

LonelyCat124 · 2026-09-22T12:45:50Z

-          :py:class:`psyclone.domain.lfric.algorithm.LFRicAlgorithmInvokeCall`
        :param options: a dictionary with options for transformations.
-        :type options: Optional[Dict[str, Any]]
        :param options["kernels"]: this option provides a list of \


Again its kept options["..."]

I removed them all now

LonelyCat124 · 2026-09-22T12:47:23Z


        # Check the algorithm arguments and kernel metadata match.
-        self.get_arguments(node, options=options, check_args=True)
+        self.get_arguments(node, kernels=kernels, check_args=True)


Should get arguments also take **kwargs (and keep options) in case of inheritance?

I removed options (it is an internal function we don't need it). I kept kwargs, just because it is a subclass calling a method implemented in subclasses with different parameters. I think this is not the best design but maybe updating this is out-of-scope for this PR.

I also fully deleted the options from the alg_invoke_2_psy_call family of transformations as this are only for internal use and don't need backwards compatibility apis.

LonelyCat124 · 2026-09-22T13:00:31Z

@@ -8,13 +8,17 @@
 '''This module provides the LoopTiling2DTrans, which transforms a 2D Loop
 construct into a tiled implementation of the construct.'''


I had previously ignored this transformation because its deprecated, happy to have it changed but maybe we can remove/delete this transformation on a similar timeline as options anyway?

But for this PR I won't want to leave an exception without kwargs, maybe we keep the deprecation of the transformation separate of this?

LonelyCat124 · 2026-09-22T13:00:52Z

-        :type nodes: :py:class:`psyclone.psyir.nodes.Node` or
-                     list[:py:class:`psyclone.psyir.nodes.Node`]
-        :param bool options["force"]: whether to ignore potential control
+        :param force: whether to ignore potential control


We don't need the param on validate.

LonelyCat124 · 2026-09-22T13:01:54Z

+        :param force: ignore potential control-flow jumps in the region.
+
+        '''
+        if force:


Is this to avoid duplicating force in kwargs? That doesn't seem possible since force is explicitly defined on this apply method so can't we just do

super().apply(nodes, force=force, options=options, **kwargs)

Done (apparently is was due to a monkeypatched validate that didn't accept kwargs, but I fixed the test now)

LonelyCat124 · 2026-09-22T13:03:30Z

            (``PREFIX_PSYDATATYPE``) - a "_" will be added automatically. \
            It defaults to "".
-        :param (str,str) options["region_name"]: an optional name to \
+        :param region_name: an optional name to \


Only needed on apply

LonelyCat124 · 2026-09-22T13:04:22Z

+            validate_kwargs["prefix"] = prefix
+        if region_name is not None:
+            validate_kwargs["region_name"] = region_name
+        self.validate(node_list, options, **validate_kwargs)


What is going on here?

LonelyCat124 · 2026-09-22T13:07:16Z

For reference, this was the script I wrote to find all remaining transformations previously:

import importlib
import pkgutil


def import_submodules(package, recursive=True):
    """ Import all submodules of a module, recursively, including subpackages

    :param package: package (name or actual module)
    :type package: str | module
    :rtype: dict[str, types.ModuleType]
    """
    if isinstance(package, str):
        package = importlib.import_module(package)
    results = {}
    for loader, name, is_pkg in pkgutil.walk_packages(package.__path__):
        full_name = package.__name__ + '.' + name
        try:
            results[full_name] = importlib.import_module(full_name)
        except ModuleNotFoundError:
            continue
        if recursive and is_pkg:
            results.update(import_submodules(full_name))
    return results

import psyclone
from psyclone.psyGen import Transformation
import_submodules(psyclone)
import sys
import inspect
all_trans = []
for module in list(sys.modules.keys())[:]:
    for name, obj in inspect.getmembers(sys.modules[module]):
        if inspect.isclass(obj) and issubclass(obj, Transformation):
            if obj not in all_trans:
                all_trans.append(obj)

no_applies = []
no_kwargs = []
has_kwargs = []
for trans in all_trans:
    if not (trans.__dict__.get("apply", None)):
        no_applies.append(trans.__name__)
        continue
    apply = trans.__dict__.get("apply")
    signature = inspect.signature(apply)
    for k, v in signature.parameters.items():
        if k == "kwargs":
            has_kwargs.append(trans.__name__)
            break
    else:
        no_kwargs.append(trans.__name__)

print(has_kwargs)
print("----------------------------")
print(no_applies)
print("----------")
print(no_kwargs)

…rings

…kwarg_trans

sergisiso added 2 commits September 21, 2026 18:10

Add kwargs to all remaining transfomrations

40aae34

Merge remote-tracking branch 'origin/master' into 2668_all_remaining_…

69b8be1

…kwarg_trans

sergisiso requested a review from LonelyCat124 September 21, 2026 21:19

sergisiso self-assigned this Sep 21, 2026

sergisiso added the ready for review label Sep 21, 2026

sergisiso deployed to integration September 21, 2026 21:20 — with GitHub Actions Active

sergisiso added 2 commits September 21, 2026 23:06

Fix sphinx issues and add typehints

3fc82d9

Fix typehint for python 3.9

f8f79a4

LonelyCat124 requested changes Sep 22, 2026

View reviewed changes

LonelyCat124 added reviewed with actions and removed ready for review labels Sep 22, 2026

sergisiso added 5 commits September 23, 2026 21:51

#2668 Continue removing transformation options and fixing their docst…

7958b7b

…rings

#2668 Continue fixing transformation kwargs

8c01921

Merge remote-tracking branch 'origin/master' into 2668_all_remaining_…

981a278

…kwarg_trans

#2668 Continue fixing transformation kwargs

1263f38

Remove unused code

ca6d36e

		@@ -8,13 +8,17 @@
		'''This module provides the LoopTiling2DTrans, which transforms a 2D Loop
		construct into a tiled implementation of the construct.'''

Conversation

sergisiso commented Sep 21, 2026

Uh oh!

codecov Bot commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sergisiso commented Sep 22, 2026

Uh oh!

LonelyCat124 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LonelyCat124 commented Sep 22, 2026

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 21, 2026 •

edited

Loading