From 18566a05920b3f428c771b17f97b201c70c9c9dc Mon Sep 17 00:00:00 2001 From: Timmy Welch Date: Tue, 13 Dec 2022 08:50:08 -0800 Subject: [PATCH] Fix setting cmdline arguments --- comictaggerlib/settings/manager.py | 34 +++++++++--------- tests/settings_test.py | 58 ++++++++++++++++++++++-------- 2 files changed, 61 insertions(+), 31 deletions(-) diff --git a/comictaggerlib/settings/manager.py b/comictaggerlib/settings/manager.py index 28fab66..bd65e8e 100644 --- a/comictaggerlib/settings/manager.py +++ b/comictaggerlib/settings/manager.py @@ -139,7 +139,7 @@ class Manager: def defaults(self) -> OptionValues: return self.normalize_options({}, file=True, cmdline=True) - def get_namespace(self, options: OptionValues) -> argparse.Namespace: + def get_namespace(self, options: OptionValues, defaults: bool = True) -> argparse.Namespace: """ Returns an argparse.Namespace object with options in the form "{group_name}_{setting_name}" `options` should already be normalized. @@ -150,14 +150,13 @@ class Manager: for setting_name, setting in group.items(): if hasattr(namespace, setting.internal_name): raise Exception(f"Duplicate internal name: {setting.internal_name}") - setattr( - namespace, - setting.internal_name, - options.get(group_name, {}).get( - setting.dest, - setting.default, - ), - ) + value, default = self.get_option(options, setting, group_name) + if not default or default and defaults: + setattr( + namespace, + setting.internal_name, + value, + ) setattr(namespace, "option_definitions", options.get("option_definitions")) return namespace @@ -167,7 +166,7 @@ class Manager: self.option_definitions[self.current_group_name][setting.dest] = setting def create_argparser(self) -> None: - """Creates an argparser object from all cmdline settings""" + """Creates an :class:`argparse.ArgumentParser` from all cmdline settings""" groups: dict[str, ArgParser] = {} self.argparser = argparse.ArgumentParser( description=self.description, @@ -201,7 +200,7 @@ class Manager: self.exclusive_group = False def exit(self, *args: Any, **kwargs: Any) -> NoReturn: - """Same as `argparser.ArgParser.exit`""" + """Same as :class:`~argparse.ArgumentParser`""" self.argparser.exit(*args, **kwargs) raise SystemExit(99) @@ -248,6 +247,7 @@ class Manager: raw_options: OptionValues | argparse.Namespace, file: bool = False, cmdline: bool = False, + defaults: bool = True, raw_options_2: OptionValues | argparse.Namespace | None = None, ) -> OptionValues: """ @@ -261,9 +261,11 @@ class Manager: for setting_name, setting in group.items(): if (setting.cmdline and cmdline) or (setting.file and file): # Ensures the option exists with the default if not already set - group_options[setting_name], _ = self.get_option(raw_options, setting, group_name) + value, default = self.get_option(raw_options, setting, group_name) + if not default or default and defaults: + group_options[setting_name] = value - # will override with option from raw_options_2 if it exists + # will override with option from raw_options_2 if it is not the default if raw_options_2 is not None: value, default = self.get_option(raw_options_2, setting, group_name) if not default: @@ -290,11 +292,11 @@ class Manager: self.create_argparser() ns = self.argparser.parse_args(args, namespace=namespace) - return self.normalize_options(ns, cmdline=True, file=False) + return self.normalize_options(ns, cmdline=True, file=True) def parse_options(self, config_path: pathlib.Path, args: list[str] | None = None) -> OptionValues: file_options = self.parse_file(config_path) - cli_options = self.parse_args(args) + cli_options = self.parse_args(args, self.get_namespace(file_options, defaults=False)) - final_options = self.normalize_options(file_options, file=True, cmdline=True, raw_options_2=cli_options) + final_options = self.normalize_options(cli_options, file=True, cmdline=True) return final_options diff --git a/tests/settings_test.py b/tests/settings_test.py index f7fcb14..7072555 100644 --- a/tests/settings_test.py +++ b/tests/settings_test.py @@ -71,8 +71,8 @@ def test_normalize(settings_manager): defaults_namespace = settings_manager.get_namespace(defaults) defaults_namespace.test = "fail" - normalized = settings_manager.normalize_options(defaults, True) - normalized_namespace = settings_manager.get_namespace(settings_manager.normalize_options(defaults, True)) + normalized = settings_manager.normalize_options(defaults, file=True) + normalized_namespace = settings_manager.get_namespace(settings_manager.normalize_options(defaults, file=True)) assert "test" not in normalized assert "tst" in normalized @@ -97,28 +97,56 @@ def test_normalize(settings_manager): def test_normalize_merge(raw, raw2, expected, settings_manager): settings_manager.add_group("tst", lambda parser: parser.add_setting("--test", default="hello")) - normalized = settings_manager.normalize_options(raw, True, raw_options_2=raw2) + normalized = settings_manager.normalize_options(raw, file=True, raw_options_2=raw2) assert normalized["tst"]["test"] == expected -def test_parse_options(settings_manager, tmp_path): +def test_cli_set(settings_manager, tmp_path): settings_file = tmp_path / "settings.json" - settings_file.write_text(json.dumps({"tst2": {"test2": "success"}, "tst3": {"test3": "fail"}})) + settings_file.write_text(json.dumps({})) settings_manager.add_group("tst", lambda parser: parser.add_setting("--test", default="hello", file=False)) - settings_manager.add_group("tst2", lambda parser: parser.add_setting("--test2", default="hello", cmdline=False)) - settings_manager.add_group("tst3", lambda parser: parser.add_setting("--test3", default="hello")) - normalized = settings_manager.parse_options(settings_file, ["--test", "success", "--test3", "success"]) + normalized = settings_manager.parse_options(settings_file, ["--test", "success"]) - # Tests that the cli will override the default assert "test" in normalized["tst"] assert normalized["tst"]["test"] == "success" - # Tests that the settings file will override the default - assert "test2" in normalized["tst2"] - assert normalized["tst2"]["test2"] == "success" - # Tests that the cli will override the settings file - assert "test3" in normalized["tst3"] - assert normalized["tst3"]["test3"] == "success" +def test_file_set(settings_manager, tmp_path): + settings_file = tmp_path / "settings.json" + settings_file.write_text( + json.dumps( + { + "tst": {"test": "success"}, + } + ) + ) + settings_manager.add_group("tst", lambda parser: parser.add_setting("--test", default="hello", cmdline=False)) + + normalized = settings_manager.parse_options(settings_file, []) + + assert "test" in normalized["tst"] + assert normalized["tst"]["test"] == "success" + + +def test_cli_override_file(settings_manager, tmp_path): + settings_file = tmp_path / "settings.json" + settings_file.write_text(json.dumps({"tst": {"test": "fail"}})) + settings_manager.add_group("tst", lambda parser: parser.add_setting("--test", default="hello")) + + normalized = settings_manager.parse_options(settings_file, ["--test", "success"]) + + assert "test" in normalized["tst"] + assert normalized["tst"]["test"] == "success" + + +def test_cli_explicit_default(settings_manager, tmp_path): + settings_file = tmp_path / "settings.json" + settings_file.write_text(json.dumps({"tst": {"test": "fail"}})) + settings_manager.add_group("tst", lambda parser: parser.add_setting("--test", default="success")) + + normalized = settings_manager.parse_options(settings_file, ["--test", "success"]) + + assert "test" in normalized["tst"] + assert normalized["tst"]["test"] == "success"