Enhance ROMI task defaults, config handling, and CLI - #14
Merged
Conversation
- Add new module `romitask/task_defaults.py` - Implement `get_class_defaults` to statically extract class‑level defaults using AST - Implement `get_all_task_defaults` for bulk extraction of defaults, including luigi parameter defaults - Provide `_extract_value` and `_safe_literal_eval` helpers for safe literal evaluation and luigi parameter handling - Add `merge_config_with_defaults` to combine user config with discovered defaults - Add `update_config_with_defaults` and `update_toml_with_defaults` to merge defaults into TOML configuration files - Ensure operations are performed without executing task code, using static source analysis only
- Import `update_config_with_defaults` from `romitask.task_defaults` in `romi_run_task.py`. - Apply `update_config_with_defaults(config, MODULES)` to the loaded pipeline configuration, ensuring undefined task values are populated with class defaults. - Update the `--config` option help text to clarify that only TOML files are concatenated when a directory is provided.
…xing `'upstream_task'` default values - In `romitask/src/romitask/task_defaults.py`, extend the `default` handling for keyword arguments: - After attempting literal evaluation, return the identifier for `ast.Name` nodes. - Build and return a dotted string for `ast.Attribute` nodes by traversing the attribute chain. - Apply equivalent logic to the first positional argument when no explicit `default` keyword is present. - Preserve existing literal evaluation behavior while adding support for class and module references.
…omitask/task_defaults.py` - Parse module‑level constants into a `global_consts` map for literal evaluation. - Build an `import_map` to resolve names imported from other modules. - Extend `_extract_value` to accept `globals_map` and `import_map`, adding support for `ast.Name` and `ast.Attribute` nodes and falling back to static analysis via new helpers. - Introduced `_load_module_globals` and `_evaluate_expression` to safely evaluate constants and environment‑based expressions. - Refactor config merging to prioritize values from the user config, removing parameters not defined in defaults. - Updated documentation examples to use `print` for clearer output.
…n `romitask/src/romitask/task_defaults.py` - Update the function’s docstring. - Refactor the introductory comment to describe the two‑pass evaluation strategy for top‑level constants. - Add a `try/except` around `importlib.util.find_spec` to gracefully handle `ModuleNotFoundError` and return an empty mapping. - Adjust surrounding wording for clarity and consistency.
- Extend the feature list in `task_defaults.py` with a bullet describing how top‑level constants and imported symbols are mapped to resolve references such as `COLMAP_EXE`. - Update the usage examples to print a single task’s defaults and to show that undefined parameters are ignored when merging configurations.
- Wrap `ast.literal_eval` in a `try/except` block in `task_defaults.py` to safely skip non‑literal class attributes during default extraction. - Surround `importlib.util.find_spec` calls with `try/except` for `ModuleNotFoundError` in `task_defaults.py`, returning an empty mapping when a module cannot be located. - Extend `_load_module_globals` exception handling in `task_defaults.py` to catch both `ModuleNotFoundError` and `ValueError`.
- Add `romitask/tests/test_task_defaults.py` covering: - `get_class_defaults` extraction of literals, annotated attributes, lists, dicts, `None`, and handling of complex expressions. - `get_all_task_defaults` for multiple classes, module‑level constants, luigi parameters (keyword & positional), empty modules, missing files, and import resolution. - `_load_module_globals` loading of simple, annotated, and environment‑based constants; handling of nonexistent modules. - `_evaluate_expression` for literals, name references, `os.environ.get`, and `os.getenv` calls. - `_extract_value` for literals, globals, attribute chains, luigi parameters, and unsupported expressions. - `_safe_literal_eval` safe evaluation of valid literals and rejection of invalid expressions. - `merge_config_with_defaults` overriding, adding missing defaults, removing undefined params, and edge cases with empty configs. - `update_config_with_defaults` using custom module mapping and skipping unmapped tasks. - `update_toml_with_defaults` updating TOML files, handling missing files and syntax errors. - Integration tests exercising the full workflow: extract defaults, merge config, and update TOML.
- Traverse class inheritance hierarchy, collecting attributes from parent classes and allowing child class attributes to override them (`get_inherited_defaults`, `get_base_class_defaults`, `extract_class_defaults`). - Build a `class_map` for quick lookup of class definitions within a module. - Merge inherited defaults before processing a class’s own defaults in `all_defaults`. - Exclude `scan_id` attribute from extracted defaults.
- Introduce new `keep_task` luigi parameter in `FilesetTarget` (default `""`) to specify a task whose filesets and all upstream dependencies should be retained. - Implement `_parse_pipeline_toml` to read `pipeline.toml` and extract task‑to‑upstream mappings. - Implement `_compute_task_hierarchy` using a directed graph (networkx) to determine the full set of ancestor tasks for a given `keep_task`. - Use `locate_task_filesets` to map preserved tasks to their corresponding fileset IDs and extend the preserve set `filesets_to_preserve`. - Adjust cleaning logic: - Update exclusion list for `_filesets_to_remove` to respect `filesets_to_preserve`. - Skip `images` metadata cleaning when the `Colmap` task is preserved. - Refine user confirmation message to list preserved filesets. - Enhance orphan metadata cleanup: - `_clean_orphan_json_files` and `_clean_orphan_directories` now accept a `known_fileset` list and remove only truly orphan entries. - Load known fileset IDs from `files.json` before cleanup. - Minor documentation and import updates (`locate_task_filesets`, type annotation for `scan`, markdown headings in docstring).
- Replace `import toml` with `import tomlkit`. - Load TOML files using `tomlkit.load` with explicit UTF‑8 file opening (`open(..., "r", encoding="utf-8")`). - Dump configuration using `tomlkit.dump` (writing `compat_cfg` instead of the original `config`). - Update all relevant load calls (`bak_scan_config`, `bak_pipe_config`, pipeline configs, and main configuration) to use the new loading pattern.
…atibility - Update `create_backup_cfg` signature in `romi_run_task.py` to `config: dict[str, dict[str, Any]]` - Revise comment to "`Save the version number for each ROMI library`" - Create a copy `compat_cfg` of `config` and convert any list or dict task parameters to strings for TOML compatibility - Filter out falsy parameters from each task before dumping - Dump the sanitized `compat_cfg` using `tomlkit.dump` instead of the original `config`
- Extend `run_task` signature in `romi_run_task.py` with `cfg_override: dict[str, Any] | None = None` - Document the new `cfg_override` parameter in the function docstring - Merge `cfg_override` into the loaded configuration before executing the task - Introduce `--cfg` click option (`cfg_override` variable) to accept a TOML string for manual parameter overrides - Parse the `--cfg` string using `tomlkit.loads` with error handling, falling back to `None` on failure - Pass the parsed `cfg_override` to all `run_task` invocations in the CLI flow - Update imports and type hints accordingly
- Fix typo in docstring.
- Rewrite `update_config`:
- Remove unused `Mapping` import.
- Merge task parameters with explicit nested loops and error handling.
- Print informative messages when a parameter cannot be updated.
- Initialize `local_config` inside the local TOML handling block and merge each loaded file using `update_config`; reassign `config` with the merged result.
- Add a debug log `logger.debug(f"{local_config=}")` after loading local definitions.
- Update Luigi command logging to display the command as a space‑joined string via `logger.info`.
- Replace generic module docstring with an extensive overview, feature list, and usage examples for both CLI and programmatic invocation. - Update function docstrings. - Minor typo corrections throughout the file.
…ask.py`
- Added comment and logic to use the provided config dictionary directly when `config` is a `dict`.
- When no previous pipeline configuration (`bak_pipe_config` is falsy), initialize `config = {}` and emit a warning `"Using full default configuration!"` instead of the prior “NO configuration” message.
- Updated loading of local pipeline TOML files to `sorted(local_path.glob('*.toml'), reverse=True)` with a comment, ensuring alphabetical resolution order where later files have lower priority.
- Replace `import toml` with `import tomlkit` in `romitask/cli/print_task_info.py` and update `toml.load` calls to `tomlkit.load`. - Update `romitask/task.py` to import `tomlkit` and use `tomlkit.load` for loading backup configurations. - Switch `romitask/task_defaults.py` imports and example code from `toml` to `tomlkit`, and modify file loading to `tomlkit.load`.
- Updated `romitask/pyproject.toml` to remove `toml` and add `tomlkit` in the dependency list.
- Updated `romitask/tests/test_task_defaults.py` to import `tomlkit` instead of `toml` and use `tomlkit.dump` for writing TOML configs. - Adjusted invalid TOML syntax test to import `ParseError` from `tomlkit.exceptions` and assert that this exception is raised.
- Updated `romitask/src/romitask/task.py` to use `logger.warning` instead of `logger.critical` when the `images` fileset is not found. This lowers the severity of the log message for non‑critical missing data.
- Replace `get_class_defaults` and `get_all_task_defaults` with a single `get_task_defaults` that dynamically imports a module and reads Luigi task parameters via `luigi.task_register.Register`. - Remove extensive AST‑based parsing logic and related helper functions. - Update imports: drop `ast` and `os`, add `luigi.task_register.Register`. - Adjust documentation and examples to reflect the new `get_task_defaults` API. - Convert default values handling: - Exclude the `scan_id` parameter. - Serialize dictionaries, lists, and type objects to strings. - Update the module’s public interface to match the new function signature.
- Replace extensive AST‑based tests with focused tests for `task_defaults.get_task_defaults` using a temporary package and module file. - Rename test class to `TestGetTaskDefaults` and update all related method names. - Remove unused imports `ast` and `os`; add `shutil.rmtree` for recursive cleanup. - Introduce `_write_module` helper to write source code to the temporary test module. - Adjust `setUp`/`tearDown` to create a temporary package (`tmp_pkg`), insert it into `sys.path`, and clean up with `rmtree`. - Update expectations to include the default `upstream_task` entry and simplify default extraction logic. - Use backticks for identifiers such as ``task_defaults.get_task_defaults``, ``tmp_pkg``, ``module_file`` and ``rmtree``.
ArthurLuciani2
approved these changes
Jul 23, 2026
…rarchy # Conflicts: # src/romitask/cli/romi_run_task.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
update_configinromi_run_task.pywith explicit merging logic and improved logging.--cfgCLI option andcfg_overrideparameter to allow TOML‑string overrides of the configuration.create_backup_cfg.keep_taskluigi parameter inFilesetTargetto preserve filesets (and upstream tasks) during cleaning, with graph‑based ancestor computation.get_inherited_defaults, etc.) and integrated it into theromi_run_taskworkflow viaupdate_config_with_defaults.