From c6aa06f5e65301473ad42b2b64f1be1853dc05f4 Mon Sep 17 00:00:00 2001 From: Lin Guo Date: Thu, 10 Sep 2026 12:13:19 -0700 Subject: [PATCH] Add more type checking to Ramble Signed-off-by: Lin Guo --- lib/ramble/ramble/analysis/backwards.py | 7 +-- lib/ramble/ramble/analysis/forward.py | 9 ++-- lib/ramble/ramble/cmd/__init__.py | 3 +- lib/ramble/ramble/cmd/common/arguments.py | 12 ++++- lib/ramble/ramble/cmd/common/info.py | 18 +++---- lib/ramble/ramble/cmd/common/list.py | 4 +- lib/ramble/ramble/cmd/config.py | 8 ++- lib/ramble/ramble/cmd/data.py | 2 +- lib/ramble/ramble/cmd/edit.py | 2 + lib/ramble/ramble/cmd/filter_groups.py | 5 +- lib/ramble/ramble/cmd/license.py | 5 +- lib/ramble/ramble/cmd/python.py | 4 +- lib/ramble/ramble/cmd/repo.py | 6 +-- lib/ramble/ramble/cmd/style.py | 13 +++-- lib/ramble/ramble/cmd/unit_test.py | 4 +- lib/ramble/ramble/cmd/workspace.py | 12 ++--- lib/ramble/ramble/config.py | 35 +++++++------ lib/ramble/ramble/expander.py | 12 ++--- lib/ramble/ramble/experiment_result.py | 3 +- lib/ramble/ramble/fetch_strategy.py | 50 +++++++++++++++---- lib/ramble/ramble/graphs.py | 2 +- lib/ramble/ramble/keywords.py | 4 ++ .../ramble/language/application_language.py | 4 +- lib/ramble/ramble/language/language_base.py | 23 +++++---- .../ramble/language/language_helpers.py | 6 +-- lib/ramble/ramble/language/shared_language.py | 4 +- lib/ramble/ramble/main.py | 21 +++++--- lib/ramble/ramble/mirror.py | 2 +- lib/ramble/ramble/paths.py | 3 ++ lib/ramble/ramble/pipeline.py | 11 ++-- lib/ramble/ramble/renderer.py | 11 ++-- lib/ramble/ramble/reports.py | 36 +++++++------ lib/ramble/ramble/repository.py | 42 ++++++++++------ lib/ramble/ramble/results_table.py | 18 ++++++- lib/ramble/ramble/software_environments.py | 24 +++++---- lib/ramble/ramble/stage.py | 16 +++--- lib/ramble/ramble/success_criteria.py | 8 +-- lib/ramble/ramble/test/cache_fetch.py | 12 +++++ lib/ramble/ramble/test/mirror.py | 12 +++++ lib/ramble/ramble/uploader.py | 6 ++- lib/ramble/ramble/variants.py | 4 +- lib/ramble/ramble/workload.py | 2 +- lib/ramble/ramble/workspace/workspace.py | 37 +++++++------- pyproject.toml | 12 ++--- 44 files changed, 339 insertions(+), 195 deletions(-) diff --git a/lib/ramble/ramble/analysis/backwards.py b/lib/ramble/ramble/analysis/backwards.py index ff41d11ecd..10dea1ccb6 100644 --- a/lib/ramble/ramble/analysis/backwards.py +++ b/lib/ramble/ramble/analysis/backwards.py @@ -9,6 +9,7 @@ """Define the backwards-reading analysis strategy""" import os +from typing import Any, Dict, FrozenSet, Tuple import ramble.success_criteria import ramble.util.lock as lk @@ -121,9 +122,9 @@ def __call__(self, workspace): exp_lock = app.experiment_lock - fom_values = {} - context_metadata = {} - null_key = (_NULL_CONTEXT, _NULL_CONTEXT, frozenset()) + fom_values: Dict[Tuple[str, str, FrozenSet[Any]], Dict[str, Any]] = {} + context_metadata: Dict[Tuple[str, str, FrozenSet[Any]], Dict[str, Any]] = {} + null_key: Tuple[str, str, FrozenSet[Any]] = (_NULL_CONTEXT, _NULL_CONTEXT, frozenset()) context_metadata[null_key] = { "name": _NULL_CONTEXT, "def_name": _NULL_CONTEXT, diff --git a/lib/ramble/ramble/analysis/forward.py b/lib/ramble/ramble/analysis/forward.py index b1aff9b813..7159fae73a 100644 --- a/lib/ramble/ramble/analysis/forward.py +++ b/lib/ramble/ramble/analysis/forward.py @@ -10,6 +10,7 @@ import os import string +from typing import Any, Dict, FrozenSet, Tuple import ramble.success_criteria import ramble.util.lock as lk @@ -61,9 +62,9 @@ def format_context(context_match, context_format): exp_lock = app.experiment_lock - fom_values = {} - context_metadata = {} - null_key = (_NULL_CONTEXT, _NULL_CONTEXT, frozenset()) + fom_values: Dict[Tuple[str, str, FrozenSet[Any]], Dict[str, Any]] = {} + context_metadata: Dict[Tuple[str, str, FrozenSet[Any]], Dict[str, Any]] = {} + null_key: Tuple[str, str, FrozenSet[Any]] = (_NULL_CONTEXT, _NULL_CONTEXT, frozenset()) context_metadata[null_key] = { "name": _NULL_CONTEXT, "def_name": _NULL_CONTEXT, @@ -75,7 +76,7 @@ def format_context(context_match, context_format): for file, file_conf in files.items(): # Start with no active contexts in a file. - active_contexts = {} + active_contexts: Dict[str, Any] = {} logger.debug(f"Reading log file: {file}") if not os.path.exists(file): diff --git a/lib/ramble/ramble/cmd/__init__.py b/lib/ramble/ramble/cmd/__init__.py index 597c0e4f00..5e32854b56 100644 --- a/lib/ramble/ramble/cmd/__init__.py +++ b/lib/ramble/ramble/cmd/__init__.py @@ -9,6 +9,7 @@ import os import re +from typing import List, Optional from llnl.util.lang import attr_setdefault @@ -50,7 +51,7 @@ def require_cmd_name(cname): #: global, cached list of all commands -- access through all_commands() -_all_commands = None +_all_commands: Optional[List[str]] = None def all_commands(): diff --git a/lib/ramble/ramble/cmd/common/arguments.py b/lib/ramble/ramble/cmd/common/arguments.py index 0b807c8265..ac602d17ca 100644 --- a/lib/ramble/ramble/cmd/common/arguments.py +++ b/lib/ramble/ramble/cmd/common/arguments.py @@ -13,7 +13,17 @@ from ramble.util.logger import logger -from spack.util.pattern import Args + +class Args: + """Class to hold positional flags and keyword arguments for parser.add_argument.""" + + flags: tuple + kwargs: dict + + def __init__(self, *flags, **kwargs): + self.flags = tuple(flags) + self.kwargs = kwargs + __all__ = [ "add_common_arguments", diff --git a/lib/ramble/ramble/cmd/common/info.py b/lib/ramble/ramble/cmd/common/info.py index c5038ed5e1..2980c807e8 100644 --- a/lib/ramble/ramble/cmd/common/info.py +++ b/lib/ramble/ramble/cmd/common/info.py @@ -9,6 +9,7 @@ import enum import fnmatch import textwrap +from collections.abc import Iterable from llnl.util.tty.colify import colified @@ -376,7 +377,7 @@ def print_single_attribute(obj, attr, verbose=False, pattern="*", format=support # Otherwise, we print the attribute's value directly. if isinstance(internal_attr, dict): to_print = list(internal_attr.keys()) - elif hasattr(internal_attr, "default_variants"): + elif internal_attr is not None and hasattr(internal_attr, "default_variants"): to_print = [] for variant in internal_attr.default_variants.values(): to_print.append(variant) @@ -387,7 +388,7 @@ def print_single_attribute(obj, attr, verbose=False, pattern="*", format=support to_print.append(variant) for variant in internal_attr.version_variants.values(): to_print.append(variant) - elif hasattr(internal_attr, "family_type"): + elif internal_attr is not None and hasattr(internal_attr, "family_type"): to_print = [f"{internal_attr.family_type}={family}" for family in internal_attr] else: to_print = internal_attr @@ -396,9 +397,7 @@ def print_single_attribute(obj, attr, verbose=False, pattern="*", format=support # if it's a list of dicts, convert the keys like above and print # otherwise filter it and print using the format specification # Otherwise, print it as a raw string. - if isinstance(to_print, (list, set, tuple)) or ( - hasattr(to_print, "__iter__") and not isinstance(to_print, str) - ): + if isinstance(to_print, Iterable) and not isinstance(to_print, str): to_print = list(to_print) if ( internal_attr @@ -415,9 +414,10 @@ def print_single_attribute(obj, attr, verbose=False, pattern="*", format=support if isinstance(internal_attr, dict): _print_verbose_dict_attr(internal_attr, pattern=pattern, indentation=indentation) elif ( - isinstance(internal_attr, (list, set, tuple)) - or (hasattr(internal_attr, "__iter__") and not isinstance(internal_attr, str)) - ) and not hasattr(internal_attr, "as_str"): + isinstance(internal_attr, Iterable) + and not isinstance(internal_attr, str) + and not hasattr(internal_attr, "as_str") + ): internal_list = list(internal_attr) # If it's a list of dicts, print each if internal_list and isinstance(internal_list[0], dict): @@ -440,7 +440,7 @@ def print_single_attribute(obj, attr, verbose=False, pattern="*", format=support color.cprint(f"{colified(to_print, tty=True, indent=4)}") color.cprint("") else: - if hasattr(internal_attr, "as_str"): + if internal_attr is not None and hasattr(internal_attr, "as_str"): color.cprint(internal_attr.as_str(verbose=True)) else: color.cprint(f"{indentation}{internal_attr}\n") diff --git a/lib/ramble/ramble/cmd/common/list.py b/lib/ramble/ramble/cmd/common/list.py index 440a456107..fcd285228b 100644 --- a/lib/ramble/ramble/cmd/common/list.py +++ b/lib/ramble/ramble/cmd/common/list.py @@ -169,8 +169,8 @@ def perform_list(args): # Filter by tags if args.tags: objects_with_tags = set(ramble.repository.paths[object_type].objects_with_tags(*args.tags)) - sorted_objects = set(sorted_objects) & objects_with_tags - sorted_objects = sorted(sorted_objects) + matching_tags = set(sorted_objects) & objects_with_tags + sorted_objects = sorted(matching_tags) if not sorted_objects: filter_strs = [] diff --git a/lib/ramble/ramble/cmd/config.py b/lib/ramble/ramble/cmd/config.py index f92991596e..1316ec13bb 100644 --- a/lib/ramble/ramble/cmd/config.py +++ b/lib/ramble/ramble/cmd/config.py @@ -26,6 +26,8 @@ section = "config" level = "long" +_add_parser = None + def setup_parser(subparser): scopes_metavar = ramble.config.scopes_metavar @@ -94,7 +96,8 @@ def setup_parser(subparser): ) # Make the add parser available later - setup_parser.add_parser = add_parser + global _add_parser + _add_parser = add_parser update = sp.add_parser("update", help="update configuration files to the latest format") ramble.cmd.common.arguments.add_common_arguments(update, ["yes_to_all"]) @@ -207,7 +210,8 @@ def config_add(args): This is a stateful operation that edits the config files.""" if not (args.file or args.path): logger.error("No changes requested. Specify a file or value.") - setup_parser.add_parser.print_help() + if _add_parser: + _add_parser.print_help() exit(1) scope, _ = _get_scope_and_section(args) diff --git a/lib/ramble/ramble/cmd/data.py b/lib/ramble/ramble/cmd/data.py index 3e302ef955..606721e01a 100644 --- a/lib/ramble/ramble/cmd/data.py +++ b/lib/ramble/ramble/cmd/data.py @@ -42,7 +42,7 @@ def data_create_db(args): raise ConfigError(f"Upload type {uploader_type_str} is not valid.") uploader_type = getattr(ramble.uploader.uploader_types, uploader_type_str) - + uploader: ramble.uploader.Uploader if uploader_type == ramble.uploader.uploader_types.BigQuery: uploader = ramble.uploader.BigQueryUploader() elif uploader_type == ramble.uploader.uploader_types.SQLite: diff --git a/lib/ramble/ramble/cmd/edit.py b/lib/ramble/ramble/cmd/edit.py index 3750833f77..162708591a 100644 --- a/lib/ramble/ramble/cmd/edit.py +++ b/lib/ramble/ramble/cmd/edit.py @@ -8,6 +8,7 @@ import glob import os +from typing import Any import ramble.cmd import ramble.paths @@ -238,6 +239,7 @@ def edit(parser, args): # It's an object type. Let's find what path it would have been at. try: obj_type = ramble.repository.ObjectTypes[type_name] + repo: Any if args.repo: repo = ramble.repository.Repo(args.repo, object_type=obj_type) elif effective_namespace: diff --git a/lib/ramble/ramble/cmd/filter_groups.py b/lib/ramble/ramble/cmd/filter_groups.py index b77e6d6c30..dc19c19440 100644 --- a/lib/ramble/ramble/cmd/filter_groups.py +++ b/lib/ramble/ramble/cmd/filter_groups.py @@ -7,6 +7,7 @@ # except according to those terms. import copy +from typing import Any, Dict, List from llnl.util.tty.colify import colify @@ -203,7 +204,7 @@ def print_filter_groups(resolved_scope_name=None, original_scope_name=None, verb lines.extend(f" - {ew}" for ew in definition["exclude_where"]) color.cprint("\n".join(lines)) else: - scope_groups = {} + scope_groups: Dict[str, List[str]] = {} for item in groups_to_print: scope = item["scope"] name = item["name"] @@ -212,7 +213,7 @@ def print_filter_groups(resolved_scope_name=None, original_scope_name=None, verb scope_groups[scope].append(name) out_stream = logger.active_stream() - colify_opts = {"indent": 4, "padding": 2} + colify_opts: Dict[str, Any] = {"indent": 4, "padding": 2} if out_stream: colify_opts["output"] = out_stream diff --git a/lib/ramble/ramble/cmd/license.py b/lib/ramble/ramble/cmd/license.py index 6d4f6d335b..f74b9c1be1 100644 --- a/lib/ramble/ramble/cmd/license.py +++ b/lib/ramble/ramble/cmd/license.py @@ -42,7 +42,7 @@ def _object_file_regex_list(is_ramble_root=True): #: regular expressions for licensed files. -licensed_files = [ +_raw_licensed_files = [ # ramble scripts r"bin/ramble$", r"bin/ramble-python$", @@ -66,6 +66,7 @@ def _object_file_regex_list(is_ramble_root=True): # examples r"examples/.*\.yaml$", ] +licensed_files = [re.compile(regex) for regex in _raw_licensed_files] #: licensed files that can have LGPL language in them @@ -327,8 +328,6 @@ def license(parser, args): if not git: logger.die("ramble license requires git in your environment") - licensed_files[:] = [re.compile(regex) for regex in licensed_files] - commands = { "list-files": list_files, "verify": verify, diff --git a/lib/ramble/ramble/cmd/python.py b/lib/ramble/ramble/cmd/python.py index caad1329bc..ed44a270f7 100644 --- a/lib/ramble/ramble/cmd/python.py +++ b/lib/ramble/ramble/cmd/python.py @@ -81,8 +81,8 @@ def python(parser, args, unknown_args): # Run user choice of interpreter if args.python_interpreter == "ipython": - return ramble.cmd.python.ipython_interpreter(args) - return ramble.cmd.python.python_interpreter(args) + return ipython_interpreter(args) + return python_interpreter(args) def ipython_interpreter(args): diff --git a/lib/ramble/ramble/cmd/repo.py b/lib/ramble/ramble/cmd/repo.py index fb7b3375bc..d0a4b56bf7 100644 --- a/lib/ramble/ramble/cmd/repo.py +++ b/lib/ramble/ramble/cmd/repo.py @@ -286,7 +286,7 @@ def repo_add(args): def repo_remove(args): """Remove a repository from Ramble's configuration.""" if args.type == "any": - obj_types = ramble.repository.ObjectTypes + obj_types = list(ramble.repository.ObjectTypes) else: obj_types = [ramble.repository.ObjectTypes[args.type]] @@ -294,7 +294,7 @@ def repo_remove(args): scopes_to_check = [args.scope] else: # Highest precedence first - scopes_to_check = reversed([s.name for s in ramble.config.config.file_scopes]) + scopes_to_check = list(reversed([s.name for s in ramble.config.config.file_scopes])) repo_removed = False for scope in scopes_to_check: @@ -315,7 +315,7 @@ def repo_remove(args): def repo_list(args): """Show registered repositories and their namespaces.""" if args.type == "any": - obj_types = ramble.repository.ObjectTypes + obj_types = list(ramble.repository.ObjectTypes) else: obj_types = [ramble.repository.ObjectTypes[args.type]] diff --git a/lib/ramble/ramble/cmd/style.py b/lib/ramble/ramble/cmd/style.py index 43a000e3a0..205ead5e54 100644 --- a/lib/ramble/ramble/cmd/style.py +++ b/lib/ramble/ramble/cmd/style.py @@ -16,7 +16,7 @@ import shutil import sys import tempfile -from typing import Callable, Dict +from typing import Callable, Dict, Tuple from llnl.util.filesystem import mkdirp, working_dir @@ -89,7 +89,7 @@ def is_object(f): # # For each file, if the filename pattern matches, we'll add per-line # exemptions if any patterns in the sub-dict match. -pattern_exemptions = { +_raw_pattern_exemptions = { # exemptions applied only to application.py files. rf"application.py|{base_class_file}$": { # Allow 'from ramble.appkit import *' in applications, @@ -144,7 +144,7 @@ def is_object(f): re.compile(file_pattern): { code: [re.compile(p) for p in patterns] for code, patterns in error_dict.items() } - for file_pattern, error_dict in pattern_exemptions.items() + for file_pattern, error_dict in _raw_pattern_exemptions.items() } # Tools run in the given order @@ -520,7 +520,10 @@ def run_black(black_cmd, file_list, args): if ver in supported_versions: target_args.extend(["--target-version", ver]) - common_args = ("--config", os.path.join(ramble.paths.prefix, "pyproject.toml")) + common_args: Tuple[str, ...] = ( + "--config", + os.path.join(ramble.paths.prefix, "pyproject.toml"), + ) if not args.fix: common_args += ("--check", "--diff") common_args += tuple(target_args) @@ -556,7 +559,7 @@ def run_black(black_cmd, file_list, args): @tool("isort") def run_isort(isort_cmd, file_list, args): - isort_args = ("--sp", os.path.join(ramble.paths.prefix, "pyproject.toml")) + isort_args: Tuple[str, ...] = ("--sp", os.path.join(ramble.paths.prefix, "pyproject.toml")) if not args.fix: isort_args += ("--check", "--diff") isort_args += tuple(get_tool_args(args, "isort")) diff --git a/lib/ramble/ramble/cmd/unit_test.py b/lib/ramble/ramble/cmd/unit_test.py index 6b39e4942c..ab49642fc6 100644 --- a/lib/ramble/ramble/cmd/unit_test.py +++ b/lib/ramble/ramble/cmd/unit_test.py @@ -185,9 +185,9 @@ def colorize(c, prefix): elif args.list == "long": for prefix, functions in sorted(tests.items()): path = colorize("*B", prefix) + "::" - functions = [colorize("c", f) for f in sorted(functions)] + color_functions = [colorize("c", f) for f in sorted(functions)] color.cprint(path) - colify(functions, indent=4) + colify(color_functions, indent=4) print() else: # args.list == "names" all_functions = [ diff --git a/lib/ramble/ramble/cmd/workspace.py b/lib/ramble/ramble/cmd/workspace.py index fd6a374ef6..33b7173568 100644 --- a/lib/ramble/ramble/cmd/workspace.py +++ b/lib/ramble/ramble/cmd/workspace.py @@ -12,7 +12,7 @@ import sys import tempfile from collections import defaultdict -from typing import Callable, Dict +from typing import Any, Callable, Dict, Set, Tuple import deprecation @@ -697,7 +697,7 @@ def workspace_push_to_cache(args): ws = ramble.cmd.require_active_workspace("workspace pushtocache", args.dry_run) filters = ramble.filters.Filters( - phase_filters="*", + phase_filters=["*"], include_where_filters=args.where, exclude_where_filters=args.exclude_where, tags=args.filter_tags, @@ -898,7 +898,7 @@ def workspace_info(args): # We built a "print_experiment_set" to access the scopes of variables for each # experiment, rather than having merged scopes as we do in the base experiment_set. # The base experiment_set is used to list *all* experiments. - all_pipelines = {} + all_pipelines: Dict[str, Set[str]] = {} color.cprint("") color.cprint(color.section_title("Experiments:")) @@ -1051,7 +1051,7 @@ def workspace_info(args): if args.variants: color.cprint(color.nested_4(" Variants: ")) - variant_set = set() + variant_set: Set[str] = set() for _, obj in app_inst.objects(): variant_set = variant_set.union( obj.experiment_variants().as_set( @@ -1092,7 +1092,7 @@ def workspace_info(args): for pipeline in sorted(all_pipelines.keys()): color.cprint("") color.cprint(color.section_title(f"Phases for {pipeline} pipeline:")) - colify(all_pipelines[pipeline], indent=4) + colify(sorted(all_pipelines[pipeline]), indent=4) # Print software stack information if args.software or args.all_software: @@ -1109,7 +1109,7 @@ def workspace_info(args): color.cprint("") color.cprint(color.section_title("Bootstrapped Utilities:")) - all_utilities = {} + all_utilities: Dict[str, Set[Tuple[Any, ...]]] = {} for workloads, _application_context in ws.all_applications(): for experiments, _workload_context in ws.all_workloads(workloads): for _exp_contents, _experiment_context in ws.all_experiments(experiments): diff --git a/lib/ramble/ramble/config.py b/lib/ramble/ramble/config.py index 3a51c0c3f0..40b40cf04c 100644 --- a/lib/ramble/ramble/config.py +++ b/lib/ramble/ramble/config.py @@ -40,7 +40,7 @@ import re import sys from contextlib import contextmanager -from typing import Any, Dict, List +from typing import Any, Dict, List, Tuple, cast from ruamel import yaml from ruamel.yaml.error import MarkedYAMLError @@ -490,7 +490,7 @@ def _process_dict_keyname_overrides(data): for sk, sv in data.items(): if sk.endswith(":"): key = syaml.syaml_str(sk[:-1]) - key.override = True + setattr(key, "override", True) # noqa: B010 else: key = sk @@ -706,7 +706,7 @@ def get_config(self, section, scope=None): """ if not hasattr(self, "_get_config_cache"): - self._get_config_cache = {} + self._get_config_cache: Dict[Tuple[Any, Any], Any] = {} key = (section, scope) if key not in self._get_config_cache: @@ -718,7 +718,7 @@ def _get_config_no_memo(self, section, scope): _validate_section_name(section) if scope is None: - scopes = self.scopes.values() + scopes = list(self.scopes.values()) else: scopes = [self._validate_scope(scope)] @@ -1158,6 +1158,7 @@ def _mark_internal(data, name): This is used by `ramble config blame` to show where config lines came from. """ + d: Any if isinstance(data, dict): d = syaml.syaml_dict( (_mark_internal(k, name), _mark_internal(v, name)) for k, v in data.items() @@ -1168,8 +1169,9 @@ def _mark_internal(data, name): d = syaml.syaml_type(data) if syaml.markable(d): - d._start_mark = yaml.Mark(name, None, None, None, None, None) - d._end_mark = yaml.Mark(name, None, None, None, None, None) + markable_d = cast(Any, d) + markable_d._start_mark = yaml.Mark(name, None, None, None, None, None) + markable_d._end_mark = yaml.Mark(name, None, None, None, None, None) return d @@ -1202,14 +1204,15 @@ def get_valid_type(path): try: validate(test_data, section_schemas[section]) except (ConfigFormatError, AttributeError) as e: - jsonschema_error = e.validation_error - if jsonschema_error.validator == "type": - return types[jsonschema_error.validator_value]() - elif jsonschema_error.validator in ("anyOf", "oneOf"): - for subschema in jsonschema_error.validator_value: - schema_type = subschema.get("type") - if schema_type is not None: - return types[schema_type]() + jsonschema_error = getattr(e, "validation_error", None) + if jsonschema_error: + if jsonschema_error.validator == "type": + return types[jsonschema_error.validator_value]() + elif jsonschema_error.validator in ("anyOf", "oneOf"): + for subschema in jsonschema_error.validator_value: + schema_type = subschema.get("type") + if schema_type is not None: + return types[schema_type]() else: return type(None) raise ConfigError(f"Cannot determine valid type for path '{path}'.") @@ -1302,7 +1305,7 @@ def process_config_path(path): ) path = path.lstrip(":") front = syaml.syaml_str(front) - front.override = True + setattr(front, "override", True) # noqa: B010 seen_override_in_path = True result.append(front) return result @@ -1398,7 +1401,7 @@ def use_configuration(*scopes_or_paths): import ramble.repository - saved_instances = {} + saved_instances: Dict[Any, Any] = {} for obj_type, singleton in ramble.repository.paths.items(): saved_instances[obj_type] = singleton._instance singleton._instance = None diff --git a/lib/ramble/ramble/expander.py b/lib/ramble/ramble/expander.py index c5c96accbe..8433606767 100644 --- a/lib/ramble/ramble/expander.py +++ b/lib/ramble/ramble/expander.py @@ -18,7 +18,7 @@ import sys from contextlib import contextmanager from enum import Enum -from typing import Dict, FrozenSet, List, Optional, Union +from typing import Any, Callable, Dict, FrozenSet, List, Optional, Union import ramble.config import ramble.error @@ -167,7 +167,7 @@ def _maybe(expander, var_name, default=""): return default -supported_math_operators = { +supported_math_operators: Dict[type, Callable[..., Any]] = { ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, @@ -194,7 +194,7 @@ def _maybe(expander, var_name, default=""): ast.RShift: operator.rshift, } -supported_scalar_function_pointers = { +supported_scalar_function_pointers: Dict[str, Callable[..., Any]] = { "str": str, "int": int, "float": float, @@ -218,12 +218,12 @@ def _maybe(expander, var_name, default=""): format_spec_regex = re.compile(r"(?P[^:]+(?:::[^:]+)*):(?P[^:]+)$") # Functions that need to be supplied with the expander -supported_scalar_function_with_self_arg_pointers = { +supported_scalar_function_with_self_arg_pointers: Dict[str, Callable[..., Any]] = { "maybe": _maybe, } -supported_list_function_pointers = { +supported_list_function_pointers: Dict[str, Callable[..., Any]] = { "range": range, } @@ -435,7 +435,7 @@ def __init__(self, in_str): self.root.root = self.root opened = [] - children = [] + children: List[List[ExpansionNode]] = [] escaped = False for i, c in enumerate(self.str): if c == ExpansionDelimiter.left and not escaped: diff --git a/lib/ramble/ramble/experiment_result.py b/lib/ramble/ramble/experiment_result.py index e26c04b477..4015ffb1f6 100644 --- a/lib/ramble/ramble/experiment_result.py +++ b/lib/ramble/ramble/experiment_result.py @@ -8,6 +8,7 @@ import os from enum import Enum +from typing import Union from ramble.namespace import namespace from ramble.software_info import SoftwareInfo @@ -59,7 +60,7 @@ def __init__(self, app_inst): """Build up the result from the given app instance""" self._app_inst = app_inst self.name = None - self.status = ExperimentStatus.UNKNOWN + self.status: Union[ExperimentStatus, str] = ExperimentStatus.UNKNOWN self.n_repeats = None self.experiment_chain = [] self.tags = [] diff --git a/lib/ramble/ramble/fetch_strategy.py b/lib/ramble/ramble/fetch_strategy.py index b3aa473f08..c54d40faf4 100644 --- a/lib/ramble/ramble/fetch_strategy.py +++ b/lib/ramble/ramble/fetch_strategy.py @@ -34,7 +34,7 @@ import shutil import sys import urllib.parse -from typing import List, Optional, Type +from typing import Any, List, Optional, Type from llnl.util import tty from llnl.util.filesystem import ( @@ -115,6 +115,7 @@ class FetchStrategy: # optional attributes in version() args. optional_attrs: List[str] = [] url: Optional[str] = None + stage: Any = None def __init__(self, **kwargs): # The stage is initialized late, so that fetch strategies can be @@ -382,14 +383,14 @@ def _check_headers(self, headers): @_needs_stage def _fetch_urllib(self, url): save_file = None - if self.stage.save_filename: + if self.stage and self.stage.save_filename: save_file = self.stage.save_filename logger.msg(f"Fetching {url}") # Check if we're about to try and open a broken symlink, and if so # remove that file to avoid a bad situation where a file "exists" but # cannot be opened (warning: this is not atomic) - if os.path.islink(save_file) and not os.path.exists(save_file): + if save_file and os.path.islink(save_file) and not os.path.exists(save_file): os.unlink(save_file) # Run urllib but grab the mime type from the http headers @@ -404,6 +405,9 @@ def _fetch_urllib(self, url): msg = f"urllib failed to fetch with error {e}" raise FailedDownloadError(url, msg) from None + if not save_file: + raise FailedDownloadError(url, "Cannot determine save filename for urllib fetch") + with open(save_file, "wb") as _open_file: shutil.copyfileobj(response, _open_file) @@ -645,6 +649,8 @@ class CacheURLFetchStrategy(URLFetchStrategy): @_needs_stage def fetch(self): + if not self.url: + raise FetchError(f"No URL specified for {self}") path = re.sub("^file://", "", self.url) # check whether the cache file exists. @@ -694,6 +700,8 @@ def __init__(self, **kwargs): super().__init__(**kwargs) # Set a URL based on the type of fetch strategy. + if not self.url_attr: + raise ValueError(f"{self.__class__} has no url_attr defined.") self.url = kwargs.get(self.url_attr) if not self.url: raise ValueError(f"{self.__class__} requires {self.url_attr} argument.") @@ -776,7 +784,7 @@ class GitFetchStrategy(VCSFetchStrategy): git_version_re = r"git version (\S+)" submodules: bool = False - submodules_delete: bool = False + submodules_delete: Any = False get_full_repo: bool = False def __init__(self, **kwargs): @@ -802,6 +810,8 @@ def version_from_git(git_exe): """ version_output = git_exe("--version", output=str) m = re.search(GitFetchStrategy.git_version_re, version_output) + if not m: + raise FetchError(f"Could not parse git version from {version_output}") return spack.version.Version(m.group(1)) @property @@ -872,7 +882,14 @@ def clone(self, dest=None, commit=None, branch=None, tag=None, bare=False): bare (bool): Execute a "bare" git clone (--bare option to git) """ # Default to spack source path - dest = dest or self.stage.source_path + if not self.url: + raise FetchError(f"Cannot clone git repository without URL in {self}") + + if not dest: + if not self.stage: + raise NoStageError(self.clone) + dest = self.stage.source_path + logger.debug(f"Cloning git repository: {self._repo_info()}") git = self.git @@ -958,7 +975,7 @@ def clone(self, dest=None, commit=None, branch=None, tag=None, bare=False): git(*pull_args, ignore_errors=1) git(*co_args) - if self.submodules_delete: + if self.submodules_delete and hasattr(self.submodules_delete, "__iter__"): with working_dir(self.stage.source_path): for submodule_to_delete in self.submodules_delete: args = ["rm", submodule_to_delete] @@ -993,6 +1010,8 @@ def protocol_supports_shallow_clone(self): """Shallow clone operations (--depth #) are not supported by the basic HTTP protocol or by no-protocol file specifications. Use (e.g.) https:// or file:// instead.""" + if not self.url: + return False return not (self.url.startswith("http://") or self.url.startswith("/")) def __str__(self): @@ -1018,6 +1037,7 @@ class CvsFetchStrategy(VCSFetchStrategy): url_attr = "cvs" optional_attrs = ["branch", "date"] + date: Optional[str] = None def __init__(self, **kwargs): # Discards the keywords in kwargs that may conflict with the next call @@ -1054,7 +1074,7 @@ def source_id(self): return id def mirror_id(self): - if not (self.branch or self.date): + if not (self.branch or self.date) or not self.url: # We need a branch or a date to make a checkout reproducible return None # Special-case handling because this is not actually a URL @@ -1076,6 +1096,9 @@ def fetch(self): logger.debug("Already fetched {self.stage.source_path}") return + if not self.url: + raise FetchError(f"No CVS URL specified for {self}") + logger.debug("Checking out CVS repository: {self.url}") with temp_cwd(): @@ -1159,7 +1182,7 @@ def source_id(self): return self.revision def mirror_id(self): - if self.revision: + if self.revision and self.url: repo_path = url_util.parse(self.url).path result = os.path.sep.join(["svn", repo_path, self.revision]) return result @@ -1170,6 +1193,9 @@ def fetch(self): logger.debug(f"Already fetched {self.stage.source_path}") return + if not self.url: + raise FetchError(f"No SVN URL specified for {self}") + logger.debug(f"Checking out subversion repository: {self.url}") args = ["checkout", "--force", "--quiet"] @@ -1269,7 +1295,7 @@ def source_id(self): return self.revision def mirror_id(self): - if self.revision: + if self.revision and self.url: repo_path = url_util.parse(self.url).path result = os.path.sep.join(["hg", repo_path, self.revision]) return result @@ -1280,6 +1306,9 @@ def fetch(self): logger.debug(f"Already fetched {self.stage.source_path}") return + if not self.url: + raise FetchError(f"No Mercurial URL specified for {self}") + args = [] if self.revision: args.append(f"at revision {self.revision}") @@ -1466,7 +1495,8 @@ def from_url_scheme(url, *args, **kwargs): for fetcher in all_strategies: url_attr = getattr(fetcher, "url_attr", None) if url_attr and url_attr == scheme: - return fetcher(url, *args, **kwargs) + fetcher_cls: Any = fetcher + return fetcher_cls(url, *args, **kwargs) raise ValueError(f'No FetchStrategy found for url with scheme: "{parsed_url.scheme}"') diff --git a/lib/ramble/ramble/graphs.py b/lib/ramble/ramble/graphs.py index d449e66b8a..e706cd4ef1 100644 --- a/lib/ramble/ramble/graphs.py +++ b/lib/ramble/ramble/graphs.py @@ -136,7 +136,7 @@ def walk(self): ) from e self._prepared = True - yield from self._sorted + yield from (self._sorted or []) def get_node(self, key): """Given a key, return the node containing this key diff --git a/lib/ramble/ramble/keywords.py b/lib/ramble/ramble/keywords.py index 2b86437ced..1549f3ca19 100644 --- a/lib/ramble/ramble/keywords.py +++ b/lib/ramble/ramble/keywords.py @@ -144,6 +144,8 @@ class Keywords: err_file: str env_path: str input_name: str + is_repeat_parent: str + is_repeat_child: str repeat_index: str spec_name: str env_name: str @@ -151,6 +153,8 @@ class Keywords: n_nodes: str processes_per_node: str n_threads: str + n_accelerators: str + accelerators_per_node: str batch_submit: str mpi_command: str workload_template_name: str diff --git a/lib/ramble/ramble/language/application_language.py b/lib/ramble/ramble/language/application_language.py index e7ea519495..24a1e6c139 100644 --- a/lib/ramble/ramble/language/application_language.py +++ b/lib/ramble/ramble/language/application_language.py @@ -6,6 +6,8 @@ # option. This file may not be copied, modified, or distributed # except according to those terms. +from typing import List + import ramble.definitions.variables import ramble.language.language_helpers import ramble.language.shared_language @@ -581,7 +583,7 @@ def _execute_stage_files(app): stage_method = cfg.get("config", {}).get("stage_method", "cp") stage_cmd = method_map[stage_method] - template = [] + template: List[str] = [] if src is not None: if dst is not None: diff --git a/lib/ramble/ramble/language/language_base.py b/lib/ramble/ramble/language/language_base.py index e37e228df6..33d18b986c 100644 --- a/lib/ramble/ramble/language/language_base.py +++ b/lib/ramble/ramble/language/language_base.py @@ -151,12 +151,12 @@ def __init__(cls, name, bases, attr_dict): # with the directives # We use type(cls) to get the metaclass, and iterate its MRO to # collect all init values and directive attributes. - all_init_values = {} - all_directive_names = set() - all_directive_functions = {} - all_directive_classes = {} + all_init_values: Dict[str, Any] = {} + all_directive_names: Set[str] = set() + all_directive_functions: Dict[str, Any] = {} + all_directive_classes: Dict[str, Any] = {} - for base_meta in reversed(type(cls).__mro__): + for base_meta in reversed(inspect.getmro(type(cls))): if hasattr(base_meta, "_directive_init_values"): all_init_values.update(base_meta._directive_init_values) if hasattr(base_meta, "_directive_names"): @@ -169,16 +169,19 @@ def __init__(cls, name, bases, attr_dict): for d, t in all_init_values.items(): setattr(cls, d, copy.deepcopy(t)) + if hasattr(DirectiveMeta, "_directive_functions"): + all_directive_functions.update(DirectiveMeta._directive_functions) + if hasattr(DirectiveMeta, "_directive_classes"): + all_directive_classes.update(DirectiveMeta._directive_classes) + if hasattr(DirectiveMeta, "_directive_names"): + all_directive_names |= DirectiveMeta._directive_names + directive_attrs = { "_directive_functions": all_directive_functions, "_directive_classes": all_directive_classes, - "_directive_names": all_directive_names | DirectiveMeta._directive_names.copy(), + "_directive_names": all_directive_names, } - for attr, val in directive_attrs.items(): - if hasattr(DirectiveMeta, attr): - val.update(getattr(DirectiveMeta, attr)) - for attr, val in directive_attrs.items(): setattr(cls, attr, val) diff --git a/lib/ramble/ramble/language/language_helpers.py b/lib/ramble/ramble/language/language_helpers.py index 305a7c789d..72fb989505 100644 --- a/lib/ramble/ramble/language/language_helpers.py +++ b/lib/ramble/ramble/language/language_helpers.py @@ -9,7 +9,7 @@ import fnmatch import functools from collections import OrderedDict -from typing import Any, List, Optional, Union +from typing import Any, Dict, List, Optional, Union from packaging.specifiers import SpecifierSet from packaging.version import InvalidVersion, Version @@ -418,8 +418,8 @@ def is_specifier_set_compatible(spec_set): def _parse_when(w_set): from ramble.util.format import when_order - variants = {} - versions = {} + variants: Dict[str, str] = {} + versions: Dict[str, Any] = {} for w_entry in sorted(w_set, key=when_order): for w in w_entry.split(): if "=" in w: diff --git a/lib/ramble/ramble/language/shared_language.py b/lib/ramble/ramble/language/shared_language.py index 673c02339e..9fb8755f36 100644 --- a/lib/ramble/ramble/language/shared_language.py +++ b/lib/ramble/ramble/language/shared_language.py @@ -8,7 +8,7 @@ import collections import contextlib -from typing import Any, Callable, List, Optional, Union +from typing import Any, Callable, Dict, List, Optional, Union import ramble.language.language_base import ramble.language.language_helpers @@ -1598,7 +1598,7 @@ def modifier( def _execute_modifier(obj): when_list = ramble.language.language_helpers.build_when_list(when, obj, name, "modifier") - mod_dict = {"name": name} + mod_dict: Dict[str, Any] = {"name": name} if mode is not None: mod_dict["mode"] = mode if on_executable is not None: diff --git a/lib/ramble/ramble/main.py b/lib/ramble/ramble/main.py index 2a6df11946..05c5e5452f 100644 --- a/lib/ramble/ramble/main.py +++ b/lib/ramble/ramble/main.py @@ -23,6 +23,7 @@ import sys import traceback import warnings +from typing import Any, Dict, List, Optional, cast import jsonschema import ruamel @@ -108,7 +109,7 @@ def add_all_commands(parser): def index_commands(): """create an index of commands by section for this help level""" - index = {} + index: Dict[str, Dict[str, List[str]]] = {} for command in ramble.cmd.all_commands(): cmd_module = ramble.cmd.get_module(command) @@ -169,7 +170,13 @@ def format_help_sections(self, level): # Create a list of subcommand actions. Argparse internals are nasty! # Note: you can only call _get_subactions() once. Even nastier! if not hasattr(self, "actions"): - self.actions = self._subparsers._actions[-1]._get_subactions() + subparsers = getattr(self, "_subparsers", None) + actions = getattr(subparsers, "_actions", []) if subparsers else [] + self.actions = ( + actions[-1]._get_subactions() + if actions and hasattr(actions[-1], "_get_subactions") + else [] + ) # make a set of commands not yet added. remaining = set(ramble.cmd.all_commands()) @@ -227,7 +234,7 @@ def add_subcommand_group(title, commands): group_description = section_descriptions.get(section, section) to_display = sections[section] - commands = [] + commands: List[str] = [] # add commands whose order we care about first. if section in section_order: @@ -274,7 +281,7 @@ def add_parser(name, **kwargs): kwargs.setdefault("formatter_class", RambleHelpFormatter) return old_add_parser(name, **kwargs) - sp.add_parser = add_parser + cast(Any, sp).add_parser = add_parser return sp def add_command(self, cmd_name): @@ -568,7 +575,7 @@ def mock_repositories(objects): for obj in objects: obj_section = ramble.repository.type_definitions[obj]["config_section"] key = syaml.syaml_str(obj_section) - key.override = True + cast(Any, key).override = True ramble.config.config.scopes["command_line"].sections[obj_section] = syaml.syaml_dict( [(key, [ramble.paths.mock_builtin_path])] @@ -750,7 +757,7 @@ def __call__(self, *argv, **kwargs): if fail_on_error and self.returncode not in (None, 0): raise RambleCommandError( - "Command exited with code %d: %s(%s).\nCommand output:\n\n%s" + "Command exited with code %s: %s(%s).\nCommand output:\n\n%s" % ( self.returncode, self.command_name, @@ -971,7 +978,7 @@ def _main(argv=None): setup_main_options(args) # activate a workspace if one was specified on the command line - workspace_format_error = None + workspace_format_error: Optional[Exception] = None if not args.no_workspace: try: ws = ramble.cmd.find_workspace(args) diff --git a/lib/ramble/ramble/mirror.py b/lib/ramble/ramble/mirror.py index bd55d830e5..ab8565275f 100644 --- a/lib/ramble/ramble/mirror.py +++ b/lib/ramble/ramble/mirror.py @@ -362,7 +362,7 @@ class MirrorStats: def __init__(self): self.present = {} self.new = {} - self.errors = {} + self.errors = set() self.current_spec = None self.added_resources = set() diff --git a/lib/ramble/ramble/paths.py b/lib/ramble/ramble/paths.py index 507c75318f..6c06751f96 100644 --- a/lib/ramble/ramble/paths.py +++ b/lib/ramble/ramble/paths.py @@ -47,3 +47,6 @@ etc_path: str = os.path.join(prefix, "etc") system_etc_path: str = "/etc" + +#: Default cache location for downloaded archives +default_fetch_cache_path: str = os.path.join(var_path, "cache") diff --git a/lib/ramble/ramble/pipeline.py b/lib/ramble/ramble/pipeline.py index 7fe0186e32..6c9f722371 100644 --- a/lib/ramble/ramble/pipeline.py +++ b/lib/ramble/ramble/pipeline.py @@ -13,14 +13,17 @@ import shutil import stat from enum import Enum +from typing import Dict, List import llnl.util.filesystem as fs from llnl.util import tty +from llnl.util.tty.colify import colify import ramble.config import ramble.expander import ramble.experiment_result import ramble.fetch_strategy +import ramble.filters import ramble.software_environments import ramble.stage import ramble.uploader @@ -458,7 +461,7 @@ def _complete(self): ) archive_url = archive_url.rstrip("/") if archive_url else None - if self.create_tar: + if self.create_tar and self.archive_name: tar_extension = ".tar.gz" tar = which("tar", required=True) tar_path = self.archive_name + tar_extension @@ -531,8 +534,8 @@ def _complete(self): if self.workspace.input_mirror_stats.errors: logger.error("Failed downloads:") - tty.colify( - (s.cformat("{name}") for s in list(self.workspace.input_mirror_stats.errors)), + colify( + [s.cformat("{name}") for s in list(self.workspace.input_mirror_stats.errors)], output=logger.active_stream(), ) logger.die("Mirroring has errors.") @@ -838,7 +841,7 @@ def _complete(self): self._copy_workspace_root_files(self.workspace, self.workspace.named_deployment) # Create an index.json of the deployment - deployment_index = {self.index_namespace: []} + deployment_index: Dict[str, List[str]] = {self.index_namespace: []} for file in self._deployment_files(): deployment_index[self.index_namespace].append( file.replace(self.workspace.named_deployment + os.path.sep, "") diff --git a/lib/ramble/ramble/renderer.py b/lib/ramble/ramble/renderer.py index 9f49c06383..c446fed05a 100644 --- a/lib/ramble/ramble/renderer.py +++ b/lib/ramble/ramble/renderer.py @@ -7,6 +7,7 @@ # except according to those terms. import itertools +from typing import Any, Dict, Set import ramble.expander import ramble.repeats @@ -154,7 +155,7 @@ def _filter_used_variables(self, matrices, zips, used_variables): del zips[zip_name] def _process_zips(self, render_group, object_variables, zips, zipped_vars): - defined_zips = {} + defined_zips: Dict[str, Dict[str, Any]] = {} for zip_group, group_def in zips.items(): defined_zips[zip_group] = {"vars": {}, "length": 0} cur_zip = defined_zips[zip_group] @@ -302,7 +303,7 @@ def _process_matrices( Matrices consume vector variables. """ last_size = -1 - matrix_vars = set() + matrix_vars: Set[str] = set() matrix_vectors = [] matrix_variables = [] for matrix in matrices: @@ -489,10 +490,10 @@ def render_objects(self, render_group, exclude_where=None, ignore_used=True, fat self._filter_used_variables(matrices, zips, used_variables) # Extract Zips - defined_zips = {} - consumed_zips = set() + defined_zips: Dict[str, Dict[str, Any]] = {} + consumed_zips: Set[str] = set() if zips: - zipped_vars = set() + zipped_vars: Set[str] = set() defined_zips = self._process_zips(render_group, object_variables, zips, zipped_vars) # Process Matrices diff --git a/lib/ramble/ramble/reports.py b/lib/ramble/ramble/reports.py index d25cd56289..6401a1a49d 100644 --- a/lib/ramble/ramble/reports.py +++ b/lib/ramble/ramble/reports.py @@ -10,7 +10,7 @@ import os import re from enum import Enum -from typing import Dict, List +from typing import Any, Dict, List import llnl.util.filesystem as fs @@ -111,7 +111,7 @@ def simplify_names(names): break # Find longest common suffix of parts - common_suffix = [] + common_suffix: List[str] = [] remaining_min_len = min(len(parts) - len(common_prefix) for parts in split_names) for i in range(1, remaining_min_len + 1): part = split_names[0][-i] @@ -578,6 +578,12 @@ def create_plot_generator(self, args, report_dir_path, exp_results): class PlotGenerator: + perf_unit: str = "" + scale_unit: str = "" + + def prep_draw(self, perf_measure, scale_var): + raise NotImplementedError("Subclasses must implement prep_draw") + def __init__( self, spec, @@ -594,7 +600,7 @@ def __init__( self.normalize = normalize self.spec = spec self.report_dir_path = report_dir_path - self.inventory = {"files": []} + self.inventory: Dict[str, List[Any]] = {"files": []} self.figsize = [12, 8] self.exp_results = exp_results @@ -677,7 +683,9 @@ def write_inventory(self): with open(self.get_inventory_path(), "w+", encoding="utf-8") as f: syaml.dump(self.inventory, stream=f) - def draw(self, perf_measure, scale_var, series, pdf_report, y_label=None): + def draw(self, perf_measure, scale_var, series, *args, **kwargs): + pdf_report = args[0] if len(args) > 0 else kwargs.get("pdf_report") + y_label = args[1] if len(args) > 1 else kwargs.get("y_label", None) series_data = self.output_df.query(f'series == "{series}"').copy() title = ( @@ -686,7 +694,6 @@ def draw(self, perf_measure, scale_var, series, pdf_report, y_label=None): ) logger.debug(f"Generating plot for {title}") - # TODO: prep_draw method in subclass ScalingPlotGenerator, not this class fig, ax = self.prep_draw(perf_measure, scale_var) if self.normalize: @@ -1019,10 +1026,11 @@ def prep_draw(self, perf_measure, scale_var): class WeakScalingPlot(ScalingPlotGenerator): plot_type = "weak_scaling" - def draw(self, perf_measure, scale_var, series, pdf_report): - y_label = perf_measure + def draw(self, perf_measure, scale_var, series, pdf_report, y_label=None, *args, **kwargs): + if y_label is None: + y_label = perf_measure - super().draw(perf_measure, scale_var, series, pdf_report, y_label) + super().draw(perf_measure, scale_var, series, pdf_report, y_label, *args, **kwargs) def add_idealized_data(self, raw_results, selected_data): selected_data = super().add_idealized_data(raw_results, selected_data) @@ -1058,10 +1066,11 @@ def normalize_data( ): super().normalize_data(data, scale_to_index, to_col=to_col, from_col=from_col) - def draw(self, perf_measure, scale_var, series, pdf_report): - y_label = perf_measure + def draw(self, perf_measure, scale_var, series, pdf_report, y_label=None, *args, **kwargs): + if y_label is None: + y_label = perf_measure - super().draw(perf_measure, scale_var, series, pdf_report, y_label) + super().draw(perf_measure, scale_var, series, pdf_report, y_label, *args, **kwargs) class FomPlot(PlotGenerator): @@ -1117,7 +1126,7 @@ def generate_plot_data(self, pdf_report): self.draw(perf_measure, scale_var, series, unit, pdf_report) # TODO: dry bar plot drawing - def draw(self, perf_measure, scale_var, series, unit, pdf_report): + def draw(self, perf_measure, scale_var, series, unit, pdf_report, *args, **kwargs): pd = import_pandas() self.output_df[ReportVars.FOM_VALUE.value] = to_numeric_if_possible( @@ -1160,7 +1169,7 @@ def draw(self, perf_measure, scale_var, series, unit, pdf_report): class ComparisonPlot(PlotGenerator): plot_type = "comparison" - def draw(self, perf_measure, scale_var, series, pdf_report): + def draw(self, perf_measure, scale_var, series, pdf_report, y_label=None, *args, **kwargs): ax = self.output_df.plot(kind="bar", figsize=self.figsize) fig = ax.get_figure() @@ -1266,7 +1275,6 @@ def draw_multiline(self, perf_measure, scale_var, pdf_report, y_label): title = f"{perf_measure} vs {scale_var}" logger.debug(f"Generating plot for {title}") - # TODO: prep_draw method in subclass ScalingPlotGenerator, not this class fig, ax = self.prep_draw(perf_measure, scale_var) for series in self.output_df.loc[:, ReportVars.SERIES.value].unique(): diff --git a/lib/ramble/ramble/repository.py b/lib/ramble/ramble/repository.py index f2da562571..4c42f8d48d 100644 --- a/lib/ramble/ramble/repository.py +++ b/lib/ramble/ramble/repository.py @@ -23,7 +23,7 @@ import traceback import types from enum import Enum -from typing import Mapping +from typing import Any, Dict, Mapping, cast from ruamel import yaml @@ -78,7 +78,7 @@ unified_config = "repo.yaml" -type_definitions = { +type_definitions: Dict[ObjectTypes, Dict[str, Any]] = { ObjectTypes.applications: { "file_name": "application.py", "dir_name": "applications", @@ -278,7 +278,7 @@ def _gen_path(repo_dirs=None, obj_type=default_type): ) path = RepoPath(*repo_dirs, object_type=obj_type) - sys.meta_path.append(path) + sys.meta_path.append(cast(Any, path)) return path @@ -323,9 +323,10 @@ def list_object_files(obj_inst, object_type): base_chain = obj_inst.__class__.__mro__[1:] for cls in base_chain: - path = importlib.util.find_spec(cls.__module__).origin + spec = importlib.util.find_spec(cls.__module__) + path = spec.origin if spec else None - if not repo_path.in_path(path) and not base_repo_path.in_path(path): + if not path or (not repo_path.in_path(path) and not base_repo_path.in_path(path)): # Stop upon hitting a non-repo file break @@ -408,7 +409,7 @@ def use_repositories(*paths_and_repos, object_type=default_type): finally: # Restore _path and sys.meta_path if remove_from_meta and temporary_repositories in sys.meta_path: - sys.meta_path.remove(temporary_repositories) + sys.meta_path.remove(cast(Any, temporary_repositories)) paths[object_type] = saved @@ -479,7 +480,7 @@ def _create_new_cache(objects_path, object_file_name, object_type): """ # Create a dictionary that will store the mapping between a # object name and its stat info - cache = {} + cache: Dict[str, os.stat_result] = {} if not os.path.isdir(objects_path): return cache for obj_name in os.listdir(objects_path): @@ -1015,15 +1016,22 @@ def check(condition, msg): raise BadRepoError(msg) # Validate repository layout. - self.config_name = None - self.config_file = None + config_name = None + config_file = None for config in type_definitions[object_type]["accepted_configs"]: - config_file = os.path.join(self.root, config) - if os.path.exists(config_file): - self.config_name = config - self.config_file = config_file - check(self.config_file, "No valid config file found") - check(os.path.isfile(self.config_file), f"No {self.config_name} found in '{root}'") + candidate = os.path.join(self.root, config) + if os.path.exists(candidate): + config_name = config + config_file = candidate + break + + if not config_file or not config_name: + raise BadRepoError("No valid config file found") + if not os.path.isfile(config_file): + raise BadRepoError(f"No {config_name} found in '{root}'") + + self.config_name: str = config_name + self.config_file: str = config_file # Read configuration and validate namespace config = self._read_config() @@ -1075,6 +1083,7 @@ def _create_namespace(self): """ parent = None + module: Any = None for i in range(1, len(self._names) + 1): ns = ".".join(self._names[:i]) @@ -1220,8 +1229,9 @@ def get(self, spec): # handler by wrapping them if ramble.config.get("config:debug"): sys.excepthook(*sys.exc_info()) + exc_type, exc_obj, exc_tb = sys.exc_info() raise FailedConstructorError( - spec.fullname, *sys.exc_info(), object_type=self.object_type + spec.fullname, exc_type, exc_obj, exc_tb, object_type=self.object_type ) from e @autospec diff --git a/lib/ramble/ramble/results_table.py b/lib/ramble/ramble/results_table.py index 1be0877a49..26b169dc83 100644 --- a/lib/ramble/ramble/results_table.py +++ b/lib/ramble/ramble/results_table.py @@ -8,6 +8,7 @@ import copy import os +from typing import Any, Dict, List, Optional from ramble.util.file_util import create_symlink from ramble.util.logger import logger @@ -27,6 +28,13 @@ class ResultsColumn: "figure_of_merit_origin_type", ] + name: Optional[str] = None + expression: Optional[str] = None + figure_of_merit: Optional[str] = None + figure_of_merit_context: Optional[str] = None + figure_of_merit_origin_type: Optional[str] = None + _template: Optional["ResultsAutoColumn"] = None + def __init__(self, conf_dict): """Construct a column from a configuration dict, assuming the structure matches the column schema in lib/ramble/ramble/schema/tables.py @@ -152,6 +160,11 @@ class ResultsAutoColumn: "figure_of_merit_origin_type", ] + name: Optional[str] = None + context_name: Optional[str] = None + figure_of_merit: Optional[str] = None + figure_of_merit_origin_type: Optional[str] = None + def __init__(self, conf_dict): """Construct an auto column from a configuration dict @@ -189,6 +202,9 @@ class ResultsTable: _where_name = "where" _transpose_name = "transpose" + group_by: List[str] + sort_by: List[str] + def __init__(self, conf_dict): """Constructor for a single table @@ -359,7 +375,7 @@ def extract_row(self, app_inst): col_obj._template = autocol_template self.generated_columns[col_name] = col_obj - column_values = {} + column_values: Dict[str, Any] = {} remaining_columns = set(self._data.keys()) # Combine manual and generated columns diff --git a/lib/ramble/ramble/software_environments.py b/lib/ramble/ramble/software_environments.py index 0106039bab..91798ca25c 100644 --- a/lib/ramble/ramble/software_environments.py +++ b/lib/ramble/ramble/software_environments.py @@ -8,7 +8,7 @@ import copy from collections import defaultdict -from typing import DefaultDict, Dict, List, Set +from typing import Any, DefaultDict, Dict, List, Optional, Set import ramble.config import ramble.error @@ -42,6 +42,10 @@ def _is_dict_empty(rendered: defaultdict): class SoftwarePackage: """Class to represent a single software package""" + spec: Optional[str] = None + compiler: Optional[str] = None + compiler_spec: Optional[str] = None + def __init__( self, name: str, @@ -720,8 +724,8 @@ def __init__(self, workspace): self._environment_templates = {} self._external_env_templates = {} self._package_templates = {} - self._rendered_packages = defaultdict(dict) - self._rendered_environments = defaultdict(dict) + self._rendered_packages: DefaultDict[Any, Dict[Any, Any]] = defaultdict(dict) + self._rendered_environments: DefaultDict[Any, Dict[Any, Any]] = defaultdict(dict) self._define_templates() @@ -808,15 +812,15 @@ def _define_templates(self): if env_info.get(namespace.external_env): # External environments are stored in a separate template dict, such that it # still goes through the rendering to concretize on the package_manager used. - new_env = ExternalEnvironment(env_template, env_info[namespace.external_env]) - self._external_env_templates[env_template] = new_env + ext_env = ExternalEnvironment(env_template, env_info[namespace.external_env]) + self._external_env_templates[env_template] = ext_env else: # Define a new template environment - new_env = TemplateEnvironment(env_template) + tmpl_env = TemplateEnvironment(env_template) if namespace.packages in env_info: for package in env_info[namespace.packages]: - new_env.add_package_name(package) - self._environment_templates[env_template] = new_env + tmpl_env.add_package_name(package) + self._environment_templates[env_template] = tmpl_env def define_compiler_packages(self, environment: RenderedEnvironment, expander: Expander): """Define packages for compilers in this environment @@ -987,12 +991,12 @@ def _check_environment(self, environment): environment (SoftwareEnvironment): Environment to check for issues in """ - pkg_names = set() + pkg_names: Set[str] = set() for pkg in environment._packages: pkg_names.add(pkg.name) - used_compilers = set() + used_compilers: Set[str] = set() compiler_warnings = [ (pkg.name, pkg.compiler) for pkg in environment._packages diff --git a/lib/ramble/ramble/stage.py b/lib/ramble/ramble/stage.py index 3ddd95c803..17a4e1575e 100644 --- a/lib/ramble/ramble/stage.py +++ b/lib/ramble/ramble/stage.py @@ -12,7 +12,7 @@ import os import stat import sys -from typing import Dict +from typing import Dict, List from llnl.util.filesystem import ( can_access, @@ -210,19 +210,21 @@ def set_subdir(self, subdir_name): @property def expected_archive_files(self): """Possible archive file paths.""" - paths = [] + paths: List[str] = [] - fnames = [] + fnames: List[str] = [] expanded = True if isinstance(self.default_fetcher, fs.URLFetchStrategy): expanded = self.default_fetcher.expand_archive - fnames.append(os.path.basename(self.default_fetcher.url)) + if self.default_fetcher.url: + fnames.append(os.path.basename(self.default_fetcher.url)) if self.mirror_paths: fnames.extend(os.path.basename(x) for x in self.mirror_paths) - paths.extend(os.path.join(self.path, f) for f in fnames) - if not expanded: + if self.path: + paths.extend(os.path.join(self.path, f) for f in fnames) + if not expanded and self.source_path: # If the download file is not compressed, the "archive" is a # single file placed in Stage.source_path paths.extend(os.path.join(self.source_path, f) for f in fnames) @@ -277,7 +279,7 @@ def fetch(self, mirror_only=False, err_msg=None): # Join URLs of mirror roots with mirror paths. Because # urljoin() will strip everything past the final '/' in # the root, so we add a '/' if it is not present. - mirror_urls = [] + mirror_urls: List[str] = [] for mirror in ramble.mirror.MirrorCollection().values(): mirror_urls.extend( url_util.join(mirror.fetch_url, rel_path) for rel_path in self.mirror_paths diff --git a/lib/ramble/ramble/success_criteria.py b/lib/ramble/ramble/success_criteria.py index 93772268da..2d389d1701 100644 --- a/lib/ramble/ramble/success_criteria.py +++ b/lib/ramble/ramble/success_criteria.py @@ -8,6 +8,7 @@ import fnmatch import re +from typing import Dict, List from ramble.util.foms import get_literal_from_regex from ramble.util.logger import logger @@ -38,7 +39,7 @@ class ScopedCriteriaList: } def __init__(self): - self.criteria = {} + self.criteria: Dict[str, List[SuccessCriteria]] = {} for scope in self._valid_scopes: self.criteria[scope] = [] @@ -60,9 +61,8 @@ def add_criteria(self, scope, name, mode, *args, owning_object=None, **kwargs): if exists: logger.die(f"Success criteria {name} is not unique.") - self.criteria[scope].append( - SuccessCriteria(name, mode, *args, owning_object=owning_object, **kwargs) - ) + kwargs.setdefault("owning_object", owning_object) + self.criteria[scope].append(SuccessCriteria(name, mode, *args, **kwargs)) def flush_scope(self, scope): """Remove criteria within a scope, and lower level scopes diff --git a/lib/ramble/ramble/test/cache_fetch.py b/lib/ramble/ramble/test/cache_fetch.py index 86cef1bee4..67c07bff16 100644 --- a/lib/ramble/ramble/test/cache_fetch.py +++ b/lib/ramble/ramble/test/cache_fetch.py @@ -13,7 +13,10 @@ from llnl.util.filesystem import mkdirp, touch +import ramble.caches import ramble.config +import ramble.paths +import ramble.util.path from ramble.fetch_strategy import CacheURLFetchStrategy, NoCacheError from ramble.stage import InputStage @@ -50,3 +53,12 @@ def test_fetch(tmpdir, _fetch_method): source_path = stage.source_path mkdirp(source_path) fetcher.fetch() + + +def test_fetch_cache_default_path(mutable_config): + """Ensure default_fetch_cache_path exists and is used when config is unset.""" + assert hasattr(ramble.paths, "default_fetch_cache_path") + assert ramble.paths.default_fetch_cache_path == os.path.join(ramble.paths.var_path, "cache") + with ramble.config.override("config:input_cache", None): + path = ramble.caches.fetch_cache_location() + assert path == ramble.util.path.canonicalize_path(ramble.paths.default_fetch_cache_path) diff --git a/lib/ramble/ramble/test/mirror.py b/lib/ramble/ramble/test/mirror.py index b1e470b34a..7fce12bc13 100644 --- a/lib/ramble/ramble/test/mirror.py +++ b/lib/ramble/ramble/test/mirror.py @@ -170,3 +170,15 @@ def test_mirror_create(tmpdir, mutable_mock_workspace_path, app_name, tmpdir_fac mirror_pipeline.run() check_mirror(str(mirror_dir), app_name, app_class) + + +def test_mirror_stats_error(): + """Ensure MirrorStats records errors into a set without raising AttributeError.""" + stats = ramble.mirror.MirrorStats() + stats.current_spec = "test_spec" + stats.error("some_resource") + present, new, errors = stats.stats() + assert "test_spec" in errors + assert len(errors) == 1 + assert len(present) == 0 + assert len(new) == 0 diff --git a/lib/ramble/ramble/uploader.py b/lib/ramble/ramble/uploader.py index 095b3ca409..991deff363 100644 --- a/lib/ramble/ramble/uploader.py +++ b/lib/ramble/ramble/uploader.py @@ -11,6 +11,7 @@ import os import sys from enum import Enum +from typing import Any, Dict, List import jsonschema @@ -50,7 +51,7 @@ def validate_data(data, schema): class Uploader: - schema = [ + schema: List[Dict[str, Any]] = [ { "table": "experiments", "schema": experiment_schema, @@ -270,6 +271,7 @@ def upload_results(results): return logger.all_msg(f"Uploading results to {uri} with {uploader_type} uploader") + uploader: Uploader if uploader_type == uploader_types.BigQuery: uploader = BigQueryUploader() elif uploader_type == uploader_types.SQLite: @@ -769,7 +771,7 @@ def chunked_upload(self, table_id, data, uri=None): keys = list(data[0].keys()) for row in data: - sqlite_row = [] + sqlite_row: List[Any] = [] for key in keys: val = row.get(key) if isinstance(val, (dict, list)): diff --git a/lib/ramble/ramble/variants.py b/lib/ramble/ramble/variants.py index 94ddc85b23..08c2ecd986 100644 --- a/lib/ramble/ramble/variants.py +++ b/lib/ramble/ramble/variants.py @@ -379,10 +379,10 @@ def _expanded_set(self, expander: Optional[Expander] = None, for_output: bool = ) if expander is None or not has_templates: - return cache + return cache or set() expanded_set = set() - for variant in cache: + for variant in cache or set(): if "{" in variant: expanded_set.add(expander.expand_var(variant)) else: diff --git a/lib/ramble/ramble/workload.py b/lib/ramble/ramble/workload.py index e10aadcce1..d030c10238 100644 --- a/lib/ramble/ramble/workload.py +++ b/lib/ramble/ramble/workload.py @@ -221,7 +221,7 @@ def find_variable(self, name): ramble.definitions.variables.Variable | None: Variable instance if it exists, ``None`` if it is not found """ - named_vars = [] + named_vars: List[Variable] = [] for var_list in self.variables.values(): named_vars.extend(var for var in var_list if var.name == name) return named_vars diff --git a/lib/ramble/ramble/workspace/workspace.py b/lib/ramble/ramble/workspace/workspace.py index 3f1247baeb..2b79982e1e 100644 --- a/lib/ramble/ramble/workspace/workspace.py +++ b/lib/ramble/ramble/workspace/workspace.py @@ -14,7 +14,7 @@ import re import shutil from collections import defaultdict -from typing import Optional, Set +from typing import Any, Dict, List, Optional, Set from ruamel import yaml @@ -25,6 +25,7 @@ import ramble.config import ramble.context import ramble.error +import ramble.experiment_result import ramble.experiment_set import ramble.keywords import ramble.repository @@ -124,7 +125,7 @@ config_schema = ramble.schema.workspace.schema #: Currently activated workspace -_active_workspace = None +_active_workspace: Optional["Workspace"] = None def valid_workspace_name(name): @@ -318,7 +319,7 @@ def template_path(ws_path, requested_template_name): def all_template_paths(path): """Returns (abs) path to available template files in the workspace""" - templates = [] + templates: List[str] = [] config_path = os.path.join(path, WORKSPACE_CONFIG_PATH) for root, _, files in os.walk(config_path): @@ -444,6 +445,7 @@ class Workspace: inventory_file_name = "ramble_inventory.json" hash_file_name = "workspace_hash.sha256" + _latest_archive: Optional[str] = None def __init__(self, root, dry_run=False, read_default_template=True): logger.debug(f"In workspace init. Root = {root}") @@ -465,7 +467,7 @@ def __init__(self, root, dry_run=False, read_default_template=True): self.software_mirror_cache = None self.software_environments = None self.metadata = syaml.syaml_dict() - self.hash_inventory = {namespace.experiment: [], "versions": []} + self.hash_inventory: Dict[str, List[Any]] = {namespace.experiment: [], "versions": []} version = ramble.util.version.get_version() self.hash_inventory["versions"].append( { @@ -487,7 +489,7 @@ def __init__(self, root, dry_run=False, read_default_template=True): # A per-package_manager dict mapping package spec to its install prefix. # This can be re-used by all experiments of the workspace. - self.pkg_path_cache = defaultdict(dict) + self.pkg_path_cache: Dict[str, Dict[str, Any]] = defaultdict(dict) # A simple dict mapping a file's src_path to its content. # This is currently used as a cache for reading per-object template contents. @@ -500,7 +502,7 @@ def __init__(self, root, dry_run=False, read_default_template=True): # A cache structured as {pkg_man: {env_name: pkg_list}}. # It's used to cache package provenance info from different package managers. - self.pkg_prov_cache = defaultdict(dict) + self.pkg_prov_cache: Dict[str, Dict[str, Any]] = defaultdict(dict) # Key for each application config should be it's filepath # Format for an application config should be: @@ -723,8 +725,8 @@ def _check_deprecated(self, config): in the workspace config. """ - error_sections = [] - deprecated_sections = [] + error_sections: List[str] = [] + deprecated_sections: List[str] = [] if deprecated_sections: logger.warn("Your workspace configuration contains deprecated sections:") @@ -855,7 +857,7 @@ def update_metadata(self, key, value): def clear(self): self.config_sections = {} - self.application_configs = [] + self.application_configs = {} self._previous_active = None # previously active environment self.specs = [] @@ -1356,7 +1358,7 @@ def process_definitions(definitions, def_type="variable"): self.dry_run = True for workload_name in workload_names: edited = True - missing_vars = set() + missing_vars: Set[str] = set() exp_set = ramble.experiment_set.ExperimentSet(self) exp_list = exp_set.render_experiment_set( app_inst.name, @@ -1765,7 +1767,7 @@ def dump_results( fs.mkdirp(self.results_dir) results_written = [] - symlinks_updated = [] + symlinks_updated: List[str] = [] dt = self.date_string() inner_delim = "." @@ -1794,7 +1796,7 @@ def dump_results( for context in exp["CONTEXTS"]: f.write(f' {context["display_name"]} figures of merit:\n') - fom_summary = {} + fom_summary: Dict[str, List[Any]] = {} for fom in context["foms"]: name = fom["name"] if name not in fom_summary: @@ -1955,15 +1957,15 @@ def _remove_scoped_variables(scope_name: str, used_variables: Set): self.software_environments = ramble.software_environments.SoftwareEnvironments(self) experiment_set = self.build_experiment_set() - workspace_used_variables = set() + workspace_used_variables: Set[str] = set() prev_app = None prev_wl = None prev_exp = None - app_used_vars = set() - wl_used_vars = set() - exp_used_vars = set() + app_used_vars: Set[str] = set() + wl_used_vars: Set[str] = set() + exp_used_vars: Set[str] = set() changed = False for _, app_inst, _ in experiment_set.all_experiments(): @@ -2788,7 +2790,8 @@ def no_active_workspace(): yield finally: if ws: - os.environ[RAMBLE_WORKSPACE_VAR] = env_var + if env_var is not None: + os.environ[RAMBLE_WORKSPACE_VAR] = env_var activate(ws) diff --git a/pyproject.toml b/pyproject.toml index e48262b9bb..decac39f92 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -163,6 +163,7 @@ module = [ "_ramble_file_editor", ] ignore_errors = false +check_untyped_defs = true [[tool.mypy.overrides]] # Ignore numpy type stubs since they use 3.12 syntax @@ -170,16 +171,11 @@ module = "numpy.*" follow_imports = "skip" [[tool.mypy.overrides]] -# TODO: Add more modules here for exhaustive type-checking +# Modules exempted from check_untyped_defs until remaining type ambiguities are resolved module = [ - "ramble.util.*", - "ramble.error", - "ramble.paths", - "ramble.filters", - "_ramble_cleaner", - "_ramble_file_editor", + "ramble.test.*", ] -check_untyped_defs = true +check_untyped_defs = false [tool.ruff] extend-include = ["bin/ramble"]