From 9e1ef7aa809835b94eaa4582b15f7222a9649019 Mon Sep 17 00:00:00 2001 From: Corentin Carton de Wiart Date: Wed, 25 Feb 2026 14:40:31 +0000 Subject: [PATCH 1/2] feat(settings): add lisflood-settings-tool CLI for linting and controlled updates - add new CLI module at src/lisflood/settings_tool.py - support updates from YAML and CLI args for lfoptions/lfuser only - preserve comments and write pretty-printed XML output - add --check mode for structure/section validation without writing - intentionally disable lfbinding/lfuser compatibility enforcement - expose tool via setup.py console entry point: lisflood-settings-tool=lisflood.settings_tool:main - document usage examples in README - add focused tests for rewrite, YAML/CLI updates, check mode, and protection against lfbinding edits --- README.md | 16 +++ setup.py | 5 + src/lisflood/settings_tool.py | 250 ++++++++++++++++++++++++++++++++++ tests/test_settings_tool.py | 124 +++++++++++++++++ 4 files changed, 395 insertions(+) create mode 100644 src/lisflood/settings_tool.py create mode 100644 tests/test_settings_tool.py diff --git a/README.md b/README.md index d8bdd2cb..538a72e7 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,22 @@ Windows users are recommended to execute LISFLOOD with a Docker image. The users are recommended to download the [reference settings xml](https://github.com/ec-jrc/lisflood-code/tree/master/src/lisfloodSettings_reference.xml) file and adapt it by inserting their own paths and modelling choices. +## Settings Tool + +This package also installs `lisflood-settings-tool`, a CLI utility to parse, lint and update LISFLOOD settings XML files while preserving comments. + +Examples: + +```bash +# Clean/lint a settings file (no updates) +lisflood-settings-tool in.xml out.xml + +# Update from YAML plus explicit overrides +lisflood-settings-tool in.xml out.xml --yaml updates.yaml --option wateruse=1 --user PathRoot=/data/project + +# Validate only (no output file is written) +lisflood-settings-tool in.xml --check +``` ## Collaborate diff --git a/setup.py b/setup.py index f69f1e17..0745ef3a 100755 --- a/setup.py +++ b/setup.py @@ -159,6 +159,11 @@ def _get_gdal_version(): ], install_requires=requirements, scripts=['bin/lisflood'], + entry_points={ + 'console_scripts': [ + 'lisflood-settings-tool=lisflood.settings_tool:main', + ], + }, zip_safe=True, classifiers=[ # complete classifier list: http://pypi.python.org/pypi?%3Aaction=list_classifiers diff --git a/src/lisflood/settings_tool.py b/src/lisflood/settings_tool.py new file mode 100644 index 00000000..6a6a0f6d --- /dev/null +++ b/src/lisflood/settings_tool.py @@ -0,0 +1,250 @@ +""" +LISFLOOD settings XML formatter and updater. +""" + +from __future__ import absolute_import + +import argparse +import re +import sys + +import yaml +from lxml import etree + + +_PLACEHOLDER_RE = re.compile(r"\$\(([^)]+)\)") +_BUILTIN_BINDING_VARS = {"ProjectDir", "ProjectPath", "SettingsDir", "SettingsPath"} + + +class SettingsToolError(Exception): + """Raised when the settings tool cannot complete requested operations.""" + + +def _parse_bool(value): + if isinstance(value, bool): + return 1 if value else 0 + sval = str(value).strip().lower() + if sval in ("1", "true", "yes", "on"): + return 1 + if sval in ("0", "false", "no", "off"): + return 0 + raise SettingsToolError("Invalid option value '{}'. Use 0/1 or true/false.".format(value)) + + +def _parse_set_expression(expression): + if "=" not in expression: + raise SettingsToolError("Invalid --set '{}' (expected section.name=value)".format(expression)) + lhs, value = expression.split("=", 1) + if "." not in lhs: + raise SettingsToolError( + "Invalid --set '{}' (expected lfoptions.name=value or lfuser.name=value)".format(expression) + ) + section, name = lhs.split(".", 1) + section = section.strip().lower() + if section not in ("lfoptions", "lfuser"): + raise SettingsToolError("Unsupported section '{}' in --set '{}'".format(section, expression)) + if not name.strip(): + raise SettingsToolError("Missing variable name in --set '{}'".format(expression)) + return section, name.strip(), value + + +def _get_required_child(root, names): + for name in names: + node = root.find(name) + if node is not None: + return node + raise SettingsToolError("Missing XML section(s): {}".format(", ".join(names))) + + +def _index_options(lfoptions_elem): + indexed = {} + for node in lfoptions_elem.findall(".//setoption"): + name = node.get("name") + if name: + indexed[name] = node + return indexed + + +def _index_textvars(section_elem): + indexed = {} + for node in section_elem.findall(".//textvar"): + name = node.get("name") + if name: + indexed[name] = node + return indexed + + +def _resolve_with_user(value, user_values): + resolved = value + for _ in range(25): + matches = _PLACEHOLDER_RE.findall(resolved) + if not matches: + return resolved + changed = False + for key in matches: + if key in user_values: + resolved = resolved.replace("$({})".format(key), user_values[key]) + changed = True + if not changed: + return resolved + raise SettingsToolError("Could not resolve placeholders after 25 iterations: {}".format(value)) + + +def _validate_compatibility(binding_nodes, user_values): + issues = [] + allowed = set(user_values.keys()) | _BUILTIN_BINDING_VARS + for name, node in binding_nodes.items(): + value = node.get("value", "") + for ref in _PLACEHOLDER_RE.findall(value): + if ref not in allowed: + issues.append( + "Binding '{}' references undefined variable '$({})'.".format(name, ref) + ) + try: + _resolve_with_user(value, user_values) + except SettingsToolError as exc: + issues.append("Binding '{}' is not resolvable: {}.".format(name, exc)) + return issues + + +def run_tool(input_path, output_path=None, yaml_path=None, set_values=None, option_values=None, user_values=None, check=False): + parser = etree.XMLParser(remove_blank_text=True, remove_comments=False) + tree = etree.parse(input_path, parser) + root = tree.getroot() + if root.tag != "lfsettings": + raise SettingsToolError("Root element must be , found <{}>.".format(root.tag)) + + lfoptions_elem = _get_required_child(root, ("lfoptions",)) + lfuser_elem = _get_required_child(root, ("lfuser",)) + lfbinding_elem = _get_required_child(root, ("lfbinding", "lfbindings")) + + options_index = _index_options(lfoptions_elem) + user_index = _index_textvars(lfuser_elem) + binding_index = _index_textvars(lfbinding_elem) + + merged_option_updates = {} + merged_user_updates = {} + + if yaml_path: + with open(yaml_path, "r") as stream: + payload = yaml.safe_load(stream) or {} + yaml_options = payload.get("lfoptions") or payload.get("options") or {} + yaml_user = payload.get("lfuser") or payload.get("user") or {} + if not isinstance(yaml_options, dict) or not isinstance(yaml_user, dict): + raise SettingsToolError("YAML file must use mapping values for lfoptions/lfuser.") + merged_option_updates.update(yaml_options) + merged_user_updates.update(yaml_user) + + for section, name, value in set_values or []: + if section == "lfoptions": + merged_option_updates[name] = value + else: + merged_user_updates[name] = value + for name, value in option_values or []: + merged_option_updates[name] = value + for name, value in user_values or []: + merged_user_updates[name] = value + + for name, value in merged_option_updates.items(): + node = options_index.get(name) + if node is None: + raise SettingsToolError("lfoptions variable '{}' not found in XML.".format(name)) + node.set("choice", str(_parse_bool(value))) + + for name, value in merged_user_updates.items(): + node = user_index.get(name) + if node is None: + raise SettingsToolError("lfuser variable '{}' not found in XML.".format(name)) + node.set("value", str(value)) + + user_values_map = dict((name, node.get("value", "")) for name, node in user_index.items()) + # Compatibility checks between lfbinding and lfuser are intentionally disabled. + # compatibility_issues = _validate_compatibility(binding_index, user_values_map) + # if compatibility_issues: + # raise SettingsToolError("Compatibility validation failed:\n- {}".format("\n- ".join(compatibility_issues))) + + if check: + return 0 + + if not output_path: + raise SettingsToolError("Output path is required unless --check is used.") + + tree.write(output_path, encoding="utf-8", pretty_print=True) + return 0 + + +def build_parser(): + parser = argparse.ArgumentParser( + description="Parse, validate, lint and update LISFLOOD settings XML files." + ) + parser.add_argument("input", help="Input LISFLOOD settings XML file.") + parser.add_argument("output", nargs="?", help="Output XML path (required unless --check).") + parser.add_argument( + "--yaml", + dest="yaml", + help="YAML file with updates. Format: {lfoptions: {name: 0/1}, lfuser: {name: value}}.", + ) + parser.add_argument( + "--set", + action="append", + help="Update value using section.name=value, where section is lfoptions or lfuser. Can be repeated.", + ) + parser.add_argument( + "--option", + action="append", + help="Update lfoptions value using name=value (equivalent to --set lfoptions.name=value).", + ) + parser.add_argument( + "--user", + action="append", + help="Update lfuser value using name=value (equivalent to --set lfuser.name=value).", + ) + parser.add_argument( + "--check", + action="store_true", + help="Validate XML structure/sections without writing output.", + ) + return parser + + +def main(argv=None): + parser = build_parser() + args = parser.parse_args(argv) + try: + set_values = [] + for expression in args.set or []: + set_values.append(_parse_set_expression(expression)) + option_values = [] + for expression in args.option or []: + if "=" not in expression: + raise SettingsToolError("Invalid --option '{}' (expected name=value)".format(expression)) + name, value = expression.split("=", 1) + if not name.strip(): + raise SettingsToolError("Invalid --option '{}' (empty name)".format(expression)) + option_values.append((name.strip(), value)) + user_values = [] + for expression in args.user or []: + if "=" not in expression: + raise SettingsToolError("Invalid --user '{}' (expected name=value)".format(expression)) + name, value = expression.split("=", 1) + if not name.strip(): + raise SettingsToolError("Invalid --user '{}' (empty name)".format(expression)) + user_values.append((name.strip(), value)) + + run_tool( + input_path=args.input, + output_path=args.output, + yaml_path=args.yaml, + set_values=set_values, + option_values=option_values, + user_values=user_values, + check=args.check, + ) + except (SettingsToolError, etree.XMLSyntaxError, OSError, yaml.YAMLError) as exc: + print("Error: {}".format(exc), file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_settings_tool.py b/tests/test_settings_tool.py new file mode 100644 index 00000000..9711ba3c --- /dev/null +++ b/tests/test_settings_tool.py @@ -0,0 +1,124 @@ +from pathlib import Path + +from lxml import etree + +from lisflood.settings_tool import main + + +SAMPLE_XML = """\ + + + + # option note + + + + + + + + Keep me + + + + + + + + + + + +""" + + +def _write(path, content): + path.write_text(content, encoding="utf-8") + + +def _parse(path): + return etree.parse(str(path)) + + +class TestSettingsTool: + def test_clean_rewrite_and_comment_propagation(self, tmp_path): + src = tmp_path / "settings.xml" + dst = tmp_path / "clean.xml" + _write(src, SAMPLE_XML) + + rc = main([str(src), str(dst)]) + assert rc == 0 + assert dst.exists() + + output = dst.read_text(encoding="utf-8") + assert "top-level comment" in output + assert "Keep me" in output + + tree = _parse(dst) + assert tree.getroot().tag == "lfsettings" + + def test_yaml_and_cli_updates(self, tmp_path): + src = tmp_path / "settings.xml" + dst = tmp_path / "updated.xml" + yml = tmp_path / "updates.yaml" + _write(src, SAMPLE_XML) + _write( + yml, + "lfoptions:\n TemperatureInKelvin: 1\nlfuser:\n SomeUserVar: from-yaml\n", + ) + + rc = main( + [ + str(src), + str(dst), + "--yaml", + str(yml), + "--option", + "wateruse=0", + "--user", + "SomeUserVar=from-cli", + ] + ) + assert rc == 0 + + tree = _parse(dst) + options = {n.get("name"): n.get("choice") for n in tree.findall(".//lfoptions//setoption")} + users = {n.get("name"): n.get("value") for n in tree.findall(".//lfuser//textvar")} + + assert options["TemperatureInKelvin"] == "1" + assert options["wateruse"] == "0" + assert users["SomeUserVar"] == "from-cli" + + def test_check_mode_no_output_written(self, tmp_path): + src = tmp_path / "settings.xml" + dst = tmp_path / "check-output.xml" + _write(src, SAMPLE_XML) + + rc = main([str(src), str(dst), "--check"]) + assert rc == 0 + assert not dst.exists() + + def test_check_allows_unresolved_binding_reference(self, tmp_path): + src = tmp_path / "settings.xml" + bad_xml = SAMPLE_XML.replace( + 'name="MaskMap" value="$(PathRoot)/mask.nc"', + 'name="MaskMap" value="$(MissingRoot)/mask.nc"', + ) + _write(src, bad_xml) + + rc = main([str(src), "--check"]) + assert rc == 0 + + def test_rejects_binding_edits_from_inputs(self, tmp_path): + src = tmp_path / "settings.xml" + dst = tmp_path / "updated.xml" + yml = tmp_path / "updates.yaml" + _write(src, SAMPLE_XML) + _write(yml, "lfbinding:\n MaskMap: /tmp/new-mask.nc\n") + + rc = main([str(src), str(dst), "--yaml", str(yml)]) + assert rc == 0 + + tree = _parse(dst) + bindings = {n.get("name"): n.get("value") for n in tree.findall(".//lfbinding//textvar")} + assert bindings["MaskMap"] == "$(PathRoot)/mask.nc" From 1700403d78107a08940ab06a5b29711f9b7d7483 Mon Sep 17 00:00:00 2001 From: Corentin Carton de Wiart Date: Fri, 27 Feb 2026 10:31:26 +0000 Subject: [PATCH 2/2] refactor(settings): redesign lisflood-settings CLI with subcommands and explicit I/O flags - rename console script from lisflood-settings-tool to lisflood-settings - replace flat interface with subcommands: check and set - switch positional input/output to explicit flags: --input/-i and --output/-o - rename --yaml to --file/-f and --option/--user to --lfoptions/--lfuser - remove deprecated generic --set section.name=value interface - allow list-style and repeated key/value updates for --lfoptions and --lfuser - enforce KEY=VALUE parsing at argparse level for clearer failures - keep edits constrained to lfoptions and lfuser (lfbinding untouched) - update README examples and expand tests for new interface and validation --- README.md | 14 +-- setup.py | 2 +- src/lisflood/settings_tool.py | 224 +++++++++++++--------------------- tests/test_settings_tool.py | 84 +++++++++---- 4 files changed, 159 insertions(+), 165 deletions(-) diff --git a/README.md b/README.md index 538a72e7..eb06ce1e 100644 --- a/README.md +++ b/README.md @@ -133,19 +133,19 @@ The users are recommended to download the [reference settings xml](https://githu ## Settings Tool -This package also installs `lisflood-settings-tool`, a CLI utility to parse, lint and update LISFLOOD settings XML files while preserving comments. +This package also installs `lisflood-settings`, a CLI utility to parse, lint and update LISFLOOD settings XML files while preserving comments. Examples: ```bash -# Clean/lint a settings file (no updates) -lisflood-settings-tool in.xml out.xml +# Validate only (no output file is written) +lisflood-settings check -i in.xml -# Update from YAML plus explicit overrides -lisflood-settings-tool in.xml out.xml --yaml updates.yaml --option wateruse=1 --user PathRoot=/data/project +# Clean/lint a settings file (no updates, writes formatted copy) +lisflood-settings set -i in.xml -o out.xml -# Validate only (no output file is written) -lisflood-settings-tool in.xml --check +# Update from file plus explicit overrides +lisflood-settings set -i in.xml -o out.xml -f updates.yaml --lfoptions wateruse=1 TemperatureInKelvin=0 --lfuser PathRoot=/data/project NetCDFTimeChunks=10 ``` diff --git a/setup.py b/setup.py index 0745ef3a..43aba326 100755 --- a/setup.py +++ b/setup.py @@ -161,7 +161,7 @@ def _get_gdal_version(): scripts=['bin/lisflood'], entry_points={ 'console_scripts': [ - 'lisflood-settings-tool=lisflood.settings_tool:main', + 'lisflood-settings=lisflood.settings_tool:main', ], }, zip_safe=True, diff --git a/src/lisflood/settings_tool.py b/src/lisflood/settings_tool.py index 6a6a0f6d..0c55e366 100644 --- a/src/lisflood/settings_tool.py +++ b/src/lisflood/settings_tool.py @@ -5,17 +5,12 @@ from __future__ import absolute_import import argparse -import re import sys import yaml from lxml import etree -_PLACEHOLDER_RE = re.compile(r"\$\(([^)]+)\)") -_BUILTIN_BINDING_VARS = {"ProjectDir", "ProjectPath", "SettingsDir", "SettingsPath"} - - class SettingsToolError(Exception): """Raised when the settings tool cannot complete requested operations.""" @@ -28,24 +23,7 @@ def _parse_bool(value): return 1 if sval in ("0", "false", "no", "off"): return 0 - raise SettingsToolError("Invalid option value '{}'. Use 0/1 or true/false.".format(value)) - - -def _parse_set_expression(expression): - if "=" not in expression: - raise SettingsToolError("Invalid --set '{}' (expected section.name=value)".format(expression)) - lhs, value = expression.split("=", 1) - if "." not in lhs: - raise SettingsToolError( - "Invalid --set '{}' (expected lfoptions.name=value or lfuser.name=value)".format(expression) - ) - section, name = lhs.split(".", 1) - section = section.strip().lower() - if section not in ("lfoptions", "lfuser"): - raise SettingsToolError("Unsupported section '{}' in --set '{}'".format(section, expression)) - if not name.strip(): - raise SettingsToolError("Missing variable name in --set '{}'".format(expression)) - return section, name.strip(), value + raise SettingsToolError("Invalid lfoptions value '{}'. Use 0/1 or true/false.".format(value)) def _get_required_child(root, names): @@ -74,40 +52,30 @@ def _index_textvars(section_elem): return indexed -def _resolve_with_user(value, user_values): - resolved = value - for _ in range(25): - matches = _PLACEHOLDER_RE.findall(resolved) - if not matches: - return resolved - changed = False - for key in matches: - if key in user_values: - resolved = resolved.replace("$({})".format(key), user_values[key]) - changed = True - if not changed: - return resolved - raise SettingsToolError("Could not resolve placeholders after 25 iterations: {}".format(value)) - - -def _validate_compatibility(binding_nodes, user_values): - issues = [] - allowed = set(user_values.keys()) | _BUILTIN_BINDING_VARS - for name, node in binding_nodes.items(): - value = node.get("value", "") - for ref in _PLACEHOLDER_RE.findall(value): - if ref not in allowed: - issues.append( - "Binding '{}' references undefined variable '$({})'.".format(name, ref) - ) - try: - _resolve_with_user(value, user_values) - except SettingsToolError as exc: - issues.append("Binding '{}' is not resolvable: {}.".format(name, exc)) - return issues - - -def run_tool(input_path, output_path=None, yaml_path=None, set_values=None, option_values=None, user_values=None, check=False): +def _key_value_arg(expression): + if "=" not in expression: + raise argparse.ArgumentTypeError( + "Invalid entry '{}' (expected KEY=VALUE).".format(expression) + ) + name, value = expression.split("=", 1) + if not name.strip(): + raise argparse.ArgumentTypeError( + "Invalid entry '{}' (empty key in KEY=VALUE).".format(expression) + ) + return name.strip(), value + + +def _flatten_pairs(raw_values): + if not raw_values: + return [] + # raw_values shape with nargs='+' and action='append': [[(k,v), ...], ...] + flattened = [] + for values in raw_values: + flattened.extend(values) + return flattened + + +def run_tool(input_path, output_path=None, file_path=None, lfoptions_values=None, lfuser_values=None, check=False): parser = etree.XMLParser(remove_blank_text=True, remove_comments=False) tree = etree.parse(input_path, parser) root = tree.getroot() @@ -116,59 +84,47 @@ def run_tool(input_path, output_path=None, yaml_path=None, set_values=None, opti lfoptions_elem = _get_required_child(root, ("lfoptions",)) lfuser_elem = _get_required_child(root, ("lfuser",)) - lfbinding_elem = _get_required_child(root, ("lfbinding", "lfbindings")) + _ = _get_required_child(root, ("lfbinding", "lfbindings")) + + if check: + return 0 + + if not output_path: + raise SettingsToolError("Output path is required for the 'set' subcommand.") options_index = _index_options(lfoptions_elem) user_index = _index_textvars(lfuser_elem) - binding_index = _index_textvars(lfbinding_elem) - merged_option_updates = {} - merged_user_updates = {} + merged_lfoptions = {} + merged_lfuser = {} - if yaml_path: - with open(yaml_path, "r") as stream: + if file_path: + with open(file_path, "r") as stream: payload = yaml.safe_load(stream) or {} yaml_options = payload.get("lfoptions") or payload.get("options") or {} yaml_user = payload.get("lfuser") or payload.get("user") or {} if not isinstance(yaml_options, dict) or not isinstance(yaml_user, dict): - raise SettingsToolError("YAML file must use mapping values for lfoptions/lfuser.") - merged_option_updates.update(yaml_options) - merged_user_updates.update(yaml_user) - - for section, name, value in set_values or []: - if section == "lfoptions": - merged_option_updates[name] = value - else: - merged_user_updates[name] = value - for name, value in option_values or []: - merged_option_updates[name] = value - for name, value in user_values or []: - merged_user_updates[name] = value - - for name, value in merged_option_updates.items(): + raise SettingsToolError("Update file must use mapping values for lfoptions/lfuser.") + merged_lfoptions.update(yaml_options) + merged_lfuser.update(yaml_user) + + for name, value in lfoptions_values or []: + merged_lfoptions[name] = value + for name, value in lfuser_values or []: + merged_lfuser[name] = value + + for name, value in merged_lfoptions.items(): node = options_index.get(name) if node is None: raise SettingsToolError("lfoptions variable '{}' not found in XML.".format(name)) node.set("choice", str(_parse_bool(value))) - for name, value in merged_user_updates.items(): + for name, value in merged_lfuser.items(): node = user_index.get(name) if node is None: raise SettingsToolError("lfuser variable '{}' not found in XML.".format(name)) node.set("value", str(value)) - user_values_map = dict((name, node.get("value", "")) for name, node in user_index.items()) - # Compatibility checks between lfbinding and lfuser are intentionally disabled. - # compatibility_issues = _validate_compatibility(binding_index, user_values_map) - # if compatibility_issues: - # raise SettingsToolError("Compatibility validation failed:\n- {}".format("\n- ".join(compatibility_issues))) - - if check: - return 0 - - if not output_path: - raise SettingsToolError("Output path is required unless --check is used.") - tree.write(output_path, encoding="utf-8", pretty_print=True) return 0 @@ -177,68 +133,64 @@ def build_parser(): parser = argparse.ArgumentParser( description="Parse, validate, lint and update LISFLOOD settings XML files." ) - parser.add_argument("input", help="Input LISFLOOD settings XML file.") - parser.add_argument("output", nargs="?", help="Output XML path (required unless --check).") - parser.add_argument( - "--yaml", - dest="yaml", - help="YAML file with updates. Format: {lfoptions: {name: 0/1}, lfuser: {name: value}}.", + subparsers = parser.add_subparsers(dest="command", required=True) + + check_parser = subparsers.add_parser( + "check", + help="Validate XML structure/sections without writing output.", ) - parser.add_argument( - "--set", - action="append", - help="Update value using section.name=value, where section is lfoptions or lfuser. Can be repeated.", + check_parser.add_argument("-i", "--input", required=True, help="Input LISFLOOD settings XML file.") + + set_parser = subparsers.add_parser( + "set", + help="Apply updates and write output XML.", ) - parser.add_argument( - "--option", - action="append", - help="Update lfoptions value using name=value (equivalent to --set lfoptions.name=value).", + set_parser.add_argument("-i", "--input", required=True, help="Input LISFLOOD settings XML file.") + set_parser.add_argument("-o", "--output", required=True, help="Output XML path.") + set_parser.add_argument( + "-f", + "--file", + dest="file", + help="YAML file with updates. Format: {lfoptions: {name: 0/1}, lfuser: {name: value}}.", ) - parser.add_argument( - "--user", + set_parser.add_argument( + "--lfoptions", action="append", - help="Update lfuser value using name=value (equivalent to --set lfuser.name=value).", + nargs="+", + type=_key_value_arg, + metavar="KEY=VALUE", + help="One or more lfoptions updates. Example: --lfoptions TemperatureInKelvin=1 wateruse=0", ) - parser.add_argument( - "--check", - action="store_true", - help="Validate XML structure/sections without writing output.", + set_parser.add_argument( + "--lfuser", + action="append", + nargs="+", + type=_key_value_arg, + metavar="KEY=VALUE", + help="One or more lfuser updates. Example: --lfuser PathRoot=/data NetCDFTimeChunks=10", ) + return parser def main(argv=None): parser = build_parser() args = parser.parse_args(argv) + try: - set_values = [] - for expression in args.set or []: - set_values.append(_parse_set_expression(expression)) - option_values = [] - for expression in args.option or []: - if "=" not in expression: - raise SettingsToolError("Invalid --option '{}' (expected name=value)".format(expression)) - name, value = expression.split("=", 1) - if not name.strip(): - raise SettingsToolError("Invalid --option '{}' (empty name)".format(expression)) - option_values.append((name.strip(), value)) - user_values = [] - for expression in args.user or []: - if "=" not in expression: - raise SettingsToolError("Invalid --user '{}' (expected name=value)".format(expression)) - name, value = expression.split("=", 1) - if not name.strip(): - raise SettingsToolError("Invalid --user '{}' (empty name)".format(expression)) - user_values.append((name.strip(), value)) + if args.command == "check": + run_tool(input_path=args.input, check=True) + return 0 + lfoptions_values = _flatten_pairs(args.lfoptions) + lfuser_values = _flatten_pairs(args.lfuser) run_tool( input_path=args.input, output_path=args.output, - yaml_path=args.yaml, - set_values=set_values, - option_values=option_values, - user_values=user_values, - check=args.check, + file_path=args.file, + lfoptions_values=lfoptions_values, + lfuser_values=lfuser_values, + check=False, ) except (SettingsToolError, etree.XMLSyntaxError, OSError, yaml.YAMLError) as exc: print("Error: {}".format(exc), file=sys.stderr) diff --git a/tests/test_settings_tool.py b/tests/test_settings_tool.py index 9711ba3c..472cb86e 100644 --- a/tests/test_settings_tool.py +++ b/tests/test_settings_tool.py @@ -1,5 +1,4 @@ -from pathlib import Path - +import pytest from lxml import etree from lisflood.settings_tool import main @@ -41,12 +40,12 @@ def _parse(path): class TestSettingsTool: - def test_clean_rewrite_and_comment_propagation(self, tmp_path): + def test_set_clean_rewrite_and_comment_propagation(self, tmp_path): src = tmp_path / "settings.xml" dst = tmp_path / "clean.xml" _write(src, SAMPLE_XML) - rc = main([str(src), str(dst)]) + rc = main(["set", "-i", str(src), "-o", str(dst)]) assert rc == 0 assert dst.exists() @@ -57,7 +56,7 @@ def test_clean_rewrite_and_comment_propagation(self, tmp_path): tree = _parse(dst) assert tree.getroot().tag == "lfsettings" - def test_yaml_and_cli_updates(self, tmp_path): + def test_set_file_and_cli_updates_with_list_values(self, tmp_path): src = tmp_path / "settings.xml" dst = tmp_path / "updated.xml" yml = tmp_path / "updates.yaml" @@ -69,14 +68,19 @@ def test_yaml_and_cli_updates(self, tmp_path): rc = main( [ + "set", + "-i", str(src), + "-o", str(dst), - "--yaml", + "--file", str(yml), - "--option", + "--lfoptions", "wateruse=0", - "--user", + "TemperatureInKelvin=1", + "--lfuser", "SomeUserVar=from-cli", + "PathRoot=/data/root", ] ) assert rc == 0 @@ -88,37 +92,75 @@ def test_yaml_and_cli_updates(self, tmp_path): assert options["TemperatureInKelvin"] == "1" assert options["wateruse"] == "0" assert users["SomeUserVar"] == "from-cli" + assert users["PathRoot"] == "/data/root" - def test_check_mode_no_output_written(self, tmp_path): + def test_set_allows_repeating_lfoptions_and_lfuser_flags(self, tmp_path): src = tmp_path / "settings.xml" - dst = tmp_path / "check-output.xml" + dst = tmp_path / "updated.xml" _write(src, SAMPLE_XML) - rc = main([str(src), str(dst), "--check"]) + rc = main( + [ + "set", + "-i", + str(src), + "-o", + str(dst), + "--lfoptions", + "TemperatureInKelvin=1", + "--lfoptions", + "wateruse=0", + "--lfuser", + "SomeUserVar=from-cli", + "--lfuser", + "PathRoot=/alternate/root", + ] + ) assert rc == 0 - assert not dst.exists() - def test_check_allows_unresolved_binding_reference(self, tmp_path): + tree = _parse(dst) + options = {n.get("name"): n.get("choice") for n in tree.findall(".//lfoptions//setoption")} + users = {n.get("name"): n.get("value") for n in tree.findall(".//lfuser//textvar")} + assert options["TemperatureInKelvin"] == "1" + assert options["wateruse"] == "0" + assert users["SomeUserVar"] == "from-cli" + assert users["PathRoot"] == "/alternate/root" + + def test_check_mode_no_output_written(self, tmp_path): src = tmp_path / "settings.xml" - bad_xml = SAMPLE_XML.replace( - 'name="MaskMap" value="$(PathRoot)/mask.nc"', - 'name="MaskMap" value="$(MissingRoot)/mask.nc"', - ) - _write(src, bad_xml) + dst = tmp_path / "check-output.xml" + _write(src, SAMPLE_XML) - rc = main([str(src), "--check"]) + rc = main(["check", "-i", str(src)]) assert rc == 0 + assert not dst.exists() - def test_rejects_binding_edits_from_inputs(self, tmp_path): + def test_rejects_binding_edits_from_file_inputs(self, tmp_path): src = tmp_path / "settings.xml" dst = tmp_path / "updated.xml" yml = tmp_path / "updates.yaml" _write(src, SAMPLE_XML) _write(yml, "lfbinding:\n MaskMap: /tmp/new-mask.nc\n") - rc = main([str(src), str(dst), "--yaml", str(yml)]) + rc = main(["set", "-i", str(src), "-o", str(dst), "--file", str(yml)]) assert rc == 0 tree = _parse(dst) bindings = {n.get("name"): n.get("value") for n in tree.findall(".//lfbinding//textvar")} assert bindings["MaskMap"] == "$(PathRoot)/mask.nc" + + def test_removed_set_option_is_rejected(self, tmp_path): + src = tmp_path / "settings.xml" + dst = tmp_path / "updated.xml" + _write(src, SAMPLE_XML) + + with pytest.raises(SystemExit): + main(["set", "-i", str(src), "-o", str(dst), "--set", "lfuser.SomeUserVar=abc"]) + + def test_lfoptions_rejects_non_key_value_token(self, tmp_path): + src = tmp_path / "settings.xml" + dst = tmp_path / "updated.xml" + _write(src, SAMPLE_XML) + + with pytest.raises(SystemExit): + main(["set", "-i", str(src), "-o", str(dst), "--lfoptions", "wateruse=1", "not-a-pair"])