diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 91ca9fc6..40a4529e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -28,6 +28,8 @@ Fixed `__). - Getting parameter descriptions from docstrings not working for dataclass inheritance (`#815 `__). +- ``default_env=True`` conflicting with ``default_config_files`` (`#818 + `__). Changed ^^^^^^^ diff --git a/jsonargparse/_actions.py b/jsonargparse/_actions.py index 24b02d9c..1183c899 100644 --- a/jsonargparse/_actions.py +++ b/jsonargparse/_actions.py @@ -629,21 +629,9 @@ def add_prefix(key): single_subcommand: ContextVar = ContextVar("single_subcommand", default=True) -parent_parsers: ContextVar = ContextVar("parent_parsers", default=[]) parse_kwargs: ContextVar = ContextVar("parse_kwargs", default={}) -@contextmanager -def parent_parsers_context(key, parser): - prev = parent_parsers.get() - curr = [] if parser is None else prev + [(key, parser)] - token = parent_parsers.set(curr) - try: - yield - finally: - parent_parsers.reset(token) - - class _ActionSubCommands(_SubParsersAction): """Extension of argparse._SubParsersAction to modify subcommands functionality.""" @@ -814,11 +802,10 @@ def handle_subcommands( # Merge environment variable values and default values subnamespace = None key = prefix + subcommand - with parent_parsers_context(key, parser): - if env: - subnamespace = subparser.parse_env(defaults=defaults, _skip_validation=True) - elif defaults: - subnamespace = subparser.get_defaults(skip_validation=True) + if env: + subnamespace = subparser.parse_env(defaults=defaults, _skip_validation=True) + elif defaults: + subnamespace = subparser.get_defaults(skip_validation=True) # Update all subcommand settings if subnamespace is not None: diff --git a/jsonargparse/_core.py b/jsonargparse/_core.py index 75b771ee..adfda3a7 100644 --- a/jsonargparse/_core.py +++ b/jsonargparse/_core.py @@ -28,7 +28,6 @@ _is_action_value_list, _is_branch_key, filter_non_parsing_actions, - parent_parsers, previous_config, ) from ._common import ( @@ -673,7 +672,6 @@ def _load_config_parser_mode( cfg_path: Union[str, os.PathLike] = "", ext_vars: Optional[dict] = None, prev_cfg: Optional[Namespace] = None, - key: Optional[str] = None, ) -> Namespace: """Loads a configuration string into a namespace. @@ -689,8 +687,6 @@ def _load_config_parser_mode( cfg_dict = load_value(cfg_str, path=cfg_path, ext_vars=ext_vars) except get_loader_exceptions() as ex: raise TypeError(f"Problems parsing config: {ex}") from ex - if key and isinstance(cfg_dict, dict): - cfg_dict = cfg_dict.get(key, {}) if not isinstance(cfg_dict, dict): raise TypeError(f"Unexpected config: {cfg_str}") return self._apply_actions(cfg_dict, prev_cfg=prev_cfg) @@ -949,24 +945,18 @@ def save_paths(cfg): ## Methods related to defaults ## - def _get_default_config_files(self) -> list[tuple[Optional[str], Path]]: + def _get_default_config_files(self) -> list[Path]: if getattr(self, "_inner_parser", False): return [] default_config_files = [] - for key, parser in parent_parsers.get(): - for pattern in parser.default_config_files: - files = sorted(glob.glob(os.path.expanduser(pattern))) - default_config_files += [(key, v) for v in files] - for pattern in self.default_config_files: - files = sorted(glob.glob(os.path.expanduser(pattern))) - default_config_files += [(None, x) for x in files] + default_config_files += sorted(glob.glob(os.path.expanduser(pattern))) if len(default_config_files) > 0: with suppress(TypeError): - return [(k, Path(v, mode=_get_config_read_mode())) for k, v in default_config_files] + return [Path(v, mode=_get_config_read_mode()) for v in default_config_files] return [] def get_default(self, dest: str) -> Any: @@ -1017,12 +1007,12 @@ def get_defaults(self, skip_validation: bool = False, **kwargs) -> Namespace: self._logger.debug("Loaded parser defaults: %s", cfg) default_config_files = self._get_default_config_files() - for key, default_config_file in default_config_files: + for default_config_file in default_config_files: 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): - cfg_file = self._load_config_parser_mode(default_config_file_content, key=key, prev_cfg=cfg) + cfg_file = self._load_config_parser_mode(default_config_file_content, prev_cfg=cfg) cfg = self.merge_config(cfg_file, cfg) try: with _ActionPrintConfig.skip_print_config(): diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py index 039a27b3..a7fa7fea 100644 --- a/jsonargparse/_typehints.py +++ b/jsonargparse/_typehints.py @@ -45,7 +45,6 @@ _find_action, _find_parent_action, _is_action_value_list, - parent_parsers_context, parse_kwargs, remove_actions, ) @@ -473,7 +472,7 @@ def skip_sub_defaults_apply(v): or (isinstance(v, dict) and any(is_subclass_spec(e) for e in v.values())) ) - with ActionTypeHint.sub_defaults_context(), parent_parsers_context(None, None): + with ActionTypeHint.sub_defaults_context(): parser._apply_actions(cfg, skip_fn=skip_sub_defaults_apply, prev_cfg=cfg.clone()) @staticmethod diff --git a/jsonargparse_tests/conftest.py b/jsonargparse_tests/conftest.py index aad70abe..8733cb8c 100644 --- a/jsonargparse_tests/conftest.py +++ b/jsonargparse_tests/conftest.py @@ -135,6 +135,11 @@ def subparser() -> ArgumentParser: return ArgumentParser(exit_on_error=False) +@pytest.fixture +def subsubparser() -> ArgumentParser: + return ArgumentParser(exit_on_error=False) + + @pytest.fixture def example_parser() -> ArgumentParser: parser = ArgumentParser(prog="app", exit_on_error=False) diff --git a/jsonargparse_tests/test_actions.py b/jsonargparse_tests/test_actions.py index 615c6bf9..217bbf24 100644 --- a/jsonargparse_tests/test_actions.py +++ b/jsonargparse_tests/test_actions.py @@ -335,8 +335,7 @@ def test_action_parser_conflict_subparser_key(parser, subparser): pytest.raises(ValueError, lambda: parser.add_argument("--inner", action=ActionParser(subparser))) -def test_action_parser_nested_dash_names(parser, subparser): - subsubparser = ArgumentParser() +def test_action_parser_nested_dash_names(parser, subparser, subsubparser): subsubparser.add_argument("--op1-like") subparser.add_argument("--op2-like", action=ActionParser(parser=subsubparser)) assert "a" == subparser.parse_args(["--op2-like.op1-like=a"]).op2_like.op1_like diff --git a/jsonargparse_tests/test_subcommands.py b/jsonargparse_tests/test_subcommands.py index b81e03bb..26131623 100644 --- a/jsonargparse_tests/test_subcommands.py +++ b/jsonargparse_tests/test_subcommands.py @@ -402,10 +402,9 @@ def test_subcommands_custom_instantiator(parser, subparser, subtests): assert init.cmd.cls.call == "subparser" -def test_subsubcommand_default_env_true(parser, subparser): +def test_subsubcommand_default_env_true(parser, subparser, subsubparser): parser.default_env = True parser.env_prefix = "APP" - subsubparser = ArgumentParser() subsubparser.add_argument("--v", type=int, default=1) subcommands1 = parser.add_subcommands() subcommands1.add_subcommand("s1", subparser) @@ -417,3 +416,23 @@ def test_subsubcommand_default_env_true(parser, subparser): with patch.dict(os.environ, {"APP_SUBCOMMAND": "s1", "APP_S1__SUBCOMMAND": "s2"}): cfg = parser.parse_args([]) assert cfg == Namespace(subcommand="s1", s1=Namespace(subcommand="s2", s2=Namespace(v=1))) + + +@pytest.mark.parametrize("default_env", [True, False]) +def test_subsubcommand_default_config_files(parser, subparser, subsubparser, default_env, tmp_cwd): + config = {"val0": 123, "cmd1": {"val1": 456, "cmd2": {"val2": 789}}} + Path("config.json").write_text(json.dumps(config)) + parser.default_env = default_env + parser.default_config_files = ["config.json"] + parser.add_argument("--val0") + subparser.add_argument("--val1") + subsubparser.add_argument("--val2") + subcommands = parser.add_subcommands() + subcommands.add_subcommand("cmd1", subparser) + subsubcommands = subparser.add_subcommands() + subsubcommands.add_subcommand("cmd2", subsubparser) + + cfg = parser.parse_args([]) + assert cfg.clone(with_meta=False) == Namespace( + val0=123, subcommand="cmd1", cmd1=Namespace(val1=456, subcommand="cmd2", cmd2=Namespace(val2=789)) + )