From aa967b0157b52a5ffc39ed8b40cf0b3b5b4df884 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 25 Jun 2026 12:25:00 +1000 Subject: [PATCH 1/6] #3461 Try to import scripts differently to allow for different scripts with the same name. --- src/psyclone/generator.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/psyclone/generator.py b/src/psyclone/generator.py index f0005709a2..f5c4886a02 100644 --- a/src/psyclone/generator.py +++ b/src/psyclone/generator.py @@ -47,13 +47,14 @@ ''' import argparse +import importlib +import logging import os +import pathlib +import shutil import sys import traceback -import importlib -import shutil from typing import Callable, Iterable, List, Optional, Tuple, Union -import logging from fparser.api import get_reader from fparser.two import Fortran2003 @@ -149,12 +150,11 @@ def load_script( raise GenerationError( f"generator: expected the script file '{filename}' to have " f"the '.py' extension") - # prepend file path - if none, the empty string equates to the current - # working directory - to the system path to guarantee we find the user - # provided module instead of a similarly named module that might - # already exist elsewhere in the system path - sys.path.insert(0, filepath) - recipe_module = importlib.import_module(module_name) + + script_path = pathlib.Path(script_name) + spec = importlib.util.spec_from_file_location(module_name, script_path) + recipe_module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(recipe_module) if hasattr(recipe_module, "FILES_TO_SKIP"): files_to_skip = recipe_module.FILES_TO_SKIP From 22cbd78d7d48117fdd540f7458ac4c836e53134a Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 25 Jun 2026 13:04:49 +1000 Subject: [PATCH 2/6] #3461 Fixed failing test that relied on setting sys.path. --- src/psyclone/tests/generator_test.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/psyclone/tests/generator_test.py b/src/psyclone/tests/generator_test.py index 4e993b4620..fd06c3b2b5 100644 --- a/src/psyclone/tests/generator_test.py +++ b/src/psyclone/tests/generator_test.py @@ -540,11 +540,12 @@ def test_script_null_trans(script_factory): '\n'.join(str(psy2).split('\n')[1:]) -def test_script_null_trans_relative(script_factory): +def test_script_null_trans_relative(script_factory, monkeypatch, tmpdir): '''Checks that generator.py works correctly when the trans() function in a valid script file does no transformations (it simply passes input to output). In this case the valid script file contains no - path and must therefore be found via the PYTHOPATH path list. + path, but is invoked from the script's directory, as a relative + path. ''' alg1, psy1 = generate(os.path.join(BASE_PATH, "lfric", @@ -552,14 +553,14 @@ def test_script_null_trans_relative(script_factory): api="lfric") empty_script = script_factory("def trans(psyir):\n pass") basename = os.path.basename(empty_script) - path = os.path.dirname(empty_script) - # Set the script directory in the PYTHONPATH - os.sys.path.append(path) + + # Change into the script's directory so it's found as a relative import. + monkeypatch.chdir(tmpdir) + alg2, psy2 = generate(os.path.join(BASE_PATH, "lfric", "1_single_invoke.f90"), api="lfric", script_name=basename) - # Remove the path from PYTHONPATH - os.sys.path.pop() + # we need to remove the first line before comparing output as # this line is an instance specific header assert '\n'.join(str(alg1).split('\n')[1:]) == \ From ab86dd3c628561f56ed4a4b64da2497b43fc7c1b Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 25 Jun 2026 15:42:54 +1000 Subject: [PATCH 3/6] #3461 Add script dir back to sys.path to support psyclone scripts importing helper scripts, but clean up sys.pathin the end. --- src/psyclone/generator.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/psyclone/generator.py b/src/psyclone/generator.py index f5c4886a02..f08b541520 100644 --- a/src/psyclone/generator.py +++ b/src/psyclone/generator.py @@ -151,6 +151,7 @@ def load_script( f"generator: expected the script file '{filename}' to have " f"the '.py' extension") + sys.path.insert(0, filepath) script_path = pathlib.Path(script_name) spec = importlib.util.spec_from_file_location(module_name, script_path) recipe_module = importlib.util.module_from_spec(spec) @@ -287,6 +288,7 @@ def generate(filename: str, # Apply provided recipe to PSyIR recipe, _, _ = load_script(script_name) recipe(psy.container.root) + del sys.path[0] alg_gen = None elif api in GOCEAN_API_NAMES or (api in LFRIC_API_NAMES and LFRIC_TESTING): @@ -340,6 +342,7 @@ def generate(filename: str, is_optional=True) if recipe: recipe(psyir) + del sys.path[0] # For each kernel called from the algorithm layer kernels = {} @@ -428,6 +431,7 @@ def generate(filename: str, # Call the optimisation script for psy-layer optimisations recipe, _, _ = load_script(script_name) recipe(psy.container.root) + del sys.path[0] # TODO issue #1618 remove Alg class and tests from PSyclone if api in LFRIC_API_NAMES and not LFRIC_TESTING: @@ -979,3 +983,5 @@ def code_transformation_mode(input_file, recipe_file, output_file, else: print(f"File '{input_file}' skipped because it is listed in " "FILES_TO_SKIP.", file=sys.stdout) + + del sys.path[0] From 51120838ef0f540f277f61509e5b1aec23f60cb5 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Fri, 26 Jun 2026 16:58:28 +1000 Subject: [PATCH 4/6] #3461 Updated documentation. --- doc/user_guide/user_scripts.rst | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/doc/user_guide/user_scripts.rst b/doc/user_guide/user_scripts.rst index 36c677c044..dd035fac21 100644 --- a/doc/user_guide/user_scripts.rst +++ b/doc/user_guide/user_scripts.rst @@ -44,23 +44,22 @@ PSyclone User Scripts The standard way to transform a codebase using psyclone is through the :ref:`psyclone_command` tool, which has an optional ``-s `` flag that allows users to specify a transformation user script to -programmatically modify the input code:: +programmatically modify the input code using a relative or absolute +path:: > psyclone -s optimise.py input_source.f90 - -In this case, the current directory is prepended to the Python search path -``PYTHONPATH`` which will then be used to try to find the script file. Thus, -the search begins in the current directory and continues over any pre-existing -directories in the search path, failing if the file cannot be found. - -Alternatively, script files may be specified with a path. In this case -the file must exist in the specified location. This location is then added to -the Python search path ``PYTHONPATH`` as before. For example:: - > psyclone -s ./optimise.py input_source.f90 > psyclone -s ../scripts/optimise.py input_source.f90 > psyclone -s /home/me/PSyclone/scripts/optimise.py input_source.f90 +PSyclone will not take ``PYTHONPATH`` into account when importing the script, +so the user must ensure that the script is found: For this, it must either be +in the current working directory (if no path is specified at all), or be +specified with a valid (relative or absolute) path. PSyclone will add the +directory of the script to its Python's ``sys.path``, meaning the PSyclone +script can import any helper script in the same directory without additional +setup. + A valid PSyclone user script file must contain a ``trans`` function which accepts a :ref:`PSyIR node` representing the root of the psy-layer code (as a FileContainer): From fb04cd2356996192006a53a696c5eaa9a5d4405b Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Sat, 27 Jun 2026 00:33:01 +1000 Subject: [PATCH 5/6] #3461 Replaced fixture with simple function since duplicated script names do not need to be handled/removed anymore. --- src/psyclone/tests/generator_test.py | 80 +++++++++++----------------- 1 file changed, 30 insertions(+), 50 deletions(-) diff --git a/src/psyclone/tests/generator_test.py b/src/psyclone/tests/generator_test.py index fd06c3b2b5..5c505064de 100644 --- a/src/psyclone/tests/generator_test.py +++ b/src/psyclone/tests/generator_test.py @@ -50,7 +50,6 @@ import re import shutil import stat -from sys import modules from typing import Optional import pytest @@ -84,37 +83,18 @@ "test_files", "gocean1p0") -@pytest.fixture(name="script_factory", scope="function") -def create_script_factor(tmpdir): - ''' Fixture that creates a psyclone optimisation script given the string +def script_factory(tmpdir, code: str) -> Path: + """ + Function that creates a psyclone optimisation script given the string representing the body of the script: script_path = script_factory("def trans(psyir):\n pass") - It has a 'function' scope and a tear down section because using a script - imports the file and this is kept in the python interpreter state, so we - delete it for future tests. - - ''' + """ tmpfile = os.path.join(tmpdir, "test_script.py") - - def populate_script(string): - with open(tmpfile, 'w+', encoding="utf8") as script: - script.write(string) - return tmpfile - - yield populate_script - # Tear down section executed after each test that uses the fixture - # If the created script was used, then its module (file) was imported - # into the interpreter runtime, we need to make sure it is deleted - modname = "test_script" - if modname in modules: - del modules[modname] - for mod in modules.values(): - try: - delattr(mod, modname) - except AttributeError: - pass + with open(tmpfile, 'w+', encoding="utf8") as script: + script.write(code) + return tmpfile def test_script_file_not_found(): @@ -165,14 +145,14 @@ def test_script_file_wrong_extension(): "extension" in str(error.value)) -def test_script_invalid_content(script_factory): +def test_script_invalid_content(tmpdir): '''Checks that load_script() in generator.py raises the expected exception when a script file does not contain valid python. This test uses the generate() function to call load_script as this is a simple way to create its required arguments. ''' - error_syntax = script_factory(""" + error_syntax = script_factory(tmpdir, """ this is invalid python """) with pytest.raises(Exception) as err: @@ -181,7 +161,7 @@ def test_script_invalid_content(script_factory): api="lfric", script_name=error_syntax) assert "invalid syntax (test_script.py, line 2)" in str(err.value) - error_import = script_factory(""" + error_import = script_factory(tmpdir, """ import non_existent """) with pytest.raises(Exception) as err: @@ -191,7 +171,7 @@ def test_script_invalid_content(script_factory): assert "No module named 'non_existent'" in str(err.value) -def test_script_invalid_content_runtime(script_factory): +def test_script_invalid_content_runtime(tmpdir): '''Checks that load_script() function in generator.py raises the expected exception when a script file contains valid python syntactically but produces a runtime exception. This test uses the @@ -199,7 +179,7 @@ def test_script_invalid_content_runtime(script_factory): to create its required arguments. ''' - runtime_error = script_factory(""" + runtime_error = script_factory(tmpdir, """ def trans(psyir): # this will produce a runtime error as b has not been assigned psyir = b @@ -211,7 +191,7 @@ def trans(psyir): assert "name 'b' is not defined" in str(error.value) -def test_script_no_trans(script_factory): +def test_script_no_trans(tmpdir): '''Checks that load_script() function in generator.py raises the expected exception when a script file does not contain a trans() function. This test uses the generate() function to call @@ -219,7 +199,7 @@ def test_script_no_trans(script_factory): arguments. ''' - no_trans_script = script_factory(""" + no_trans_script = script_factory(tmpdir, """ def nottrans(psyir): pass @@ -235,7 +215,7 @@ def tran(): in str(error.value)) -def test_script_no_trans_alg(capsys, script_factory): +def test_script_no_trans_alg(capsys, tmpdir): '''Checks that load_script() function in generator.py does not raise an exception when a script file does not contain a trans_alg() function as these are optional. At the moment this function is @@ -244,7 +224,7 @@ def test_script_no_trans_alg(capsys, script_factory): its required arguments. ''' - no_alg_script = script_factory("def trans(psyir):\n pass") + no_alg_script = script_factory(tmpdir, "def trans(psyir):\n pass") _, _ = generate( os.path.join(BASE_PATH, "gocean1p0", "single_invoke.f90"), api="gocean", script_name=no_alg_script) @@ -254,7 +234,7 @@ def test_script_no_trans_alg(capsys, script_factory): assert "Deprecation warning:" not in captured.err -def test_script_with_legacy_trans_signature(capsys, script_factory): +def test_script_with_legacy_trans_signature(capsys, tmpdir): '''Checks that load_script() function in generator.py does not raise an exception when a script file uses the legacy trans signature. @@ -264,7 +244,7 @@ def test_script_with_legacy_trans_signature(capsys, script_factory): This will eventually be deprecated. ''' - legacy_script = script_factory(""" + legacy_script = script_factory(tmpdir, """ def trans(psy): # The following are backwards-compatible expressions with legacy scripts _ = psy.invokes.invoke_list @@ -440,13 +420,13 @@ def test_no_script_gocean(): assert "module psy_single_invoke_test" in str(psy) -def test_script_gocean(script_factory): +def test_script_gocean(tmpdir): '''Test that the generate function in generator.py returns successfully if a script (containing both trans_alg() and trans() functions) is specified. ''' - alg_script = script_factory(""" + alg_script = script_factory(tmpdir, """ def trans_alg(psyir): pass @@ -496,12 +476,12 @@ def _broken(_1, _2): assert "Failed to create PSyIR from file '" in err -def test_script_attr_error(script_factory): +def test_script_attr_error(tmpdir): '''Checks that generator.py raises an appropriate error when a script file contains a trans() function which raises an attribute error. ''' - error_script = script_factory(""" + error_script = script_factory(tmpdir, """ from psyclone.psyGen import Loop from psyclone.transformations import ColourTrans @@ -520,12 +500,12 @@ def trans(psyir): assert 'object has no attribute' in str(excinfo.value) -def test_script_null_trans(script_factory): +def test_script_null_trans(tmpdir): '''Checks that generator.py works correctly when the trans() function in a valid script file does no transformations. ''' - empty_script = script_factory("def trans(psyir):\n pass") + empty_script = script_factory(tmpdir, "def trans(psyir):\n pass") alg1, psy1 = generate(os.path.join(BASE_PATH, "lfric", "1_single_invoke.f90"), api="lfric") @@ -540,7 +520,7 @@ def test_script_null_trans(script_factory): '\n'.join(str(psy2).split('\n')[1:]) -def test_script_null_trans_relative(script_factory, monkeypatch, tmpdir): +def test_script_null_trans_relative(monkeypatch, tmpdir): '''Checks that generator.py works correctly when the trans() function in a valid script file does no transformations (it simply passes input to output). In this case the valid script file contains no @@ -551,7 +531,7 @@ def test_script_null_trans_relative(script_factory, monkeypatch, tmpdir): alg1, psy1 = generate(os.path.join(BASE_PATH, "lfric", "1_single_invoke.f90"), api="lfric") - empty_script = script_factory("def trans(psyir):\n pass") + empty_script = script_factory(tmpdir, "def trans(psyir):\n pass") basename = os.path.basename(empty_script) # Change into the script's directory so it's found as a relative import. @@ -568,13 +548,13 @@ def test_script_null_trans_relative(script_factory, monkeypatch, tmpdir): assert str(psy1) == str(psy2) -def test_script_trans_lfric(script_factory): +def test_script_trans_lfric(tmpdir): '''Checks that generator.py works correctly when a transformation is provided as a script, i.e. it applies the transformations correctly. ''' - fuse_loop_script = script_factory(""" + fuse_loop_script = script_factory(tmpdir, """ from psyclone.domain.lfric.transformations import LFRicLoopFuseTrans def trans(psyir): module = psyir.children[0] @@ -1897,7 +1877,7 @@ def test_no_script_lfric_new(monkeypatch): assert "use _psyclone_builtins" not in alg -def test_script_lfric_new(monkeypatch, script_factory): +def test_script_lfric_new(monkeypatch, tmpdir): '''Test that the generate function in generator.py returns successfully if a script (containing both trans_alg() and trans() functions) is specified. This test uses the new PSyIR approach to @@ -1906,7 +1886,7 @@ def test_script_lfric_new(monkeypatch, script_factory): monkeypatching. ''' - alg_script = script_factory(""" + alg_script = script_factory(tmpdir, """ def trans_alg(psyir): pass From efe64591a214b1a17c08f1e3b00c5d1b9da4663d Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Sat, 27 Jun 2026 00:33:18 +1000 Subject: [PATCH 6/6] #3461 Updated comments. --- src/psyclone/generator.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/psyclone/generator.py b/src/psyclone/generator.py index f08b541520..9f1cc43bcb 100644 --- a/src/psyclone/generator.py +++ b/src/psyclone/generator.py @@ -151,7 +151,13 @@ def load_script( f"generator: expected the script file '{filename}' to have " f"the '.py' extension") + # Add the script directory to sys.path, so scripts can easily import + # helper scripts in the same directory (this step is not needed to + # import the script itself, but it maintains backwards compatibility). sys.path.insert(0, filepath) + + # This will import the module, but not make it part of the + # system list of all modules, i.e. it can be used elsewhere. script_path = pathlib.Path(script_name) spec = importlib.util.spec_from_file_location(module_name, script_path) recipe_module = importlib.util.module_from_spec(spec)