From 9bd4666810a1fbaa3b1559b52fa2ed1ffc11bc27 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 7 May 2026 15:00:56 +0200 Subject: [PATCH 1/7] Replace raw `dict` by `CategorizedPaths` The current dict is pretty opaque and makes spelling mistakes easy. Avoid by using a typed, named tuple. --- easybuild/framework/easyconfig/tools.py | 25 ++++----- easybuild/main.py | 2 +- easybuild/tools/github.py | 68 ++++++++++++++----------- test/framework/easyconfig.py | 13 +++-- 4 files changed, 54 insertions(+), 54 deletions(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index 0907589754..3b522b3d35 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -62,7 +62,7 @@ from easybuild.tools.environment import restore_env from easybuild.tools.filetools import EASYBLOCK_CLASS_PREFIX, PathOrStr, get_cwd, find_easyconfigs, is_patch_file from easybuild.tools.filetools import locate_files, read_file, resolve_path, which, write_file -from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO +from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO, CategorizedPaths from easybuild.tools.github import det_pr_labels, det_pr_title, download_repo, fetch_easyconfigs_from_commit from easybuild.tools.github import fetch_easyconfigs_from_pr, fetch_pr_data from easybuild.tools.github import fetch_files_from_commit, fetch_files_from_pr @@ -622,39 +622,32 @@ def dump_env_script(easyconfigs): dump_env_easyblock(app, orig_env=orig_env, ec_path=ec.path, script_path=script_path) -def categorize_files_by_type(paths: List[PathOrStr]): +def categorize_files_by_type(paths: List[PathOrStr]) -> CategorizedPaths: """ Splits list of filepaths into a 4 separate lists: easyconfigs, files to delete, patch files and files with extension .py """ - res = { - 'easyconfigs': [], - 'files_to_delete': [], - 'patch_files': [], - 'py_files': [], - } + res = CategorizedPaths([], [], [], []) for path in paths: if isinstance(path, Path): path = str(path) if path.startswith(':'): - res['files_to_delete'].append(path[1:]) + res.files_to_delete.append(path[1:]) elif path.endswith('.py'): - res['py_files'].append(path) + res.py_files.append(path) # file must exist in order to check whether it's a patch file elif os.path.isfile(path) and is_patch_file(path): - res['patch_files'].append(path) + res.patch_files.append(path) elif path.endswith('.patch'): if not os.path.exists(path): raise EasyBuildError('File %s does not exist, did you mistype the path?', path) - elif not os.path.isfile(path): + if not os.path.isfile(path): raise EasyBuildError('File %s is expected to be a regular file, but is a folder instead', path) - else: - raise EasyBuildError('%s is not detected as a valid patch file. Please verify its contents!', - path) + raise EasyBuildError('%s is not detected as a valid patch file. Please verify its contents!', path) else: # anything else is considered to be an easyconfig file - res['easyconfigs'].append(path) + res.easyconfigs.append(path) return res diff --git a/easybuild/main.py b/easybuild/main.py index dda45eb00d..22d14f2ee0 100755 --- a/easybuild/main.py +++ b/easybuild/main.py @@ -402,7 +402,7 @@ def process_eb_args(eb_args, eb_go, cfg_settings, modtool, testing, init_session no_ec_opts = [options.aggregate_regtest, options.regtest, any_pr_option_set, search_query] # determine paths to easyconfigs - determined_paths = det_easyconfig_paths(categorized_paths['easyconfigs']) + determined_paths = det_easyconfig_paths(categorized_paths.easyconfigs) # only copy easyconfigs here if we're not using --try-* (that's handled below) copy_ec = options.copy_ec and not tweaked_ecs_paths diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 9bb224537b..4440476a1d 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -48,6 +48,7 @@ from http import HTTPStatus from http.client import HTTPException from string import ascii_letters +from typing import List, NamedTuple from urllib.request import HTTPError, URLError, urlopen from easybuild.base import fancylogger @@ -90,6 +91,13 @@ _log.warning("Failed to import 'git' Python module: %s", err) +class CategorizedPaths(NamedTuple): + easyconfigs: List[str] + files_to_delete: List[str] + patch_files: List[str] + py_files: List[str] + + GITHUB_URL = 'https://github.com' GITHUB_API_URL = 'https://api.github.com' GITHUB_BRANCH_MAIN = 'main' @@ -1052,7 +1060,8 @@ def setup_repo(git_repo, target_account, target_repo, branch_name, silent=False, @only_if_module_is_available('git', pkgname='GitPython') -def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_account=None, commit_msg=None): +def _easyconfigs_pr_common(paths: CategorizedPaths, ecs, start_branch=None, pr_branch=None, + start_account=None, commit_msg=None): """ Common code for new_pr and update_pr functions: * check whether all supplied paths point to existing files @@ -1072,8 +1081,8 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ # we need files to create the PR with non_existing_paths = [] ec_paths = [] - if paths['easyconfigs'] or paths['py_files']: - for path in paths['easyconfigs'] + paths['py_files']: + if paths.easyconfigs or paths.py_files: + for path in paths.easyconfigs + paths.py_files: if not os.path.exists(path): non_existing_paths.append(path) else: @@ -1085,7 +1094,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ exit_code=EasyBuildExit.OPTION_ERROR ) - if not any(paths.values()): + if not any(paths): raise EasyBuildError("No paths specified", exit_code=EasyBuildExit.OPTION_ERROR) pr_target_repo = det_pr_target_repo(paths) @@ -1139,7 +1148,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ # figure out commit message to use if commit_msg: - if (pr_target_repo == GITHUB_EASYCONFIGS_REPO and all(file_info['new']) and not paths['files_to_delete'] + if (pr_target_repo == GITHUB_EASYCONFIGS_REPO and all(file_info['new']) and not paths.files_to_delete and is_new_pr): # Only if opening a new PR msg = "When only adding new easyconfigs a PR commit msg (--pr-commit-msg) should not be used, as " msg += "the PR title will be automatically generated." @@ -1150,11 +1159,11 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ raise EasyBuildError(msg) cnt = len(file_info['paths_in_repo']) _log.debug("Using specified commit message for all %d new/modified files at once: %s", cnt, commit_msg) - elif pr_target_repo == GITHUB_EASYCONFIGS_REPO and all(file_info['new']) and not paths['files_to_delete']: + elif pr_target_repo == GITHUB_EASYCONFIGS_REPO and all(file_info['new']) and not paths.files_to_delete: # automagically derive meaningful commit message if all easyconfig files are new commit_msg = "adding easyconfigs: %s" % ', '.join(os.path.basename(p) for p in file_info['paths_in_repo']) - if paths['patch_files']: - commit_msg += " and patches: %s" % ', '.join(os.path.basename(p) for p in paths['patch_files']) + if paths.patch_files: + commit_msg += " and patches: %s" % ', '.join(os.path.basename(p) for p in paths.patch_files) elif pr_target_repo == GITHUB_EASYBLOCKS_REPO and all(file_info['new']): commit_msg = "adding easyblocks: %s" % ', '.join(os.path.basename(p) for p in file_info['paths_in_repo']) else: @@ -1163,22 +1172,22 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ if not new] if modified_files: msg += '\nModified: ' + ', '.join(modified_files) - if paths['files_to_delete']: - msg += '\nDeleted: ' + ', '.join(paths['files_to_delete']) + if paths.files_to_delete: + msg += '\nDeleted: ' + ', '.join(paths.files_to_delete) raise EasyBuildError("A meaningful commit message must be specified via --pr-commit-msg when " "modifying/deleting files or targeting the framework repo." + msg, exit_code=EasyBuildExit.OPTION_ERROR) # figure out to which software name patches relate, and copy them to the right place - if paths['patch_files']: - patch_specs = det_patch_specs(paths['patch_files'], file_info, [target_dir]) + if paths.patch_files: + patch_specs = det_patch_specs(paths.patch_files, file_info, [target_dir]) print_msg("copying patch files to %s..." % target_dir) patch_info = copy_patch_files(patch_specs, target_dir) # determine path to files to delete (if any) deleted_paths = [] - for fn in paths['files_to_delete']: + for fn in paths.files_to_delete: fullpath = os.path.join(repo_path, fn) if os.path.exists(fullpath): deleted_paths.append(fullpath) @@ -1217,8 +1226,8 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ if pr_branch is None: if ec_paths and pr_target_repo == GITHUB_EASYCONFIGS_REPO: label = file_info['ecs'][0].name + re.sub('[.-]', '', file_info['ecs'][0].version) - elif pr_target_repo == GITHUB_EASYBLOCKS_REPO and paths.get('py_files'): - label = os.path.splitext(os.path.basename(paths['py_files'][0]))[0] + elif pr_target_repo == GITHUB_EASYBLOCKS_REPO and paths.py_files: + label = os.path.splitext(os.path.basename(paths.py_files[0]))[0] else: label = ''.join(random.choice(ascii_letters) for _ in range(10)) pr_branch = '%s_new_pr_%s' % (time.strftime("%Y%m%d%H%M%S"), label) @@ -1233,7 +1242,7 @@ def _easyconfigs_pr_common(paths, ecs, start_branch=None, pr_branch=None, start_ git_repo.index.add(file_info['paths_in_repo']) git_repo.index.add(dep_info['paths_in_repo']) - if paths['patch_files']: + if paths.patch_files: _log.debug("Staging all %d new/modified patch files", len(patch_info['paths_in_repo'])) git_repo.index.add(patch_info['paths_in_repo']) @@ -1913,7 +1922,7 @@ def add_pr_labels(pr, branch=GITHUB_DEVELOP_BRANCH): @only_if_module_is_available('git', pkgname='GitPython') -def new_branch_github(paths, ecs, commit_msg=None): +def new_branch_github(paths: CategorizedPaths, ecs, commit_msg=None): """ Create new branch on GitHub using specified files @@ -1971,7 +1980,8 @@ def det_pr_title(ecs): @only_if_module_is_available('git', pkgname='GitPython') -def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=None, pr_metadata=None, commit_msg=None): +def new_pr_from_branch(branch_name: CategorizedPaths, title=None, descr=None, + pr_target_repo=None, pr_metadata=None, commit_msg=None): """ Create new pull request from specified branch on GitHub. """ @@ -2152,7 +2162,7 @@ def new_pr_from_branch(branch_name, title=None, descr=None, pr_target_repo=None, print_msg("This PR should be labelled %s" % ', '.join(labels), log=_log, prefix=False) -def new_pr(paths, ecs, title=None, descr=None, commit_msg=None): +def new_pr(paths: CategorizedPaths, ecs, title=None, descr=None, commit_msg=None): """ Open new pull request using specified files @@ -2184,8 +2194,8 @@ def new_pr(paths, ecs, title=None, descr=None, commit_msg=None): raise EasyBuildError(msg, exit_code=EasyBuildExit.EASYCONFIG_ERROR) patch = patch_info['name'] - if patch not in paths['patch_files'] and not os.path.isfile(os.path.join(os.path.dirname(ec_path), - patch)): + if patch not in paths.patch_files and not os.path.isfile(os.path.join(os.path.dirname(ec_path), + patch)): print_warning("new patch file %s, referenced by %s, is not included in this PR" % (patch, ec.filename())) @@ -2226,7 +2236,7 @@ def det_account_branch_for_pr(pr_id, github_user=None, pr_target_repo=None): return account, branch -def det_pr_target_repo(paths): +def det_pr_target_repo(paths: CategorizedPaths): """Determine target repository for pull request from given cagetorized list of files :param paths: paths to categorized lists of files (easyconfigs, files to delete, patches, .py files) @@ -2234,21 +2244,18 @@ def det_pr_target_repo(paths): pr_target_repo = build_option('pr_target_repo') # determine target repository for PR based on which files are provided - # (see categorize_files_by_type function) if pr_target_repo is None: _log.info("Trying to derive target repository based on specified files...") - easyconfigs, files_to_delete, patch_files, py_files = [paths[key] for key in sorted(paths.keys())] - # Python files provided, and no easyconfig files or patches - if py_files and not (easyconfigs or patch_files): + if paths.py_files and not (paths.easyconfigs or paths.patch_files): _log.info("Only Python files provided, no easyconfig files or patches...") # if all Python files are easyblocks, target repo should be easyblocks; # otherwise, target repo is assumed to be framework - if all(get_easyblock_class_name(path) for path in py_files): + if all(get_easyblock_class_name(path) for path in paths.py_files): pr_target_repo = GITHUB_EASYBLOCKS_REPO _log.info("All Python files are easyblocks, target repository is assumed to be %s", pr_target_repo) else: @@ -2257,7 +2264,8 @@ def det_pr_target_repo(paths): # if no Python files are provided, only easyconfigs & patches, or if files to delete are .eb files, # then target repo is assumed to be easyconfigs - elif easyconfigs or patch_files or (files_to_delete and all(x.endswith('.eb') for x in files_to_delete)): + elif paths.easyconfigs or paths.patch_files or (paths.files_to_delete and all(x.endswith('.eb') + for x in paths.files_to_delete)): pr_target_repo = GITHUB_EASYCONFIGS_REPO _log.info("Only easyconfig and patch files found, target repository is assumed to be %s", pr_target_repo) @@ -2268,7 +2276,7 @@ def det_pr_target_repo(paths): @only_if_module_is_available('git', pkgname='GitPython') -def update_branch(branch_name, paths, ecs, github_account=None, commit_msg=None): +def update_branch(branch_name, paths: CategorizedPaths, ecs, github_account=None, commit_msg=None): """ Update specified branch in GitHub using specified files @@ -2303,7 +2311,7 @@ def update_branch(branch_name, paths, ecs, github_account=None, commit_msg=None) @only_if_module_is_available('git', pkgname='GitPython') -def update_pr(pr_id, paths, ecs, commit_msg=None): +def update_pr(pr_id, paths: CategorizedPaths, ecs, commit_msg=None): """ Update specified pull request using specified files diff --git a/test/framework/easyconfig.py b/test/framework/easyconfig.py index cb37fc0fc2..a9c53bec86 100644 --- a/test/framework/easyconfig.py +++ b/test/framework/easyconfig.py @@ -61,7 +61,7 @@ from easybuild.framework.easyconfig.style import check_easyconfigs_style from easybuild.framework.easyconfig.tools import alt_easyconfig_paths, categorize_files_by_type, check_sha256_checksums from easybuild.framework.easyconfig.tools import dep_graph, det_copy_ec_specs, find_related_easyconfigs, get_paths_for -from easybuild.framework.easyconfig.tools import parse_easyconfigs +from easybuild.framework.easyconfig.tools import parse_easyconfigs, CategorizedPaths from easybuild.framework.easyconfig.tweak import obtain_ec_for, tweak, tweak_one from easybuild.framework.extension import construct_exts_filter_cmds from easybuild.toolchains.system import SystemToolchain @@ -3815,8 +3815,7 @@ def test_hidden_toolchain(self): def test_categorize_files_by_type(self): """Test categorize_files_by_type""" - self.assertEqual({'easyconfigs': [], 'files_to_delete': [], 'patch_files': [], 'py_files': []}, - categorize_files_by_type([])) + self.assertEqual(categorize_files_by_type([]), CategorizedPaths([], [], [], [])) toy_patch_fn = 'toy-0.0_fix-silly-typo-in-printf-statement.patch' toy_patch = os.path.join(TEST_DIR, 'sandbox', 'sources', 'toy', toy_patch_fn) @@ -3841,10 +3840,10 @@ def test_categorize_files_by_type(self): str(gzip_ec), 'foo', ] - self.assertEqual(res['easyconfigs'], expected) - self.assertEqual(res['files_to_delete'], ['toy-0.0-deps.eb']) - self.assertEqual(res['patch_files'], [toy_patch]) - self.assertEqual(res['py_files'], [toy_easyblock, configuremake]) + self.assertEqual(res.easyconfigs, expected) + self.assertEqual(res.files_to_delete, ['toy-0.0-deps.eb']) + self.assertEqual(res.patch_files, [toy_patch]) + self.assertEqual(res.py_files, [toy_easyblock, configuremake]) # Error cases tmpdir = tempfile.mkdtemp() From 1da6939751fb9cd048b1795f4795d72da746858c Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Mon, 21 Sep 2026 15:46:24 +0200 Subject: [PATCH 2/7] Add comment on where `CategorizedPaths` lives --- easybuild/framework/easyconfig/tools.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/easybuild/framework/easyconfig/tools.py b/easybuild/framework/easyconfig/tools.py index 3b522b3d35..9061e24dcb 100644 --- a/easybuild/framework/easyconfig/tools.py +++ b/easybuild/framework/easyconfig/tools.py @@ -62,7 +62,7 @@ from easybuild.tools.environment import restore_env from easybuild.tools.filetools import EASYBLOCK_CLASS_PREFIX, PathOrStr, get_cwd, find_easyconfigs, is_patch_file from easybuild.tools.filetools import locate_files, read_file, resolve_path, which, write_file -from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO, CategorizedPaths +from easybuild.tools.github import GITHUB_EASYCONFIGS_REPO from easybuild.tools.github import det_pr_labels, det_pr_title, download_repo, fetch_easyconfigs_from_commit from easybuild.tools.github import fetch_easyconfigs_from_pr, fetch_pr_data from easybuild.tools.github import fetch_files_from_commit, fetch_files_from_pr @@ -72,6 +72,11 @@ from easybuild.tools.utilities import only_if_module_is_available, quote_str from easybuild.tools.version import VERSION as EASYBUILD_VERSION +# Belongs in this file and should be imported from here +# but needs to be defined in the github module to avoid cyclic imports +from easybuild.tools.github import CategorizedPaths + + # optional Python packages, these might be missing # failing imports are just ignored # a NameError should be caught where these are used From 87652ea8fc96d4612cb451fdb34c99da74389deb Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 4 Sep 2026 10:52:08 +0200 Subject: [PATCH 3/7] Make `CategorizedPaths` backwards compatible --- easybuild/tools/github.py | 84 +++++++++++++++++++++++++++++++++++++-- test/framework/github.py | 66 +++++++++++++++++++++++++++++- 2 files changed, 144 insertions(+), 6 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 4440476a1d..e5e1934f63 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -44,11 +44,12 @@ import tempfile import time import urllib.error +from collections.abc import MutableMapping from datetime import datetime, timedelta from http import HTTPStatus from http.client import HTTPException from string import ascii_letters -from typing import List, NamedTuple +from typing import ClassVar, List, Tuple from urllib.request import HTTPError, URLError, urlopen from easybuild.base import fancylogger @@ -91,24 +92,99 @@ _log.warning("Failed to import 'git' Python module: %s", err) -class CategorizedPaths(NamedTuple): +class CategorizedPaths(MutableMapping): # Todo: use NamedTuple or dataclass instead + """Named tuple for categorized paths, to avoid using a dictionary with string keys.""" easyconfigs: List[str] files_to_delete: List[str] patch_files: List[str] py_files: List[str] + _others: dict # Other keys than the above for backwards compatibility + _fields: ClassVar[Tuple[str, ...]] + + def __init__(self, easyconfigs: List[str], files_to_delete: List[str], + patch_files: List[str], py_files: List[str]): + self.easyconfigs = easyconfigs + self.files_to_delete = files_to_delete + self.patch_files = patch_files + self.py_files = py_files + self._others = {} + + def __getitem__(self, key): + _log.deprecated("Accessing CategorizedPaths via index is deprecated, use named attributes instead", '6.0') + try: + return getattr(self, key) + except AttributeError: + return self._others[key] + + def __setitem__(self, key, value): + _log.deprecated("Accessing CategorizedPaths via index is deprecated, use named attributes instead", '6.0') + if key in self._fields: + setattr(self, key, value) + else: + self._others[key] = value + + def __delitem__(self, key): + _log.deprecated("Accessing CategorizedPaths via index is deprecated, use named attributes instead", '6.0') + if key in self._fields: + raise KeyError(f"Cannot delete field '{key}' from CategorizedPaths") + else: + del self._others[key] + + def __contains__(self, key): + _log.deprecated("Using CategorizedPaths as a dictionary is deprecated, use named attributes instead", '6.0') + return key in self._fields or key in self._others + + def __len__(self): + _log.deprecated("Using CategorizedPaths as a dictionary is deprecated, use named attributes instead", '6.0') + return len(self._fields) + len(self._others) + + def __iter__(self): + _log.deprecated("Using CategorizedPaths as a dictionary is deprecated, use named attributes instead", '6.0') + return itertools.chain(self._fields, self._others) + + def __eq__(self, other): + if isinstance(other, CategorizedPaths): + return self._asdict() == other._asdict() + return self._asdict() == other + + def __ne__(self, other): + return not (self == other) + + def __repr__(self): + return repr(self._asdict()) + + def _asdict(self): + """Internal helper to return all items without the deprecation warning.""" + return dict(itertools.chain([(key, getattr(self, key)) for key in self._fields], self._others.items())) + + def keys(self): + _log.deprecated("Using CategorizedPaths as a dictionary is deprecated, use named attributes instead", '6.0') + return itertools.chain(self._fields, self._others) + + def values(self): + _log.deprecated("Using CategorizedPaths as a dictionary is deprecated, use named attributes instead", '6.0') + return self._asdict().values() + + def items(self): + _log.deprecated("Using CategorizedPaths as a dictionary is deprecated, use named attributes instead", '6.0') + return self._asdict().items() + + +CategorizedPaths._fields = tuple(name for name in CategorizedPaths.__annotations__ if not name.startswith('_')) + GITHUB_URL = 'https://github.com' GITHUB_API_URL = 'https://api.github.com' GITHUB_BRANCH_MAIN = 'main' GITHUB_BRANCH_MASTER = 'master' -GITHUB_DIR_TYPE = u'dir' +GITHUB_DIR_TYPE = 'dir' GITHUB_EB_MAIN = 'easybuilders' GITHUB_EASYBLOCKS_REPO = 'easybuild-easyblocks' GITHUB_EASYCONFIGS_REPO = 'easybuild-easyconfigs' GITHUB_FRAMEWORK_REPO = 'easybuild-framework' GITHUB_DEVELOP_BRANCH = 'develop' -GITHUB_FILE_TYPE = u'file' +GITHUB_FILE_TYPE = 'file' GITHUB_PR_STATE_OPEN = 'open' GITHUB_PR_STATES = [GITHUB_PR_STATE_OPEN, 'closed', 'all'] GITHUB_PR_ORDER_CREATED = 'created' diff --git a/test/framework/github.py b/test/framework/github.py index 1c3d7b0ee1..db1539e52c 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -131,6 +131,68 @@ def tearDown(self): super().tearDown() + def test_CategorizedPaths_backwards_compat(self): + """Test backwards compatibility of CategorizedPaths (use as dictionary).""" + paths = gh.CategorizedPaths( + easyconfigs=['foo.eb'], + files_to_delete=['bar.txt'], + patch_files=['baz.patch'], + py_files=['qux.py'], + ) + expected_keys = ['easyconfigs', 'files_to_delete', 'patch_files', 'py_files'] + expected_values = [['foo.eb'], ['bar.txt'], ['baz.patch'], ['qux.py']] + with self.temporarily_allow_deprecated_behaviour(), self.mocked_stderr(): + for key, expected_value in zip(expected_keys, expected_values): + self.assertEqual(paths[key], expected_value) + self.assertIn("Accessing CategorizedPaths via index is deprecated, use named attributes instead", + self.get_stderr()) + + self.assertEqual(list(paths.keys()), expected_keys) + self.assertEqual(list(paths), list(paths.keys())) # Iteration + self.assertEqual(list(paths.values()), expected_values) + self.assertEqual(list(paths.items()), list(zip(expected_keys, expected_values))) + self.assertIn("Using CategorizedPaths as a dictionary is deprecated, use named attributes instead", + self.get_stderr()) + self.assertEqual(len(paths), len(expected_keys)) + with self.assertRaises(KeyError): + _ = paths['non_existing_key'] + self.assertEqual(paths.get('non_existing_key', 'default'), 'default') + + # Add new dictionary entry + paths['new_field'] = ['newfile.txt'] + expected_keys.append('new_field') + expected_values.append(['newfile.txt']) + self.assertEqual(list(paths.keys()), expected_keys) + self.assertEqual(list(paths), list(paths.keys())) # Iteration + self.assertEqual(list(paths.values()), expected_values) + self.assertEqual(list(paths.items()), list(zip(expected_keys, expected_values))) + del paths['new_field'] + self.assertEqual(list(paths.keys()), expected_keys[:-1]) + + # Check that modifying a list in-place works as expected + paths['easyconfigs'] = ['newfoo.eb'] + self.assertEqual(paths.easyconfigs, ['newfoo.eb']) + paths['easyconfigs'].append('another.eb') + self.assertEqual(paths.easyconfigs, ['newfoo.eb', 'another.eb']) + + # Comparisons do not trigger deprecations + self.assertEqual(paths, paths) + self.assertNotEqual(paths, + gh.CategorizedPaths(easyconfigs=[], files_to_delete=[], patch_files=[], py_files=[])) + self.assertNotEqual(paths, {}) + self.assertNotEqual({}, paths) + paths2 = gh.CategorizedPaths(easyconfigs=['foo.eb'], files_to_delete=['bar.txt'], patch_files=['foo.patch'], + py_files=['foo.py']) + path_dict = {'easyconfigs': ['foo.eb'], 'files_to_delete': ['bar.txt'], 'patch_files': ['foo.patch'], + 'py_files': ['foo.py']} + with self.temporarily_allow_deprecated_behaviour(), self.mocked_stderr(): + # Converting to dict triggers deprecation warning + self.assertEqual(dict(paths2), path_dict) + # Direct comparison to dict does not + self.assertEqual(paths2, path_dict) + self.assertEqual(path_dict, paths2) + self.assertEqual(repr(paths2), repr(path_dict)) + def test_det_pr_title(self): """Test det_pr_title function""" rawtxt = textwrap.dedent(""" @@ -675,7 +737,7 @@ def test_install_github_token(self): # poor mans mocking of getpass # inject leading/trailing spaces to verify stripping of provided value - def fake_getpass(*args, **kwargs): + def fake_getpass(*_args, **_kwargs): return ' ' + self.github_token + ' ' orig_getpass = gh.getpass.getpass @@ -1369,7 +1431,7 @@ def test_github_create_test_report(self): self.assertNotIn(pattern.lower(), res['full']) # mock create_gist function, we don't want to actually create a gist every time we run this test... - def fake_create_gist(*args, **kwargs): + def fake_create_gist(*_args, **_kwargs): return 'https://gist.github.com/%s/test' % GITHUB_TEST_ACCOUNT easybuild.tools.testing.create_gist = fake_create_gist From 27947b01beacb049bbf06f21425977439244954a Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 4 Sep 2026 17:49:48 +0200 Subject: [PATCH 4/7] Avoid test failure in `test_list_software` The test wrongly matched: > 'gzip' unexpectedly found in '== Temporary log file in case of crash /tmp/eb-2gzipb3w/eb-o3rmuvme/eb-p0oq22mq/eb-test-9go8uv1x.log i.e. it matched the path. Use whole-word regex to avoid the short string. --- test/framework/options.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/framework/options.py b/test/framework/options.py index 2883285639..bf5574abf3 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -5911,7 +5911,7 @@ def test_list_software(self): ] self.assertMultiRegex(patterns, txt, multi_line=True) - self.assertNotIn('gzip', txt) + self.assertNotRegex(txt, r'\bgzip\b') self.assertNotIn('CrayCCE', txt) args = [ @@ -5930,7 +5930,7 @@ def test_list_software(self): ] self.assertMultiRegex(patterns, txt, multi_line=True) - self.assertNotIn('gzip', txt) + self.assertNotRegex(txt, r'\bgzip\b') self.assertNotIn('CrayCCE', txt) def test_parse_optarch(self): From d9fa297fc57034035d2ccca860060c688a1e6731 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 9 Sep 2026 09:45:31 +0200 Subject: [PATCH 5/7] Enhance comment on deprecation --- easybuild/tools/github.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index e5e1934f63..bca105de94 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -92,13 +92,16 @@ _log.warning("Failed to import 'git' Python module: %s", err) -class CategorizedPaths(MutableMapping): # Todo: use NamedTuple or dataclass instead +# Todo in EasyBuild 6: Use NamedTuple or dataclass instead the dict-interface is no longer required +class CategorizedPaths(MutableMapping): """Named tuple for categorized paths, to avoid using a dictionary with string keys.""" easyconfigs: List[str] files_to_delete: List[str] patch_files: List[str] py_files: List[str] + # All members below are solely for backwards compatibility and can be removed in EB 6 + _others: dict # Other keys than the above for backwards compatibility _fields: ClassVar[Tuple[str, ...]] From 4c0ad1a93ff82fcf757c810c6e07f75dbc0d81dc Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 9 Sep 2026 16:33:34 +0200 Subject: [PATCH 6/7] Allow a dict instance passed to public GitHub functions --- easybuild/tools/github.py | 30 ++++++++++++++++++++++++++++++ test/framework/github.py | 11 ++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index bca105de94..9f17ba5c14 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -113,6 +113,14 @@ def __init__(self, easyconfigs: List[str], files_to_delete: List[str], self.py_files = py_files self._others = {} + @classmethod + def _from_dict(cls, data: dict) -> "CategorizedPaths": + """Create a CategorizedPaths instance from a dictionary.""" + known_values = {field: data.get(field, []) for field in CategorizedPaths._fields} + obj = CategorizedPaths(**known_values) + obj._others = {key: value for key, value in data.items() if key not in CategorizedPaths._fields} + return obj + def __getitem__(self, key): _log.deprecated("Accessing CategorizedPaths via index is deprecated, use named attributes instead", '6.0') try: @@ -2009,6 +2017,11 @@ def new_branch_github(paths: CategorizedPaths, ecs, commit_msg=None): :param ecs: list of parsed easyconfigs, incl. for dependencies (if robot is enabled) :param commit_msg: commit message to use """ + if isinstance(paths, dict): + _log.deprecated("`paths` argument to new_branch_github should be a CategorizedPaths instance " + "instead of a dict", '6.0') + paths = CategorizedPaths._from_dict(paths) + branch_name = build_option('pr_branch_name') if commit_msg is None: commit_msg = build_option('pr_commit_msg') @@ -2252,6 +2265,10 @@ def new_pr(paths: CategorizedPaths, ecs, title=None, descr=None, commit_msg=None :param commit_msg: commit message to use """ + if isinstance(paths, dict): + _log.deprecated("`paths` argument to new_pr should be a CategorizedPaths instance instead of a dict", '6.0') + paths = CategorizedPaths._from_dict(paths) + if commit_msg is None: commit_msg = build_option('pr_commit_msg') @@ -2320,6 +2337,11 @@ def det_pr_target_repo(paths: CategorizedPaths): :param paths: paths to categorized lists of files (easyconfigs, files to delete, patches, .py files) """ + if isinstance(paths, dict): + _log.deprecated("`paths` argument to det_pr_target_repo should be a CategorizedPaths instance " + "instead of a dict", '6.0') + paths = CategorizedPaths._from_dict(paths) + pr_target_repo = build_option('pr_target_repo') # determine target repository for PR based on which files are provided @@ -2364,6 +2386,11 @@ def update_branch(branch_name, paths: CategorizedPaths, ecs, github_account=None :param ecs: list of parsed easyconfigs, incl. for dependencies (if robot is enabled) :param commit_msg: commit message to use """ + if isinstance(paths, dict): + _log.deprecated("`paths` argument to update_branch should be a CategorizedPaths instance " + "instead of a dict", '6.0') + paths = CategorizedPaths._from_dict(paths) + if commit_msg is None: commit_msg = build_option('pr_commit_msg') @@ -2399,6 +2426,9 @@ def update_pr(pr_id, paths: CategorizedPaths, ecs, commit_msg=None): :param ecs: list of parsed easyconfigs, incl. for dependencies (if robot is enabled) :param commit_msg: commit message to use """ + if isinstance(paths, dict): + _log.deprecated("`paths` argument to update_pr should be a CategorizedPaths instance instead of a dict", '6.0') + paths = CategorizedPaths._from_dict(paths) pr_target_repo = det_pr_target_repo(paths) if pr_target_repo is None: diff --git a/test/framework/github.py b/test/framework/github.py index db1539e52c..519a6f5a0c 100644 --- a/test/framework/github.py +++ b/test/framework/github.py @@ -188,11 +188,20 @@ def test_CategorizedPaths_backwards_compat(self): with self.temporarily_allow_deprecated_behaviour(), self.mocked_stderr(): # Converting to dict triggers deprecation warning self.assertEqual(dict(paths2), path_dict) - # Direct comparison to dict does not + # Direct comparison to dict does not trigger deprecation warning self.assertEqual(paths2, path_dict) self.assertEqual(path_dict, paths2) self.assertEqual(repr(paths2), repr(path_dict)) + self.assertEqual(gh.CategorizedPaths._from_dict(path_dict), path_dict) + path_dict["unrelated"] = "Anything" + self.assertEqual(gh.CategorizedPaths._from_dict(path_dict), path_dict) + # Not all key + path_dict = {'easyconfigs': ['foo.eb'], 'files_to_delete': ['bar.txt']} + expected = dict(path_dict) + expected.update({'patch_files': [], 'py_files': []}) + self.assertEqual(gh.CategorizedPaths._from_dict(path_dict), expected) + def test_det_pr_title(self): """Test det_pr_title function""" rawtxt = textwrap.dedent(""" From 9050d2f7db957bb833a62b30449a62b00236a53e Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 9 Sep 2026 16:33:48 +0200 Subject: [PATCH 7/7] Add type hints --- easybuild/tools/github.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/easybuild/tools/github.py b/easybuild/tools/github.py index 9f17ba5c14..4deb37f934 100644 --- a/easybuild/tools/github.py +++ b/easybuild/tools/github.py @@ -49,7 +49,7 @@ from http import HTTPStatus from http.client import HTTPException from string import ascii_letters -from typing import ClassVar, List, Tuple +from typing import Any, ClassVar, Dict, List, Optional, Tuple from urllib.request import HTTPError, URLError, urlopen from easybuild.base import fancylogger @@ -1147,8 +1147,9 @@ def setup_repo(git_repo, target_account, target_repo, branch_name, silent=False, @only_if_module_is_available('git', pkgname='GitPython') -def _easyconfigs_pr_common(paths: CategorizedPaths, ecs, start_branch=None, pr_branch=None, - start_account=None, commit_msg=None): +def _easyconfigs_pr_common(paths: CategorizedPaths, ecs: Dict[str, Any], start_branch: Optional[str] = None, + pr_branch: Optional[str] = None, start_account: Optional[str] = None, + commit_msg: Optional[str] = None): """ Common code for new_pr and update_pr functions: * check whether all supplied paths point to existing files @@ -1358,7 +1359,7 @@ def _easyconfigs_pr_common(paths: CategorizedPaths, ecs, start_branch=None, pr_b return file_info, deleted_paths, git_repo, pr_branch, diff_stat, pr_target_repo -def create_remote(git_repo, account, repo, https=False): +def create_remote(git_repo: str, account: str, repo: str, https=False): """ Create remote in specified git working directory for specified account & repository. @@ -2009,7 +2010,7 @@ def add_pr_labels(pr, branch=GITHUB_DEVELOP_BRANCH): @only_if_module_is_available('git', pkgname='GitPython') -def new_branch_github(paths: CategorizedPaths, ecs, commit_msg=None): +def new_branch_github(paths: CategorizedPaths, ecs: Dict[str, Any], commit_msg: Optional[str] = None): """ Create new branch on GitHub using specified files @@ -2072,7 +2073,7 @@ def det_pr_title(ecs): @only_if_module_is_available('git', pkgname='GitPython') -def new_pr_from_branch(branch_name: CategorizedPaths, title=None, descr=None, +def new_pr_from_branch(branch_name: str, title=None, descr=None, pr_target_repo=None, pr_metadata=None, commit_msg=None): """ Create new pull request from specified branch on GitHub. @@ -2254,7 +2255,8 @@ def new_pr_from_branch(branch_name: CategorizedPaths, title=None, descr=None, print_msg("This PR should be labelled %s" % ', '.join(labels), log=_log, prefix=False) -def new_pr(paths: CategorizedPaths, ecs, title=None, descr=None, commit_msg=None): +def new_pr(paths: CategorizedPaths, ecs: Dict[str, Any], title: Optional[str] = None, descr: Optional[str] = None, + commit_msg: Optional[str] = None): """ Open new pull request using specified files @@ -2377,7 +2379,8 @@ def det_pr_target_repo(paths: CategorizedPaths): @only_if_module_is_available('git', pkgname='GitPython') -def update_branch(branch_name, paths: CategorizedPaths, ecs, github_account=None, commit_msg=None): +def update_branch(branch_name: Optional[str], paths: CategorizedPaths, ecs: Dict[str, Any], + github_account: Optional[str] = None, commit_msg: Optional[str] = None): """ Update specified branch in GitHub using specified files