diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 46fc1244..81288824 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -27,7 +27,8 @@ Added - Parsing now keeps track of where each value came from. Errors due to a given value say so, e.g. ``Source: config file config.yaml:3`` followed by that line of the file, and the new ``provenance`` flag of the print config argument adds - it as comments (`#975 `__). + it as comments (`#975 `__, + `#1001 `__). - New ``docstrings`` and ``typeshed`` extras, which install individually the respective optional dependencies of the ``signatures`` extra (`#988 `__). diff --git a/DOCUMENTATION.rst b/DOCUMENTATION.rst index 0c36ecf6..6e6db979 100644 --- a/DOCUMENTATION.rst +++ b/DOCUMENTATION.rst @@ -1508,9 +1508,9 @@ comma, e.g. ``--print_config=comments,skip_default``. The ``comments`` and dataclass, the descriptions from that class. - ``provenance``: add to each value a YAML comment saying where it came from, i.e. a default, a default config file, a config file, a config string, an - environment variable or a command line argument. For config files parsed as - YAML, the comment includes the line number, e.g. ``# config file - config.yaml:3``. + environment variable or a command line argument, or ``implicit`` and what + implied it, e.g. a ``class_path`` not given. For config files parsed as YAML, + the comment includes the line number, e.g. ``# config file config.yaml:3``. - ``skip_default``: skip entries whose value is the same as the default. - ``skip_unset``: skip entries that were not given a value, see :ref:`unset-values`. diff --git a/jsonargparse/_formatters.py b/jsonargparse/_formatters.py index 8facb7be..1427a97e 100644 --- a/jsonargparse/_formatters.py +++ b/jsonargparse/_formatters.py @@ -343,6 +343,8 @@ def _set_provenance_comments(cfg, provenance: dict, key_lines: dict, prefix: str def describe_source(source, cache: dict | None = None) -> str: """Returns a description of where a value came from, including the line number for config files.""" + if source.implied_by is not None: + return f"{source.description}, from {describe_source(source.implied_by, cache)}" text = source.description if source.origin is None else f"{source.description} {_describe_origin(source.origin)}" position = _find_position(source, {} if cache is None else cache) return text if position is None else f"{text}:{position[0]}" diff --git a/jsonargparse/_namespace.py b/jsonargparse/_namespace.py index cdae856a..5890d5fe 100644 --- a/jsonargparse/_namespace.py +++ b/jsonargparse/_namespace.py @@ -22,11 +22,16 @@ class ValueSource(NamedTuple): origin: Any = None # the path of a config file or the name of an environment variable mode: str | None = None # the parser mode of a config file, required to find line numbers key: str | None = None # the key in a config file that the value corresponds to + implied_by: "ValueSource | None" = None # for implicit values, e.g. a class_path, the source of what implied it def __deepcopy__(self, memo): return self # immutable, so copies of namespaces can share it, which is much faster, e.g. for list items +# Marks a value that was not given, like a class_path derived from a type, until the source that implied it is known +implicit_source = ValueSource("implicit") + + # The source of the values being parsed, which is set to the values written to namespaces. Code that # processes values that were already parsed must run with None, so that they keep their sources. value_source: ContextVar[ValueSource | None] = ContextVar("value_source", default=None) @@ -284,7 +289,7 @@ def update(self, value: "Namespace | Any", key: str | None = None, only_unset: b for subkey, subval in value.items(): if not only_unset or prefix + subkey not in self: self[prefix + subkey] = subval - _set_source(self, prefix + subkey, provenance.get(subkey, value_source.get())) + _set_source(self, prefix + subkey, _resolve_implicit(provenance.get(subkey, value_source.get()))) return self def get(self, key: str, default: Any = None) -> Any: @@ -366,6 +371,19 @@ def _get_provenance_dict(namespace: Namespace) -> dict[str, ValueSource]: return provenance +def set_implicit_source(namespace: Namespace, key: str) -> None: + """Marks a key as implicit, so that later it gets as source the one of the value that implied it.""" + _get_provenance_dict(namespace)[key] = implicit_source + + +def _resolve_implicit(source: ValueSource | None) -> ValueSource | None: + """For an implicit source, the same with the source of the values being parsed as what implied it.""" + implied_by = value_source.get() + if source is not implicit_source or implied_by is None: + return source + return source._replace(implied_by=implied_by) + + def _set_source(namespace: Namespace, key: str, source: ValueSource | None) -> None: leaf_key, parent_ns, _ = namespace._parse_key(key) if source is None: @@ -416,6 +434,8 @@ def fill_provenance(value: Any, source: ValueSource) -> None: item_source = _child_source(source, "." + del_clash_mark(name)) if not isinstance(item, Namespace) and name not in provenance: provenance[name] = item_source + elif provenance.get(name) is implicit_source: + provenance[name] = implicit_source._replace(implied_by=item_source) fill_provenance(item, item_source) elif isinstance(value, (list, dict)): for name, item in enumerate(value) if isinstance(value, list) else value.items(): diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py index 38170839..474d14e1 100644 --- a/jsonargparse/_typehints.py +++ b/jsonargparse/_typehints.py @@ -87,6 +87,7 @@ ValueSource, copy_provenance, fill_provenance, + set_implicit_source, subclasses_disabled_meta_key, value_source_context, ) @@ -737,6 +738,8 @@ def _check_type(self, value, append=False, cfg=None, mode=None): prev_val = cfg.get(self.dest) if cfg else unset_sentinel if prev_val is unset_sentinel and not sub_defaults.get() and is_subclass_spec(self.default): prev_val = Namespace(class_path=self.default["class_path"]) + copy_provenance(self.default, prev_val, key="class_path") # keeps whether it is implicit + fill_provenance(prev_val, ValueSource("default")) kwargs = { "sub_add_kwargs": getattr(self, "sub_add_kwargs", {}), @@ -1816,6 +1819,7 @@ def adapt_typehints( if return_type and not inspect.isabstract(return_type): with suppress(ValueError): prev_val = Namespace(class_path=get_import_path(return_type)) + set_implicit_source(prev_val, "class_path") val = subclass_spec_as_namespace(val, prev_val) if not is_subclass_spec(val): raise ImportError( @@ -1869,6 +1873,7 @@ def adapt_typehints( with suppress(ValueError): # implicit prev_val class_path prev_val = Namespace(class_path=get_code_given_class_path(typehint)) + set_implicit_source(prev_val, "class_path") if parse_kwargs.get().get("defaults") is True: prev_implicit_defaults = True @@ -1881,12 +1886,14 @@ def adapt_typehints( isinstance(val, NestedArg) and is_subclasses_disabled(typehint) ): class_type_path = Namespace(class_path=get_code_given_class_path(typehint)) + set_implicit_source(class_type_path, "class_path") val = subclass_spec_as_namespace(val, class_type_path) else: val = subclass_spec_as_namespace(val, prev_val) if val and not is_subclass_spec(val) and "init_args" not in val: # implicit val class_path val = Namespace(class_path=get_code_given_class_path(typehint), init_args=val) + set_implicit_source(val, "class_path") if not is_subclass_spec(val): msg = "Does not implement protocol" if is_protocol(typehint) else "Not a valid subclass of" diff --git a/jsonargparse_tests/test_provenance.py b/jsonargparse_tests/test_provenance.py index 4891c5ed..6115a9c6 100644 --- a/jsonargparse_tests/test_provenance.py +++ b/jsonargparse_tests/test_provenance.py @@ -7,7 +7,7 @@ import pickle import shutil from pathlib import Path -from typing import Optional +from typing import Callable, Optional from unittest.mock import patch import pytest @@ -333,6 +333,67 @@ def test_subclass_subconfig_file(parser, tmp_cwd): assert get_sources(parser.parse_args(["--config=cfg.json"])) == expected +def test_subclass_implicit_class_path(parser, tmp_cwd): + parser.add_argument("--opt", type=Optimizer) + Path("cfg.json").write_text('{"opt": {"lr": 0.5}}') + assert get_sources(parser.parse_args(["--config=cfg.json"])) == { + "opt.class_path": "implicit, from config file cfg.json", + "opt.init_args.lr": "config file cfg.json", + } + assert get_sources(parser.parse_args(["--opt.lr=0.5"])) == { + "opt.class_path": "implicit, from command line argument --opt.lr", + "opt.init_args.lr": "command line argument --opt.lr", + } + assert get_sources(parser.parse_args(['--opt={"init_args": {"lr": 0.5}}'])) == { + "opt.class_path": "implicit, from command line argument --opt", + "opt.init_args.lr": "command line argument --opt", + } + + +def test_subclass_implicit_class_path_kept_by_later_init_arg(parser, tmp_cwd): + parser.add_argument("--opt", type=Optimizer) + Path("cfg.json").write_text('{"opt": {"lr": 0.5}}') + assert get_sources(parser.parse_args(["--config=cfg.json", "--opt.lr=0.7"])) == { + "opt.class_path": "implicit, from config file cfg.json", + "opt.init_args.lr": "command line argument --opt.lr", + } + + +def test_subclass_class_path_from_default(parser, monkeypatch): + parser.add_argument("--opt", type=Optimizer, default={"class_path": sgd}) + parser.add_argument("--implicit", type=Optimizer, default={"lr": 0.3}) + monkeypatch.setenv("APP_OPT", '{"lr": 0.5}') + monkeypatch.setenv("APP_IMPLICIT", '{"lr": 0.5}') + parser.default_env = True + assert get_sources(parser.parse_args([])) == { + "opt.class_path": "default", + "opt.init_args.lr": "environment variable APP_OPT", + "opt.init_args.momentum": "default", + "implicit.class_path": "implicit, from default", + "implicit.init_args.lr": "environment variable APP_IMPLICIT", + } + + +def test_list_of_subclasses_implicit_class_path(parser, tmp_cwd): + parser.add_argument("--opts", type=list[Optimizer]) + Path("cfg.json").write_text(json.dumps({"opts": [{"lr": 0.5}, {"class_path": sgd}]})) + assert get_sources(parser.parse_args(["--config=cfg.json"])) == { + "opts[0].class_path": "implicit, from config file cfg.json", + "opts[0].init_args.lr": "config file cfg.json", + "opts[1].class_path": "config file cfg.json", + "opts[1].init_args.lr": "default", + "opts[1].init_args.momentum": "default", + } + + +def test_callable_return_class_implicit_class_path(parser): + parser.add_argument("--opt", type=Callable[[], Optimizer]) + assert get_sources(parser.parse_args(["--opt.lr=0.5"])) == { + "opt.class_path": "implicit, from command line argument --opt.lr", + "opt.init_args.lr": "command line argument --opt.lr", + } + + def test_subcommand(parser, subparser, tmp_cwd): subparser.add_argument("--config", action="config") subparser.add_argument("--val", type=int, default=1) @@ -696,7 +757,7 @@ def test_subclass_implicit_init_args_line_numbers(yaml_parser, tmp_cwd): yaml_parser.add_argument("--opt", type=Optimizer) Path("cfg.yaml").write_text("opt:\n lr: 0.5\n") assert get_sources(yaml_parser.parse_args(["--config=cfg.yaml"])) == { - "opt.class_path": "config file cfg.yaml:1", + "opt.class_path": "implicit, from config file cfg.yaml:1", "opt.init_args.lr": "config file cfg.yaml:2", } @@ -712,7 +773,7 @@ def test_subclass_subconfig_file_line_numbers(yaml_parser, tmp_cwd): "opt.init_args.momentum": "config file sub.yaml:3", } assert get_sources(yaml_parser.parse_args(["--opt=implicit.yaml"])) == { - "opt.class_path": "config file implicit.yaml", + "opt.class_path": "implicit, from config file implicit.yaml", "opt.init_args.lr": "config file implicit.yaml:1", } @@ -814,6 +875,19 @@ def test_print_config_provenance(yaml_parser, tmp_cwd, monkeypatch): ) +@skip_if_no_ruamel +def test_print_config_provenance_implicit_class_path(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--opt", type=Optimizer) + Path("cfg.yaml").write_text("opt:\n lr: 0.5\n") + out = get_parse_args_stdout(yaml_parser, ["--config=cfg.yaml", "--print_config=provenance"]) + assert out == ( + "opt:\n" + f" class_path: {optimizer} # implicit, from config file cfg.yaml:1\n" + " init_args:\n" + " lr: 0.5 # config file cfg.yaml:2\n" + ) + + @skip_if_no_ruamel def test_print_config_provenance_list_of_subclasses(yaml_parser, tmp_cwd): yaml_parser.add_argument("--opts", type=list[Optimizer])