From c918a9884a71e79672d81ad787836dbd17a1551e Mon Sep 17 00:00:00 2001 From: Mauricio Villegas <5780272+mauvilsa@users.noreply.github.com> Date: Mon, 8 Dec 2025 20:48:09 -0500 Subject: [PATCH] Fix multiple subcommand settings in default_config_files and fail for subcommand chosen (#738). --- CHANGELOG.rst | 4 +++ jsonargparse/_actions.py | 10 +++--- jsonargparse/_common.py | 2 ++ jsonargparse/_core.py | 10 ++++-- jsonargparse/_util.py | 7 ++-- jsonargparse_tests/test_subcommands.py | 46 +++++++++++++++++++++++++- 6 files changed, 69 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 40a4529e..05bbfb6b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -30,12 +30,16 @@ Fixed inheritance (`#815 `__). - ``default_env=True`` conflicting with ``default_config_files`` (`#818 `__). +- ``default_config_files`` with settings for multiple subcommands not working + correctly (`#819 `__). Changed ^^^^^^^ - List of paths types now show in the help the supported options for providing paths like ``'["PATH1",...]' | LIST_OF_PATHS_FILE | -`` (`#816 `__). +- Providing a choice of subcommand in ``default_config_files`` is now an error + (`#819 `__). v4.44.0 (2025-11-25) diff --git a/jsonargparse/_actions.py b/jsonargparse/_actions.py index 1183c899..6b4267d7 100644 --- a/jsonargparse/_actions.py +++ b/jsonargparse/_actions.py @@ -9,7 +9,7 @@ from contextvars import ContextVar from typing import Any, Optional, Union -from ._common import Action, NonParsingAction, is_not_subclass_type, is_subclass, parser_context +from ._common import Action, NonParsingAction, is_not_subclass_type, is_subclass, parser_context, parsing_defaults from ._loaders_dumpers import get_loader_exceptions, load_value from ._namespace import Namespace, NSKeyError, split_key, split_key_root from ._optionals import _get_config_read_mode, ruamel_support @@ -721,7 +721,7 @@ def get_subcommands( return None, None action = parser._subcommands_action - require_single = single_subcommand.get() + require_single = single_subcommand.get() and not parsing_defaults.get() # Get subcommand settings keys subcommand_keys = [k for k in action.choices if isinstance(cfg.get(prefix + k), Namespace)] @@ -731,12 +731,14 @@ def get_subcommands( dest = prefix + action.dest if dest in cfg and cfg.get(dest) is not None: subcommand = cfg[dest] + if parsing_defaults.get(): + raise NSKeyError(f"A specific subcommand can't be provided in defaults, got '{subcommand}'") elif len(subcommand_keys) > 0 and (fail_no_subcommand or require_single): cfg[dest] = subcommand = subcommand_keys[0] if len(subcommand_keys) > 1: warnings.warn( - f'Multiple subcommand settings provided ({", ".join(subcommand_keys)}) without an ' - f'explicit "{dest}" key. Subcommand "{subcommand}" will be used.' + f"Multiple subcommand settings provided ({', '.join(subcommand_keys)}) without an " + f"explicit '{dest}' key. Subcommand '{subcommand}' will be used." ) # Remove extra subcommand settings diff --git a/jsonargparse/_common.py b/jsonargparse/_common.py index 885c87bb..c52dfdba 100644 --- a/jsonargparse/_common.py +++ b/jsonargparse/_common.py @@ -58,6 +58,7 @@ def __call__(self, class_type: type[ClassType], *args, **kwargs) -> ClassType: parser_capture: ContextVar[bool] = ContextVar("parser_capture", default=False) defaults_cache: ContextVar[Optional[Namespace]] = ContextVar("defaults_cache", default=None) lenient_check: ContextVar[Union[bool, str]] = ContextVar("lenient_check", default=False) +parsing_defaults: ContextVar[bool] = ContextVar("parsing_defaults", default=False) load_value_mode: ContextVar[Optional[str]] = ContextVar("load_value_mode", default=None) class_instantiators: ContextVar[Optional[InstantiatorsDictType]] = ContextVar("class_instantiators", default=None) nested_links: ContextVar[list[dict]] = ContextVar("nested_links", default=[]) @@ -70,6 +71,7 @@ def __call__(self, class_type: type[ClassType], *args, **kwargs) -> ClassType: "parser_capture": parser_capture, "defaults_cache": defaults_cache, "lenient_check": lenient_check, + "parsing_defaults": parsing_defaults, "load_value_mode": load_value_mode, "class_instantiators": class_instantiators, "nested_links": nested_links, diff --git a/jsonargparse/_core.py b/jsonargparse/_core.py index adfda3a7..db1e1351 100644 --- a/jsonargparse/_core.py +++ b/jsonargparse/_core.py @@ -457,7 +457,7 @@ def parse_args( # type: ignore[override] skip_validation=skip_validation, ) - except (TypeError, KeyError) as ex: + except (TypeError, KeyError, argparse.ArgumentError) as ex: self.error(str(ex), ex) self._logger.debug("Parsed command line arguments: %s", args) @@ -1011,7 +1011,7 @@ def get_defaults(self, skip_validation: bool = False, **kwargs) -> Namespace: default_config_file_content = default_config_file.get_content() if not default_config_file_content.strip(): continue - with change_to_path_dir(default_config_file), parser_context(parent_parser=self): + with change_to_path_dir(default_config_file), parser_context(parent_parser=self, parsing_defaults=True): cfg_file = self._load_config_parser_mode(default_config_file_content, prev_cfg=cfg) cfg = self.merge_config(cfg_file, cfg) try: @@ -1022,10 +1022,12 @@ def get_defaults(self, skip_validation: bool = False, **kwargs) -> Namespace: defaults=False, skip_validation=skip_validation, skip_required=True, + fail_no_subcommand=False, ) except (TypeError, KeyError, argparse.ArgumentError) as ex: raise argument_error( - f'Problem in default config file "{default_config_file}": {ex.args[0]}' + f"Problem in default config file '{default_config_file}': {ex.args[0]}", + default_config_file=str(default_config_file), ) from ex meta = cfg.get("__default_config__") if isinstance(meta, list): @@ -1054,6 +1056,8 @@ def error(self, message: str, ex: Optional[Exception] = None) -> NoReturn: raise argument_error(message) from ex parser = getattr(ex, "subcommand_parser", None) or self + if getattr(ex, "default_config_file", None): + parser.default_config_files = [] parser.print_usage(sys.stderr) help_action = next((a for a in parser._actions if isinstance(a, argparse._HelpAction)), None) diff --git a/jsonargparse/_util.py b/jsonargparse/_util.py index a420214c..ceb6b8a6 100644 --- a/jsonargparse/_util.py +++ b/jsonargparse/_util.py @@ -43,8 +43,11 @@ default_config_option_help = "Path to a configuration file." -def argument_error(message: str) -> ArgumentError: - return ArgumentError(None, message) +def argument_error(message: str, default_config_file: Optional[str] = None) -> ArgumentError: + ex = ArgumentError(None, message) + if default_config_file: + ex.default_config_file = default_config_file # type: ignore[attr-defined] + return ex class JsonargparseWarning(UserWarning): diff --git a/jsonargparse_tests/test_subcommands.py b/jsonargparse_tests/test_subcommands.py index 26131623..42b09cb9 100644 --- a/jsonargparse_tests/test_subcommands.py +++ b/jsonargparse_tests/test_subcommands.py @@ -160,7 +160,7 @@ def test_subcommands_parse_string_first_implicit_subcommand(subcommands_parser): with warnings.catch_warnings(record=True) as w: cfg = subcommands_parser.parse_string('{"a": {"ap1": "ap1_cfg"}, "b": {"nums": {"val1": 2}}}') assert len(w) == 1 - assert 'Subcommand "a" will be used' in str(w[0].message) + assert "Subcommand 'a' will be used" in str(w[0].message) assert cfg.subcommand == "a" assert "b" not in cfg @@ -436,3 +436,47 @@ def test_subsubcommand_default_config_files(parser, subparser, subsubparser, def assert cfg.clone(with_meta=False) == Namespace( val0=123, subcommand="cmd1", cmd1=Namespace(val1=456, subcommand="cmd2", cmd2=Namespace(val2=789)) ) + + +def test_subcommands_in_default_config_files(parser, subtests, tmp_cwd): + parser.default_config_files = ["defaults.json"] + subs = parser.add_subcommands(required=True, dest="sub") + sub1 = ArgumentParser() + sub1.add_argument("--sub1val") + subs.add_subcommand("sub1", sub1) + sub2 = ArgumentParser() + sub2.add_argument("--sub2val") + subs.add_subcommand("sub2", sub2) + + defaults: dict = { + "sub1": {"sub1val": 2}, + "sub2": {"sub2val": 3}, + } + Path("defaults.json").write_text(json.dumps(defaults)) + + with subtests.test("choose subcommand defaults"): + cfg = parser.parse_args(["sub1"]) + assert cfg.sub == "sub1" + assert cfg.sub1 == Namespace(sub1val=2) + assert "sub2" not in cfg + cfg = parser.parse_args(["sub2"]) + assert cfg.sub == "sub2" + assert cfg.sub2 == Namespace(sub2val=3) + assert "sub1" not in cfg + + with subtests.test("implicit subcommand defaults"): + with warnings.catch_warnings(record=True) as w: + cfg = parser.parse_args([]) + assert "Subcommand 'sub1' will be used" in str(w[0].message) + assert cfg.sub == "sub1" + assert cfg.sub1 == Namespace(sub1val=2) + assert "sub2" not in cfg + + with subtests.test("no subcommand in defaults"): + defaults["sub"] = "sub2" + Path("defaults.json").write_text(json.dumps(defaults)) + err = get_parse_args_stderr(parser, []) + assert ( + "Problem in default config file 'defaults.json': A specific " + "subcommand can't be provided in defaults, got 'sub2'" + ) in err