diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a19aad29..a4c1d72a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -24,6 +24,15 @@ Added - Support for methods and ``__init__`` defined with ``functools.partialmethod``, both for adding their parameters and as import paths of callables (`#665 `__). +- 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 `__). + +Fixed +^^^^^ +- Parsing a list took time quadratic in its number of items (`#975 + `__). Changed ^^^^^^^ @@ -63,6 +72,9 @@ Changed ``tomli`` for reading in python 3.10, instead of the unmaintained ``toml`` package. Dumped toml arrays are now multi-line (`#973 `__). +- The ``comments`` flag of the print config argument is now always accepted and + listed in the help, and fails with an informative error when ``ruamel.yaml`` + is not installed (`#975 `__). Removed ^^^^^^^ diff --git a/DOCUMENTATION.rst b/DOCUMENTATION.rst index 1c566b2b..0afe982b 100644 --- a/DOCUMENTATION.rst +++ b/DOCUMENTATION.rst @@ -222,7 +222,9 @@ arguments is given to it, instead of taking them from the command line: (2.3, ) If parsing fails, by default the usage is printed and the program exits. With -``exit_on_error=False`` an :class:`.ArgumentError` is raised instead. +``exit_on_error=False`` an :class:`.ArgumentError` is raised instead. When the +failure is due to a given value, the error message says where it came from, e.g. +``Source: config file config.yaml:3`` followed by that line of the file. Override order @@ -1409,13 +1411,18 @@ Serialization Parsers that have an ``action="config"`` argument also get a ``--print_config`` option. It is useful for tools with many options, to create an initial config file with all default values. The option accepts one or more flags separated by -comma, e.g. ``--print_config=comments,skip_default``: - -- ``comments``: add the help descriptions as YAML comments. Requires the - `ruamel.yaml `__ package. The comments - are the descriptions of the groups and arguments of the parser and, for values - that correspond to a class, e.g. the ``init_args`` of a subclass or the fields - of a dataclass, the descriptions from that class. +comma, e.g. ``--print_config=comments,skip_default``. The ``comments`` and +``provenance`` flags require the `ruamel.yaml +`__ package: + +- ``comments``: add the help descriptions as YAML comments. The comments are the + descriptions of the groups and arguments of the parser and, for values that + correspond to a class, e.g. the ``init_args`` of a subclass or the fields of a + 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``. - ``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/_actions.py b/jsonargparse/_actions.py index a2da5491..e8b82c80 100644 --- a/jsonargparse/_actions.py +++ b/jsonargparse/_actions.py @@ -19,7 +19,7 @@ parser_context, ) from ._loaders_dumpers import get_loader_exceptions, load_value -from ._namespace import Namespace +from ._namespace import Namespace, ValueSource, copy_provenance, value_source_context from ._optionals import _get_config_read_mode, ruamel_support from ._paths import change_to_path_dir from ._type_checking import ArgumentParser @@ -140,6 +140,7 @@ def apply_config(parser, cfg, dest, value) -> None: cfg_file = parser.parse_path(value, **kwargs) cfg_merged = merge_config(parser, cfg_file, cfg) cfg.__dict__.update(cfg_merged.__dict__) + copy_provenance(cfg_merged, cfg) if cfg.get(dest) is get_parsing_setting("unset_sentinel"): cfg[dest] = [] cfg[dest].append(cfg_path) @@ -182,20 +183,26 @@ def __init__( help=( "Print the configuration after applying all other arguments and exit. The optional " "flags customizes the output and are one or more keywords separated by comma. The " - "supported flags are:%s skip_default, skip_unset." - ) - % (" comments," if ruamel_support else ""), + "supported flags are: comments, provenance, skip_default, skip_unset." + ), ) def __call__(self, parser, namespace, value, option_string=None): kwargs = {"subparser": parser, "key": None, "skip_unset": False, "skip_validation": False} - valid_flags = {"": None, "skip_default": "skip_default", "skip_unset": "skip_unset"} - if ruamel_support: - valid_flags["comments"] = "with_comments" + valid_flags = { + "": None, + "comments": "with_comments", + "provenance": "_with_provenance", + "skip_default": "skip_default", + "skip_unset": "skip_unset", + } flags = value[0].split(",") invalid_flags = [f for f in flags if f not in valid_flags] if len(invalid_flags) > 0: raise argument_error(f'Invalid option "{invalid_flags[0]}" for {option_string}') + ruamel_flags = [f for f in flags if f in {"comments", "provenance"}] + if ruamel_flags and not ruamel_support: + raise argument_error(f'{option_string} flag "{ruamel_flags[0]}" requires the ruamel.yaml package') for flag in [f for f in flags if f != ""]: kwargs[valid_flags[flag]] = True while hasattr(parser, "parent_parser"): @@ -308,7 +315,8 @@ def _load_config(self, value, parser): cfg = self.resolve_subclass_spec(cfg) if not isinstance(cfg, (dict, Namespace)): raise TypeError(f'Parser key "{self.dest}": Unable to load config "{value}"') - with load_config_path_context(cfg_path), change_to_path_dir(cfg_path): + source = None if cfg_path is None else ValueSource("config file", cfg_path, parser.parser_mode) + with load_config_path_context(cfg_path), change_to_path_dir(cfg_path), value_source_context(source): cfg = parser._apply_actions(cfg, parent_key=self.dest) return cfg except (SubclassesDisabledError, ImportDenied) as ex: diff --git a/jsonargparse/_common.py b/jsonargparse/_common.py index d9016e09..46d752cb 100644 --- a/jsonargparse/_common.py +++ b/jsonargparse/_common.py @@ -14,7 +14,7 @@ _GenericAlias, ) -from ._namespace import Namespace +from ._namespace import Namespace, ValueSource, value_source from ._optionals import ( _set_config_read_mode, _set_docstring_parse_options, @@ -79,6 +79,7 @@ def __bool__(self): nested_links: ContextVar[list[dict]] = ContextVar("nested_links", default=[]) applied_instantiation_links: ContextVar[set | None] = ContextVar("applied_instantiation_links", default=None) path_dump_preserve_relative: ContextVar[bool] = ContextVar("path_dump_preserve_relative", default=False) +command_line_option: ContextVar[str | None] = ContextVar("command_line_option", default=None) parser_context_vars = { @@ -93,6 +94,7 @@ def __bool__(self): "nested_links": nested_links, "applied_instantiation_links": applied_instantiation_links, "path_dump_preserve_relative": path_dump_preserve_relative, + "command_line_option": command_line_option, } @@ -110,6 +112,14 @@ def parser_context(**kwargs): context_var.reset(token) +def command_line_source(action: argparse.Action, option_string: str | None = None) -> ValueSource: + """Returns the source of a value given as a command line argument, named as the option that was used.""" + option = command_line_option.get() or option_string # a nested parse refers to the option being parsed + if option is None: + option = max(action.option_strings, key=len) if action.option_strings else action.dest + return ValueSource("command line argument", option) + + class ImportDenied(ImportError, ValueError): """Raised when an import path given as a value is not allowed. @@ -763,7 +773,11 @@ def _check_type_(self, value, **kwargs): if not hasattr(self, "_check_type_kwargs"): self._check_type_kwargs = set(inspect.signature(self._check_type).parameters) kwargs = {k: v for k, v in kwargs.items() if k in self._check_type_kwargs} - return self._check_type(value, **kwargs) + token = value_source.set(None) # adapting a value does not change where it came from + try: + return self._check_type(value, **kwargs) + finally: + value_source.reset(token) class NonParsingAction(Action): diff --git a/jsonargparse/_completions.py b/jsonargparse/_completions.py index 52da2201..bf0df067 100644 --- a/jsonargparse/_completions.py +++ b/jsonargparse/_completions.py @@ -80,7 +80,7 @@ def parse_known_args(self, args=None, namespace=None): def get_argcomplete_namespace(parser, namespace): - namespace.__class__ = __import__("jsonargparse").Namespace + namespace = __import__("jsonargparse").Namespace(namespace) return merge_config(parser, parser.get_defaults(skip_validation=True), namespace).as_flat() diff --git a/jsonargparse/_core.py b/jsonargparse/_core.py index 1cbd5eac..6a77e781 100644 --- a/jsonargparse/_core.py +++ b/jsonargparse/_core.py @@ -23,6 +23,7 @@ previous_config, ) from ._common import ( + command_line_source, config_schema_key, debug_mode_active, get_optionals_as_positionals_actions, @@ -37,7 +38,7 @@ from ._completions import ( get_completion_script as get_completion_script_internal, ) -from ._formatters import DefaultHelpFormatter, get_env_var +from ._formatters import DefaultHelpFormatter, describe_source, get_env_var, get_source_snippet from ._instantiation import InstantiateMethod from ._jsonnet import ActionJsonnet from ._jsonschema import ActionJsonSchema @@ -52,12 +53,17 @@ from ._namespace import ( Namespace, NSKeyError, + ValueSource, + copy_provenance, get_non_meta_sorted_keys, + get_provenance, is_meta_key, recreate_branches, remove_meta, split_key_leaf, split_key_root, + value_source, + value_source_context, ) from ._optionals import ( _get_config_read_mode, @@ -113,6 +119,17 @@ _parse_known_has_intermixed = "intermixed" in inspect.signature(argparse.ArgumentParser._parse_known_args).parameters +def _get_error_source(ex: BaseException | None) -> ValueSource | None: + """Returns where the value that caused an error came from, the innermost one when nested.""" + source = None + while ex is not None: + if getattr(ex, "value_source_reported", False): # already in the message of a nested error + return None + source = getattr(ex, "value_source", source) + ex = ex.__cause__ or ex.__context__ + return source + + class ActionsContainer(ArgumentLinking, InstantiateMethod, SignatureArguments, argparse._ActionsContainer): """Extension of ``argparse._ActionsContainer`` to support additional functionalities.""" @@ -314,7 +331,12 @@ def _parse_known_args_internal(self, args=None, namespace=None, *, argcomplete: kwargs = {} if _parse_known_has_intermixed: kwargs["intermixed"] = False - namespace, args = self._parse_known_args(args, namespace, **kwargs) + # not a value_source_context, since its errors would get the source of the previous argument + token = value_source.set(None) + try: + namespace, args = self._parse_known_args(args, namespace, **kwargs) + finally: + value_source.reset(token) except argparse.ArgumentError as ex: self.error(str(ex), ex) @@ -333,7 +355,8 @@ def _positional_optionals(self, cfg, unk): value = unk.pop(0) try: - cfg[action.dest] = self._check_value_key(action, value, action.dest, cfg) + with value_source_context(command_line_source(action)): + cfg[action.dest] = self._check_value_key(action, value, action.dest, cfg) except (TypeError, ValueError) as ex: if isinstance(value, str) and value.startswith("--"): raise argument_error(f"unrecognized arguments: {' '.join([value] + unk)}") from ex @@ -352,6 +375,16 @@ def _parse_optional(self, arg_string): arg_string += "=" return super()._parse_optional(arg_string) + def _get_values(self, action, arg_strings): + values = super()._get_values(action, arg_strings) + # the source of what the action sets right after this, none for a positional that was left out + value_source.set(command_line_source(action) if arg_strings or action.option_strings else None) + return values + + def _get_value(self, action, arg_string): + value_source.set(None) # also used by argparse after the arguments, to convert string defaults + return super()._get_value(action, arg_string) + def _parse_common( self, cfg: Namespace, @@ -382,29 +415,30 @@ def _parse_common( if env is None and self._default_env: env = True - if not skip_subcommands: - handle_subcommands(self, cfg, env=env, defaults=defaults, fail_no_subcommand=fail_no_subcommand) + with value_source_context(None): + if not skip_subcommands: + handle_subcommands(self, cfg, env=env, defaults=defaults, fail_no_subcommand=fail_no_subcommand) - if defaults: - with parser_context(lenient_check=True): - ActionTypeHint.add_sub_defaults(self, cfg) + if defaults: + with parser_context(lenient_check=True): + ActionTypeHint.add_sub_defaults(self, cfg) - with parser_context(parent_parser=self): - if not lenient_check.get() and self.parser_mode == "omegaconf+": - cfg = omegaconf_apply(self, cfg) + with parser_context(parent_parser=self): + if not lenient_check.get() and self.parser_mode == "omegaconf+": + cfg = omegaconf_apply(self, cfg) - _ActionPrintConfig.print_config_if_requested(self, cfg) + _ActionPrintConfig.print_config_if_requested(self, cfg) - try: - ActionLink.apply_parsing_links(self, cfg) - except Exception as ex: - self.error(str(ex), ex) + try: + ActionLink.apply_parsing_links(self, cfg) + except Exception as ex: + self.error(str(ex), ex) - if not skip_validation: - self.validate(cfg, skip_required=skip_required) + if not skip_validation: + self.validate(cfg, skip_required=skip_required) - if not lenient_check.get() and not nested_parse: - cfg = subclasses_disabled_remove_class_path(cfg) + if not lenient_check.get() and not nested_parse: + cfg = subclasses_disabled_remove_class_path(cfg) return cfg @@ -558,10 +592,12 @@ def _load_env_vars(self, env: dict[str, str] | os._Environ, defaults: bool) -> N if isinstance(action, ActionSubCommands): env_val = env[env_var] if env_val in action.choices: - cfg[action.dest] = subcommand = self._check_value_key(action, env_val, action.dest, cfg) + with value_source_context(ValueSource("environment variable", env_var)): + cfg[action.dest] = subcommand = self._check_value_key(action, env_val, action.dest, cfg) pcfg = action._name_parser_map[env_val].parse_env(env=env, defaults=defaults, _skip_validation=True) for k, v in vars(pcfg).items(): cfg[subcommand + "." + k] = v + copy_provenance(pcfg, cfg[subcommand]) for action, env_var in given: if not isinstance(action, (ActionConfigFile, ActionSubCommands)): env_val = env[env_var] @@ -581,7 +617,8 @@ def _load_env_vars(self, env: dict[str, str] | os._Environ, defaults: bool) -> N env_val = list_env_val if isinstance(list_env_val, list) else [env_val] except get_loader_exceptions(): env_val = [env_val] - cfg[action.dest] = self._check_value_key(action, env_val, action.dest, cfg) + with value_source_context(ValueSource("environment variable", env_var)): + cfg[action.dest] = self._check_value_key(action, env_val, action.dest, cfg) self._apply_actions(cfg) return cfg @@ -659,6 +696,7 @@ def parse_path( ext_vars=ext_vars, env=env, defaults=defaults, + _source=ValueSource("config file", fpath, self.parser_mode), **kwargs, ) @@ -689,12 +727,12 @@ def parse_string( Raises: ArgumentError: If the parsing fails and ``exit_on_error=False``. """ - skip_validation, fail_no_subcommand = get_private_kwargs( - kwargs, _skip_validation=False, _fail_no_subcommand=True + skip_validation, fail_no_subcommand, source = get_private_kwargs( + kwargs, _skip_validation=False, _fail_no_subcommand=True, _source=ValueSource("config string") ) try: - with parser_context(load_value_mode=self.parser_mode): + with parser_context(load_value_mode=self.parser_mode), value_source_context(source): cfg = self._load_config_parser_mode(content, path, ext_vars, previous_config.get()) if defaults or env: @@ -789,6 +827,7 @@ def dump( skip_validation: bool = False, with_comments: bool = False, skip_link_targets: bool = True, + **kwargs, ) -> str: """Generates a serialized string for the given configuration object. @@ -808,8 +847,13 @@ def dump( Raises: TypeError: If any of the values of namespace is invalid according to the parser. """ + with_provenance = get_private_kwargs(kwargs, _with_provenance=False) check_valid_dump_format(format) + provenance = None + if with_provenance: + provenance = get_provenance(subclasses_disabled_remove_class_path(namespace.clone())) + cfg = namespace.clone(with_meta=False) with parser_context(load_value_mode=self.parser_mode): @@ -832,7 +876,9 @@ def dump( self._dump_delete_default_entries(cfg_dict, defaults.as_dict()) with parser_context(parent_parser=self): - return dump_using_format(self, cfg_dict, dump_format=format, with_comments=with_comments) + return dump_using_format( + self, cfg_dict, dump_format=format, with_comments=with_comments, provenance=provenance + ) def _dump_cleanup_actions(self, cfg, actions, dump_kwargs, prefix=""): skip_unset = dump_kwargs["skip_unset"] @@ -1050,14 +1096,15 @@ def get_defaults(self, skip_validation: bool = False) -> Namespace: An object with all default values as attributes. """ cfg = Namespace() - for action in filter_non_parsing_actions(self._actions): - if ( - action.default != argparse.SUPPRESS - and action.dest != argparse.SUPPRESS - and not isinstance(action.default, UnknownDefault) - ): - default = recreate_branches(action.default) - cfg[action.dest] = default + with value_source_context(ValueSource("default")): + for action in filter_non_parsing_actions(self._actions): + if ( + action.default != argparse.SUPPRESS + and action.dest != argparse.SUPPRESS + and not isinstance(action.default, UnknownDefault) + ): + default = recreate_branches(action.default) + cfg[action.dest] = default self._logger.debug("Loaded parser defaults: %s", cfg) @@ -1071,7 +1118,9 @@ def get_defaults(self, skip_validation: bool = False) -> Namespace: default_config_file_content = default_config_file.read_text() if not default_config_file_content.strip(): continue - cfg_file = self._load_config_parser_mode(default_config_file_content, prev_cfg=cfg) + source = ValueSource("default config file", default_config_file, self.parser_mode) + with value_source_context(source): + cfg_file = self._load_config_parser_mode(default_config_file_content, prev_cfg=cfg) cfg = merge_config(self, cfg_file, cfg) try: with _ActionPrintConfig.skip_print_config(): @@ -1130,12 +1179,22 @@ def get_completion_script(self, completion_type: str, **kwargs) -> str: def error(self, message: str, ex: Exception | None = None) -> NoReturn: """Logs error message if a logger is set and exits or raises an :class:`ArgumentError`.""" + source = _get_error_source(ex) + if source is not None: + cache: dict = {} + message += f"\n Source: {describe_source(source, cache)}" + snippet = get_source_snippet(source, cache) + if snippet is not None: + message += f"\n {snippet}" + error = argument_error(message) + if source is not None: + error.value_source_reported = True # type: ignore[attr-defined] # so that it is not added again self._logger.error(message) if not self.exit_on_error: - raise argument_error(message) from ex + raise error from ex elif debug_mode_active(): self._logger.debug("Debug enabled, thus raising exception instead of exit.") - raise argument_error(message) from ex + raise error from ex parser = getattr(ex, "subcommand_parser", None) or self if getattr(ex, "default_config_file", None): @@ -1196,44 +1255,52 @@ def check_required(cfg, parser, prefix): def check_values(cfg): sorted_keys = {k: find_action(self, k) for k in get_non_meta_sorted_keys(cfg)} for key, action in sorted_keys.items(): - parent_action = None - if action is None: - if is_branch_key(self, key): - continue - parent_action, subcommand = find_parent_action_and_subcommand(self, key, exclude=_ActionConfigLoad) - if parent_action: - parent_key = subcommand + "." + parent_action.dest if subcommand else parent_action.dest - if key.startswith(parent_key + ".") and sorted_keys.get(parent_key) is parent_action: - # only check action once with entire value + try: + parent_action = None + if action is None: + if is_branch_key(self, key): continue - val = cfg[key] - if action is not None: - if (val is get_parsing_setting("unset_sentinel") and skip_unset) or lenient_check.get(): - continue - try: - self._check_value_key(action, val, key, ccfg) - except TypeError as ex: - if not (val == {} and ActionTypeHint.is_subclass_typehint(action)): + parent_action, subcommand = find_parent_action_and_subcommand( + self, key, exclude=_ActionConfigLoad + ) + if parent_action: + parent_key = subcommand + "." + parent_action.dest if subcommand else parent_action.dest + if key.startswith(parent_key + ".") and sorted_keys.get(parent_key) is parent_action: + # only check action once with entire value + continue + val = cfg[key] + if action is not None: + if (val is get_parsing_setting("unset_sentinel") and skip_unset) or lenient_check.get(): + continue + try: + self._check_value_key(action, val, key, ccfg) + except TypeError as ex: + if not (val == {} and ActionTypeHint.is_subclass_typehint(action)): + raise ex + else: + if self._accepts_extra_key(key): + continue + if isinstance(parent_action, ActionSubCommands) and "." in key: + subcommand, subkey = split_key_root(key) + ex = NSKeyError(f"Subcommand '{subcommand}' does not accept option '{subkey}'") + ex.subcommand_parser = parent_action._name_parser_map[subcommand] raise ex - else: - if self._accepts_extra_key(key): - continue - if isinstance(parent_action, ActionSubCommands) and "." in key: - subcommand, subkey = split_key_root(key) - ex = NSKeyError(f"Subcommand '{subcommand}' does not accept option '{subkey}'") - ex.subcommand_parser = parent_action._name_parser_map[subcommand] - raise ex - group_key = next((g for g in self.groups if key.startswith(g + ".")), None) - if group_key: - subkey = key[len(group_key) + 1 :] - raise NSKeyError(f"Group '{group_key}' does not accept option '{subkey}'") - if self._subcommands_action: - if cfg.get(self._subcommands_action.dest): - subcommand = f"'{cfg[self._subcommands_action.dest]}'" - else: - subcommand = f"{{{list(self._subcommands_action.choices)[0]},...}}" - raise NSKeyError(f"Option '{key}' is not accepted before subcommand {subcommand}") - raise NSKeyError(f"Option '{key}' is not accepted") + group_key = next((g for g in self.groups if key.startswith(g + ".")), None) + if group_key: + subkey = key[len(group_key) + 1 :] + raise NSKeyError(f"Group '{group_key}' does not accept option '{subkey}'") + if self._subcommands_action: + if cfg.get(self._subcommands_action.dest): + subcommand = f"'{cfg[self._subcommands_action.dest]}'" + else: + subcommand = f"{{{list(self._subcommands_action.choices)[0]},...}}" + raise NSKeyError(f"Option '{key}' is not accepted before subcommand {subcommand}") + raise NSKeyError(f"Option '{key}' is not accepted") + except (TypeError, KeyError) as ex: + source = get_provenance(cfg).get(key) + if source is not None and not hasattr(ex, "value_source"): + ex.value_source = source + raise with parser_context(load_value_mode=self.parser_mode): check_values(cfg) @@ -1332,6 +1399,10 @@ def _apply_actions( skip_fn: Callable[[Any], bool] | None = None, ) -> Namespace: """Runs _check_value_key on actions present in config.""" + # A source being applied is only set to raw values, i.e. the ones given in dicts. Values + # already in namespaces were parsed before, so processing them keeps their sources. + source = value_source.get() + is_raw = source is not None and isinstance(cfg, dict) if isinstance(cfg, dict): cfg = Namespace(cfg) if parent_key: @@ -1341,6 +1412,7 @@ def _apply_actions( keys = [parent_key + "." + k for k in cfg_branch.keys(branches=True, nested=False)] else: keys = list(cfg.keys(branches=True, nested=False)) + raw_keys = set(keys) if is_raw else set() if prev_cfg: prev_cfg = prev_cfg.clone() @@ -1349,62 +1421,69 @@ def _apply_actions( config_keys: set[str] = set() num = 0 - while num < len(keys): - key = keys[num] - exclude = _ActionConfigLoad if key in config_keys else None - action, subcommand = find_action_and_subcommand(self, key, exclude=exclude) - - if isinstance(action, ActionJsonnet): - ext_vars_key = action._ext_vars - if ext_vars_key and ext_vars_key not in keys[:num]: - keys = keys[:num] + [ext_vars_key] + [k for k in keys[num:] if k != ext_vars_key] - continue + with value_source_context(source): + while num < len(keys): + key = keys[num] + exclude = _ActionConfigLoad if key in config_keys else None + action, subcommand = find_action_and_subcommand(self, key, exclude=exclude) + + if isinstance(action, ActionJsonnet): + ext_vars_key = action._ext_vars + if ext_vars_key and ext_vars_key not in keys[:num]: + keys = keys[:num] + [ext_vars_key] + [k for k in keys[num:] if k != ext_vars_key] + continue - num += 1 + num += 1 + if source is not None: + config_key = key[len(parent_key) + 1 :] if parent_key else key + raw_source = source if source.mode is None else source._replace(key=config_key) + value_source.set(raw_source if key in raw_keys else None) - if action is None and key.rsplit(".", 1)[-1] == config_schema_key: - cfg.pop(key) # only meant for editors, see completion type jsonschema - continue + if action is None and key.rsplit(".", 1)[-1] == config_schema_key: + cfg.pop(key) # only meant for editors, see completion type jsonschema + continue - if action is None or isinstance(action, ActionSubCommands): - value = cfg[key] - if action is None and self._accepts_extra_key(key): - continue # unknown key of a pydantic model, its value is given as is to the model - if isinstance(value, dict): - value = Namespace(value) - if isinstance(value, Namespace): - new_keys = value.keys(branches=True, nested=False) - keys += [key + "." + k for k in new_keys if key + "." + k not in keys] - cfg[key] = value - continue + if action is None or isinstance(action, ActionSubCommands): + value = cfg[key] + if action is None and self._accepts_extra_key(key): + continue # unknown key of a pydantic model, its value is given as is to the model + if isinstance(value, dict): + value = Namespace(value) + if key in raw_keys: + raw_keys.update(f"{key}.{k}" for k in value.keys(branches=True, nested=False)) + if isinstance(value, Namespace): + new_keys = value.keys(branches=True, nested=False) + keys += [key + "." + k for k in new_keys if key + "." + k not in keys] + cfg[key] = value + continue - action_dest = action.dest if subcommand is None else subcommand + "." + action.dest - append = False - if action_dest not in cfg and key.endswith("+"): - append = True - cfg[action_dest] = cfg.pop(key) - elif action_dest != key and key in cfg: - # the key is an alias of the action's dest, i.e. another accepted name for it - cfg[action_dest] = cfg.pop(key) - value = cfg[action_dest] - if skip_fn and skip_fn(value): - continue - with parser_context(parent_parser=self, lenient_check=True): - value = self._check_value_key(action, value, action_dest, prev_cfg, append=append) - if isinstance(action, _ActionConfigLoad): - value = action.resolve_subclass_spec(value) - config_keys.add(action_dest) - keys.append(action_dest) - elif isinstance(action, ActionConfigFile): - if isinstance(value, str): - cfg.pop(action_dest) - preserve = Namespace({k: cfg[k] for k in keys[num:]}) - ActionConfigFile.apply_config(self, cfg, action_dest, value) - cfg.update(preserve) + action_dest = action.dest if subcommand is None else subcommand + "." + action.dest + append = False + if action_dest not in cfg and key.endswith("+"): + append = True + cfg[action_dest] = cfg.pop(key) + elif action_dest != key and key in cfg: + # the key is an alias of the action's dest, i.e. another accepted name for it + cfg[action_dest] = cfg.pop(key) + value = cfg[action_dest] + if skip_fn and skip_fn(value): continue - elif getattr(action, "jsonnet_ext_vars", False): - prev_cfg[action_dest] = value - cfg[action_dest] = value + with parser_context(parent_parser=self, lenient_check=True): + value = self._check_value_key(action, value, action_dest, prev_cfg, append=append) + if isinstance(action, _ActionConfigLoad): + value = action.resolve_subclass_spec(value) + config_keys.add(action_dest) + keys.append(action_dest) + elif isinstance(action, ActionConfigFile): + if isinstance(value, str): + cfg.pop(action_dest) + preserve = Namespace({k: cfg[k] for k in keys[num:]}) + ActionConfigFile.apply_config(self, cfg, action_dest, value) + cfg.update(preserve) + continue + elif getattr(action, "jsonnet_ext_vars", False): + prev_cfg[action_dest] = value + cfg[action_dest] = value return cfg[parent_key] if parent_key else cfg def _check_value_key( diff --git a/jsonargparse/_formatters.py b/jsonargparse/_formatters.py index 11ed703f..e8925107 100644 --- a/jsonargparse/_formatters.py +++ b/jsonargparse/_formatters.py @@ -31,7 +31,7 @@ ) from ._link_arguments import ActionLink from ._namespace import Namespace -from ._optionals import import_ruamel +from ._optionals import import_pyyaml, import_ruamel from ._subcommands import ActionSubCommands, find_action from ._type_checking import ArgumentParser, ruamelCommentedMap from ._typehints import ( @@ -134,8 +134,8 @@ class YAMLCommentFormatter: def __init__(self, help_formatter: HelpFormatter): self.help_formatter = help_formatter - def add_yaml_comments(self, cfg: str) -> str: - """Adds help text as yaml comments.""" + def add_yaml_comments(self, cfg: str, with_help: bool = True, provenance: dict | None = None) -> str: + """Adds help text and/or provenance as yaml comments.""" from ._core import ArgumentParser ruyaml = import_ruamel("add_yaml_comments") @@ -145,9 +145,12 @@ def add_yaml_comments(self, cfg: str) -> str: parser = parent_parser.get() assert isinstance(parser, ArgumentParser) if isinstance(cfg, dict): - if parser.description is not None: - self.set_yaml_start_comment(parser.description, cfg) - self.set_comments(cfg, parser, get_group_titles(parser)) + if with_help: + if parser.description is not None: + self.set_yaml_start_comment(parser.description, cfg) + self.set_comments(cfg, parser, get_group_titles(parser)) + if provenance: + _set_provenance_comments(cfg, provenance, key_lines={}) out = StringIO() yaml.dump(cfg, out) return out.getvalue() @@ -311,6 +314,104 @@ def set_yaml_argument_comment( cfg.yaml_set_comment_before_after_key(key, before="\n" + text, indent=2 * depth) +def _set_provenance_comments(cfg, provenance: dict, key_lines: dict, prefix: str = "", depth: int = 0) -> None: + """Adds to each value of a ruamel.yaml object a comment saying where it came from.""" + for key, value in cfg.items(): + full_key = prefix + key + source = provenance.get(full_key) + if source is not None: + text = describe_source(source, key_lines) + if isinstance(value, str) and "\n" in value: + # an end of line comment would be placed after the lines of the string + cfg.yaml_set_comment_before_after_key(key, before=text, indent=2 * depth) + else: + cfg.yaml_add_eol_comment(text, key, column=0) + if isinstance(value, dict): + _set_provenance_comments(value, provenance, key_lines, full_key + ".", depth + 1) + elif isinstance(value, list): + for num, item in enumerate(value): + if isinstance(item, dict): + _set_provenance_comments(item, provenance, key_lines, f"{full_key}[{num}].", depth + 1) + + +line_number_modes = {"yaml", "omegaconf", "omegaconf+"} +snippet_width = 80 + + +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.""" + 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]}" + + +def get_source_snippet(source, cache: dict) -> str | None: + """Returns the line of a config file where a value came from, shortened around the key if long.""" + position = _find_position(source, cache) + if position is None: + return None + line, column, text = position + if len(text) > snippet_width: # e.g. a compact single line json + start = max(0, column - 10) + end = start + snippet_width + text = ("..." if start > 0 else "") + text[start:end] + ("..." if end < len(text) else "") + return f"{line} | {text}" + + +def _describe_origin(origin) -> str: + """For config files, the path relative to the working directory if inside it, so that it can be opened.""" + import pathlib + + from ._paths import Path, get_initial_working_directory + + if not isinstance(origin, Path) or origin.is_url or origin.is_fsspec: + return str(origin) + absolute = pathlib.Path(origin.absolute) + cwd = pathlib.Path(get_initial_working_directory()) + return str(absolute.relative_to(cwd) if absolute.is_relative_to(cwd) else absolute) + + +def _find_position(source, cache: dict) -> tuple[int, int, str] | None: + """Returns the line number, column and line of the config file where a value came from, if known.""" + if source.mode not in line_number_modes or source.key is None: + return None + if id(source.origin) not in cache: # each config file is read only once + content = source.origin.read_text() + cache[id(source.origin)] = (_get_config_key_positions(content), content.splitlines()) + positions, lines = cache[id(source.origin)] + key = source.key + while key: + for candidate in [key, ("." + key).replace(".init_args.", ".")[1:]]: # init_args can be implicit + if candidate in positions: + line, column = positions[candidate] + return line, column, lines[line - 1].rstrip() + cut = max(key.rfind("."), key.rfind("[")) # the parent, e.g. "a" for "a.b" or "a[0]" + key = key[:cut] if cut > 0 else "" + return None + + +def _get_config_key_positions(content: str) -> dict[str, tuple[int, int]]: + """Returns the line number and column of each key in a yaml config, composed with the loader used for parsing.""" + from ._loaders_dumpers import get_yaml_default_loader + + yaml = import_pyyaml("_get_config_key_positions") + positions: dict[str, tuple[int, int]] = {} + + def collect(node, key): + if isinstance(node, yaml.MappingNode): + children = [(f"{key}.{k.value}" if key else k.value, k, v) for k, v in node.value] + elif isinstance(node, yaml.SequenceNode): + children = [(f"{key}[{num}]", item, item) for num, item in enumerate(node.value)] + else: + return + for child_key, position_node, value_node in children: # for duplicate keys the last is kept, as when loading + positions[child_key] = (position_node.start_mark.line + 1, position_node.start_mark.column) + collect(value_node, child_key) + + collect(yaml.compose(content, Loader=get_yaml_default_loader()), "") + return positions + + class DefaultHelpFormatter(HelpFormatter): """Help message formatter that includes types, default values and env var names. diff --git a/jsonargparse/_loaders_dumpers.py b/jsonargparse/_loaders_dumpers.py index d23f83c7..261dead3 100644 --- a/jsonargparse/_loaders_dumpers.py +++ b/jsonargparse/_loaders_dumpers.py @@ -273,11 +273,11 @@ def yaml_dump(data): return yaml.dump(data, Dumper=get_yaml_default_dumper(), **dump_yaml_kwargs) -def yaml_comments_dump(data, parser): +def yaml_comments_dump(data, parser, with_help=True, provenance=None): dump = dumpers["yaml"](data) formatter_class = create_help_formatter_with_comments(parser.formatter_class) formatter = formatter_class(parser.prog) - return formatter.add_yaml_comments(dump) + return formatter.add_yaml_comments(dump, with_help, provenance) def json_compact_dump(data): @@ -321,19 +321,28 @@ def check_valid_dump_format(dump_format: str): raise ValueError(f'Unknown output format "{dump_format}".') -def dump_using_format(parser: ArgumentParser, data: dict, dump_format: str, with_comments: bool = False) -> str: +def dump_using_format( + parser: ArgumentParser, + data: dict, + dump_format: str, + with_comments: bool = False, + provenance: dict | None = None, +) -> str: if dump_format == "parser_mode": default_format = "yaml" if pyyaml_available else "json" dump_format = parser.parser_mode if parser.parser_mode in dumpers else default_format - if with_comments: + if with_comments or provenance is not None: if f"{dump_format}_comments" not in dumpers: if dump_format == "yaml": raise ValueError("ruamel.yaml is required for dumping YAML with comments.") raise ValueError(f"Dumping with comments is not supported for format '{dump_format}'.") dump_format = f"{dump_format}_comments" data = replace_unset(data) - args = (data, parser) if dump_format.endswith("_comments") else (data,) - dump = dumpers[dump_format](*args) + if dump_format.endswith("_comments"): + # help comments are also added when a comments format is given directly + dump = dumpers[dump_format](data, parser, with_comments or provenance is None, provenance) + else: + dump = dumpers[dump_format](data) if parser.dump_header and comment_prefix.get(dump_format): prefix = comment_prefix[dump_format] header = "\n".join(prefix + line for line in parser.dump_header) @@ -412,8 +421,8 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self._yaml_formatter = YAMLCommentFormatter(self) - def add_yaml_comments(self, cfg: str) -> str: - """Adds help text as yaml comments.""" - return self._yaml_formatter.add_yaml_comments(cfg) + def add_yaml_comments(self, cfg: str, with_help: bool = True, provenance: dict | None = None) -> str: + """Adds help text and/or provenance as yaml comments.""" + return self._yaml_formatter.add_yaml_comments(cfg, with_help, provenance) return DynamicHelpFormatter diff --git a/jsonargparse/_namespace.py b/jsonargparse/_namespace.py index 1c6e1df1..cdae856a 100644 --- a/jsonargparse/_namespace.py +++ b/jsonargparse/_namespace.py @@ -3,13 +3,47 @@ import argparse from collections import OrderedDict from collections.abc import Iterator -from typing import Any +from contextlib import contextmanager +from contextvars import ContextVar +from typing import Any, NamedTuple __all__ = ["Namespace"] subclasses_disabled_meta_key = "__subclasses_disabled__" -meta_keys = {"__default_config__", "__path__", "__orig__", subclasses_disabled_meta_key} +provenance_key = "__provenance__" +meta_keys = {"__default_config__", "__path__", "__orig__", subclasses_disabled_meta_key, provenance_key} + + +class ValueSource(NamedTuple): + """Where a parsed value came from.""" + + description: str + 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 + + def __deepcopy__(self, memo): + return self # immutable, so copies of namespaces can share it, which is much faster, e.g. for list items + + +# 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) + + +@contextmanager +def value_source_context(source: ValueSource | None) -> Iterator[None]: + token = value_source.set(source) + try: + yield + except Exception as ex: + current = value_source.get() + if current is not None and not hasattr(ex, "value_source"): + ex.value_source = current # type: ignore[attr-defined] # for error messages to say where a value came from + raise + finally: + value_source.reset(token) class NSKeyError(KeyError): @@ -35,20 +69,33 @@ def is_meta_key(key: str) -> bool: def recreate_branches(data, skip_keys=None): + token = value_source.set(None) # copying does not change where the values came from + try: + return _recreate_branches(data, skip_keys) + finally: + value_source.reset(token) + + +def _recreate_branches(data, skip_keys): new_data = data if isinstance(data, (Namespace, dict)) and not isinstance(data, OrderedDict): new_data = type(data)() for key, val in getattr(data, "__dict__", data).items(): if skip_keys is None or key not in skip_keys: - new_data[key] = recreate_branches(val, skip_keys) + new_data[key] = _recreate_branches(val, skip_keys) + provenance = getattr(data, provenance_key, None) + if provenance and skip_keys is None: + object.__setattr__(new_data, provenance_key, dict(provenance)) elif isinstance(data, list): - new_data = [recreate_branches(v, skip_keys) for v in data] + new_data = [_recreate_branches(v, skip_keys) for v in data] return new_data class Namespace(argparse.Namespace): """Extension of argparse's Namespace to support nesting and subscript access.""" + __slots__ = (provenance_key,) # stored outside __dict__, so that it is not seen as a value + def __init__(self, *args, **kwargs): """Initializer for Namespace instance. @@ -131,7 +178,13 @@ def __setattr__(self, name: str, value: Any) -> None: if "." in name: self.__setitem__(name, value) else: - super().__setattr__(add_clash_mark(name), value) + name = add_clash_mark(name) + super().__setattr__(name, value) + source = value_source.get() + if source is not None and name not in meta_keys: + if not isinstance(value, Namespace): + _get_provenance_dict(self)[name] = source + fill_provenance(value, source) def __setitem__(self, key: str, item: Any) -> None: """Sets an item to a possibly nested namespace.""" @@ -227,9 +280,11 @@ def update(self, value: "Namespace | Any", key: str | None = None, only_unset: b if key and not isinstance(self.get(key), Namespace): self[key] = Namespace() prefix = key + "." if key else "" + provenance = get_provenance(value, items=False) # items in lists keep their own sources 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())) return self def get(self, key: str, default: Any = None) -> Any: @@ -247,7 +302,7 @@ def pop(self, key: str, default: Any = None) -> Any: return parent_ns.__dict__.pop(leaf_key, default) -clash_names: set[str] = set(dir(Namespace)) +clash_names: set[str] = set(dir(Namespace)) - {provenance_key} clash_mark = "\u200b" @@ -301,3 +356,90 @@ def get_non_meta_sorted_keys(namespace: Namespace) -> list[str]: def get_value_and_parent(namespace: Namespace, key: str) -> tuple[Any, Namespace, str]: leaf_key, parent_ns, _ = namespace._parse_required_key(key) return parent_ns[leaf_key], parent_ns, leaf_key + + +def _get_provenance_dict(namespace: Namespace) -> dict[str, ValueSource]: + provenance = getattr(namespace, provenance_key, None) + if provenance is None: + provenance = {} + object.__setattr__(namespace, provenance_key, provenance) + return provenance + + +def _set_source(namespace: Namespace, key: str, source: ValueSource | None) -> None: + leaf_key, parent_ns, _ = namespace._parse_key(key) + if source is None: + (getattr(parent_ns, provenance_key, None) or {}).pop(leaf_key, None) + elif isinstance(parent_ns, Namespace): + _get_provenance_dict(parent_ns)[leaf_key] = source + + +def get_provenance(namespace: Namespace, items: bool = True) -> dict[str, ValueSource]: + """Returns the sources of the leaf values of a namespace, as a flat dict. + + Args: + namespace: The namespace from which to get the sources. + items: Whether to include the namespaces in lists and dicts, e.g. ``opts[0].class_path``, instead of the + source of the list or dict as a whole. + """ + provenance: dict[str, ValueSource] = {} + _collect_provenance(namespace, "", provenance, items) + return provenance + + +def _collect_provenance(value: Any, key: str, provenance: dict[str, ValueSource], items: bool) -> None: + if isinstance(value, Namespace): + own = getattr(value, provenance_key, None) or {} + for name, item in vars(value).items(): + if name in meta_keys: + continue + item_key = f"{key}.{del_clash_mark(name)}" if key else del_clash_mark(name) + count = len(provenance) + _collect_provenance(item, item_key, provenance, items) + if name in own and not isinstance(item, Namespace) and len(provenance) == count: + provenance[item_key] = own[name] + elif items and isinstance(value, list): + for num, item in enumerate(value): + _collect_provenance(item, f"{key}[{num}]", provenance, items) + elif items and isinstance(value, dict): + for name, item in value.items(): + _collect_provenance(item, f"{key}.{name}", provenance, items) + + +def fill_provenance(value: Any, source: ValueSource) -> None: + """Sets a source to the leaf values that don't have one yet, of a namespace or of namespaces in lists and dicts.""" + if isinstance(value, Namespace): + provenance = _get_provenance_dict(value) + for name, item in vars(value).items(): + if name in meta_keys: + continue + item_source = _child_source(source, "." + del_clash_mark(name)) + if not isinstance(item, Namespace) and name not in provenance: + provenance[name] = 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(): + if isinstance(item, (Namespace, list, dict)): + fill_provenance(item, _child_source(source, f"[{name}]" if isinstance(value, list) else f".{name}")) + + +def copy_provenance(source: Namespace, target: Namespace, key: str | None = None) -> None: + """Copies the sources of the leaf values of a namespace to the same leaves of another one.""" + source_provenance = getattr(source, provenance_key, None) or {} + target_provenance = _get_provenance_dict(target) + for name, value in vars(source).items(): + if key is not None and name != key: + continue + target_value = target.__dict__.get(name) + if isinstance(value, Namespace): + if isinstance(target_value, Namespace) and target_value is not value: + copy_provenance(value, target_value) + elif name in source_provenance and name in target.__dict__: + target_provenance[name] = source_provenance[name] + + +def _child_source(source: ValueSource, suffix: str) -> ValueSource: + """The source narrowed to a child key, e.g. suffix ".name" or "[0]", only needed for config file line numbers.""" + if source.mode is None: + return source + return source._replace(key=((source.key or "") + suffix).lstrip(".")) diff --git a/jsonargparse/_optionals.py b/jsonargparse/_optionals.py index d4226d67..cf4e7ddd 100644 --- a/jsonargparse/_optionals.py +++ b/jsonargparse/_optionals.py @@ -347,12 +347,15 @@ def omegaconf_apply(parser, cfg): from omegaconf import OmegaConf from ._common import parser_context + from ._namespace import copy_provenance with parser_context(path_dump_preserve_relative=True): cfg_dict = parser.dump(cfg, skip_validation=True, skip_unset=False, skip_link_targets=False) cfg_omegaconf = OmegaConf.create(cfg_dict) cfg_dict = OmegaConf.to_container(cfg_omegaconf, resolve=True) - return parser._apply_actions(cfg_dict) + cfg_resolved = parser._apply_actions(cfg_dict) + copy_provenance(cfg, cfg_resolved) # an interpolated value comes from where the interpolation is + return cfg_resolved def omegaconf_tokenize(path: str) -> list[str]: diff --git a/jsonargparse/_paths.py b/jsonargparse/_paths.py index faabc9cc..4c3cd0be 100644 --- a/jsonargparse/_paths.py +++ b/jsonargparse/_paths.py @@ -18,6 +18,7 @@ ) _current_path_dir: ContextVar[str | None] = ContextVar("_current_path_dir", default=None) +_initial_cwd: ContextVar[str | None] = ContextVar("_initial_cwd", default=None) class _CachedStdin(StringIO): @@ -365,8 +366,10 @@ def change_to_path_dir(path: Path | str | None) -> Iterator[str | None]: path_dir = scheme + path_dir token = _current_path_dir.set(path_dir) + initial_cwd_token = None if chdir and path_dir: chdir = os.getcwd() + initial_cwd_token = _initial_cwd.set(_initial_cwd.get() or chdir) path_dir = os.path.abspath(path_dir) os.chdir(path_dir) @@ -376,3 +379,10 @@ def change_to_path_dir(path: Path | str | None) -> Iterator[str | None]: _current_path_dir.reset(token) if chdir: os.chdir(chdir) + if initial_cwd_token is not None: + _initial_cwd.reset(initial_cwd_token) + + +def get_initial_working_directory() -> str: + """Returns the working directory from before changing to the directories of config files.""" + return _initial_cwd.get() or os.getcwd() diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py index 929e3a42..22b2b59d 100644 --- a/jsonargparse/_typehints.py +++ b/jsonargparse/_typehints.py @@ -59,6 +59,7 @@ from ._common import ( ImportDenied, check_import_path, + command_line_source, config_schema_key, get_generic_origin, get_parsing_setting, @@ -77,11 +78,19 @@ from ._instantiation import dynamic_class_instantiator from ._loaders_dumpers import ( basic_json_or_yaml_load, + get_load_value_mode, get_loader_exceptions, json_or_yaml_loader_exceptions, load_value, ) -from ._namespace import Namespace, subclasses_disabled_meta_key +from ._namespace import ( + Namespace, + ValueSource, + copy_provenance, + fill_provenance, + subclasses_disabled_meta_key, + value_source_context, +) from ._optionals import ( capture_typing_extension_shadows, get_alias_target, @@ -692,26 +701,29 @@ def __call__(self, *args, **kwargs): raise ValueError("ActionTypeHint does not allow nargs=0.") return ActionTypeHint(**kwargs) parser, cfg, val, opt_str = args - if not (self.nargs == "?" and val is None): - # the option string can be an alias of the dest, i.e. another accepted name for it - option = self.get_option_string_base(opt_str) - if option: - if opt_str.startswith(f"{option}.init_args."): - sub_opt = opt_str[len(f"{option}.init_args.") :] - else: - sub_opt = opt_str[len(f"{option}.") :] - val = NestedArg(key=sub_opt, val=val) - append = isinstance(opt_str, str) and opt_str.endswith("+") and opt_str[:-1] in self.option_strings - val = self._check_type_(val, append=append, cfg=cfg, mode=parser.parser_mode) - if is_subclass_spec(val): - prev_val = cfg.get(self.dest) - if is_subclass_spec(prev_val) and "init_args" in prev_val: - ActionTypeHint.discard_init_args_on_class_path_change( - self, - prev_val.init_args, - val.get("init_args"), - ) - cfg.update(val, self.dest) + source = command_line_source(self, opt_str) + # a nested parse of the value, e.g. of an init arg of a subclass, refers to the same option + with parser_context(command_line_option=source.origin), value_source_context(source): + if not (self.nargs == "?" and val is None): + # the option string can be an alias of the dest, i.e. another accepted name for it + option = self.get_option_string_base(opt_str) + if option: + if opt_str.startswith(f"{option}.init_args."): + sub_opt = opt_str[len(f"{option}.init_args.") :] + else: + sub_opt = opt_str[len(f"{option}.") :] + val = NestedArg(key=sub_opt, val=val) + append = isinstance(opt_str, str) and opt_str.endswith("+") and opt_str[:-1] in self.option_strings + val = self._check_type_(val, append=append, cfg=cfg, mode=parser.parser_mode) + if is_subclass_spec(val): + prev_val = cfg.get(self.dest) + if is_subclass_spec(prev_val) and "init_args" in prev_val: + ActionTypeHint.discard_init_args_on_class_path_change( + self, + prev_val.init_args, + val.get("init_args"), + ) + cfg.update(val, self.dest) return None def get_option_string_base(self, opt_str) -> str | None: @@ -774,6 +786,8 @@ def _check_type(self, value, append=False, cfg=None, mode=None): if ex: raise ex + if config_path is not None: + fill_provenance(val, ValueSource("config file", config_path, mode)) if isinstance(val, (Namespace, dict)): if path_meta is not None: val["__path__"] = path_meta @@ -919,6 +933,7 @@ def adapt_subconfig_path(val, typehint, adapt_kwargs): raise_unexpected_value(f"Invalid content in sub-config file {val}: {ex}", exception=ex) with load_config_path_context(path), change_to_path_dir(path): val = adapt_typehints(subconfig, typehint, **adapt_kwargs) + fill_provenance(val, ValueSource("config file", path, get_load_value_mode())) if isinstance(val, (Namespace, dict)): val["__path__"] = path return val @@ -1575,11 +1590,14 @@ def adapt_typehints( elif not isinstance(val, list): raise_unexpected_value(f"Expected a {typehint_origin}", val) if subtypehints is not None: + # orig_val is only read and prev_val is replaced, so they are not copied, which per item is quadratic + shared = {k: adapt_kwargs[k] for k in ("orig_val", "prev_val") if k in adapt_kwargs} + copied = deepcopy({k: v for k, v in adapt_kwargs.items() if k not in shared}) for n, v in enumerate(val): if isinstance(prev_val, list) and len(prev_val) == len(val): - adapt_kwargs_n = {**deepcopy(adapt_kwargs), "prev_val": prev_val[n]} + adapt_kwargs_n = {**deepcopy(copied), **shared, "prev_val": prev_val[n]} else: - adapt_kwargs_n = deepcopy(adapt_kwargs) + adapt_kwargs_n = {**deepcopy(copied), **shared} with change_to_path_dir(list_path): val[n] = adapt_typehints(v, subtypehints[0], **adapt_kwargs_n) if typehint_origin is deque: @@ -2197,6 +2215,8 @@ def subclass_spec_as_namespace(val, prev_val=None): val["class_path"] = prev_val["class_path"] else: val = Namespace(class_path=prev_val["class_path"], init_args=val) + if isinstance(prev_val, Namespace): + copy_provenance(prev_val, val, key="class_path") return val @@ -2602,6 +2622,8 @@ def subclasses_disabled_remove_class_path(value): if value.pop(subclasses_disabled_meta_key, False): init_args = Namespace({**value.get("init_args", {}), **value.get("dict_kwargs", {})}) + if isinstance(value.get("init_args"), Namespace): + copy_provenance(value["init_args"], init_args) if "__path__" in value: # the value came from a sub-config file init_args["__path__"] = value["__path__"] return init_args diff --git a/jsonargparse/_util.py b/jsonargparse/_util.py index 43393ce5..b5dec5ea 100644 --- a/jsonargparse/_util.py +++ b/jsonargparse/_util.py @@ -27,7 +27,7 @@ parser_context, ) from ._loaders_dumpers import json_compact_dump, load_value -from ._namespace import Namespace +from ._namespace import Namespace, value_source_context from ._optionals import _get_config_read_mode from ._paths import Path from ._type_checking import ArgumentParser @@ -65,11 +65,12 @@ def merge_config(parser, source: Namespace, target: Namespace) -> Namespace: """ from ._typehints import ActionTypeHint - source = source.clone() - target = target.clone() - with parser_context(parent_parser=parser): - ActionTypeHint.discard_init_args_on_class_path_change(parser, target, source) - target.update(source) + with value_source_context(None): # merging does not change where the values came from + source = source.clone() + target = target.clone() + with parser_context(parent_parser=parser): + ActionTypeHint.discard_init_args_on_class_path_change(parser, target, source) + target.update(source) return target diff --git a/jsonargparse_tests/test_optionals.py b/jsonargparse_tests/test_optionals.py index 53f15c63..b33c2424 100644 --- a/jsonargparse_tests/test_optionals.py +++ b/jsonargparse_tests/test_optionals.py @@ -1,6 +1,7 @@ from __future__ import annotations import sys +from unittest.mock import patch import pytest @@ -161,12 +162,12 @@ def test_ruamel_support_false(): ctx.match("test_ruamel_support_false") -@pytest.mark.skipif(ruamel_support, reason="ruamel.yaml package should not be installed") -def test_print_config_comments_unavailable(print_parser): - help_str = get_parser_help(print_parser) - assert "comments," not in help_str - with pytest.raises(ArgumentError, match='Invalid option "comments"'): - get_parse_args_stdout(print_parser, ["--print_config=comments"]) +@pytest.mark.parametrize("flag", ["comments", "provenance"]) +def test_print_config_flag_requires_ruamel(print_parser, flag): + with patch("jsonargparse._actions.ruamel_support", False): + assert f"{flag}," in get_parser_help(print_parser) + with pytest.raises(ArgumentError, match=f'"{flag}" requires the ruamel.yaml package'): + get_parse_args_stdout(print_parser, [f"--print_config={flag}"]) # config read mode tests diff --git a/jsonargparse_tests/test_provenance.py b/jsonargparse_tests/test_provenance.py new file mode 100644 index 00000000..0084e5c3 --- /dev/null +++ b/jsonargparse_tests/test_provenance.py @@ -0,0 +1,837 @@ +from __future__ import annotations + +import copy +import dataclasses +import json +import os +import pickle +from pathlib import Path +from typing import Optional +from unittest.mock import patch + +import pytest + +from jsonargparse import ArgumentError, ArgumentParser, Namespace, set_loader, set_parsing_settings +from jsonargparse._formatters import describe_source +from jsonargparse._namespace import get_provenance +from jsonargparse._optionals import omegaconf_support, pyyaml_available, ruamel_support +from jsonargparse_tests.conftest import ( + get_parse_args_stderr, + get_parse_args_stdout, + get_parser_help, + skip_if_no_pyyaml, +) + +skip_if_no_ruamel = pytest.mark.skipif( + not (ruamel_support and pyyaml_available), + reason="ruamel.yaml and PyYAML packages are required", +) + + +@dataclasses.dataclass +class Encoder: + layers: int = 2 + dropout: float = 0.0 + + +@dataclasses.dataclass +class Model: + lr: float = 0.1 + encoder: Encoder = dataclasses.field(default_factory=Encoder) + + +class Optimizer: + def __init__(self, lr: float = 0.1): + self.lr = lr # pragma: no cover + + +class SGD(Optimizer): + def __init__(self, lr: float = 0.2, momentum: float = 0.9): # pragma: no cover + super().__init__(lr) + self.momentum = momentum + + +class Trainer: + def __init__(self, opts: Optional[list[Optimizer]] = None): + self.opts = opts # pragma: no cover + + +sgd = f"{__name__}.SGD" +optimizer = f"{__name__}.Optimizer" + + +@pytest.fixture +def parser() -> ArgumentParser: + """Parser in json mode, for which there are no line numbers, so that extras are not required.""" + parser = ArgumentParser(exit_on_error=False, parser_mode="json", env_prefix="APP") + parser.add_argument("--config", action="config") + return parser + + +@pytest.fixture +def yaml_parser() -> ArgumentParser: + parser = ArgumentParser(exit_on_error=False, parser_mode="yaml", env_prefix="APP") + parser.add_argument("--config", action="config") + return parser + + +def get_sources(cfg: Namespace) -> dict[str, str]: + """Returns the description of where each value came from, except for config arguments. + + There is no public way to get the provenance, other than the provenance flag of print config, which requires + ruamel.yaml. The internals are used so that tracking the provenance is tested without extras. + """ + return { + key: describe_source(source) + for key, source in get_provenance(cfg).items() + if key.rsplit(".", 1)[-1] != "config" + } + + +# sources + + +def test_default(parser): + parser.add_argument("pos", nargs="?", default="p") + parser.add_argument("--val", type=int, default=3) + parser.add_argument("--name", type=str, default="x") + assert get_sources(parser.parse_args([])) == {"pos": "default", "val": "default", "name": "default"} + + +def test_command_line(parser): + parser.add_argument("pos", type=int) + parser.add_argument("--val", type=int, default=3) + parser.add_argument("--flag", action="store_true") + assert get_sources(parser.parse_args(["1", "--val=7", "--flag"])) == { + "pos": "command line argument pos", + "val": "command line argument --val", + "flag": "command line argument --flag", + } + + +def test_command_line_positional_left_out(parser): + parser.add_argument("pos", nargs="?", default="p") + parser.add_argument("--val", type=int, default=3) + assert get_sources(parser.parse_args(["--val=7"])) == {"pos": "default", "val": "command line argument --val"} + + +def test_optionals_as_positionals(parser, parsing_settings_patch): + set_parsing_settings(parse_optionals_as_positionals=True) + parser.add_argument("--val", type=int, default=3) + assert get_sources(parser.parse_args(["7"])) == {"val": "command line argument --val"} + + +def test_environment_variable(parser, monkeypatch): + parser.default_env = True + parser.add_argument("--val", type=int, default=3) + parser.add_argument("--model", type=Model) + monkeypatch.setenv("APP_VAL", "11") + monkeypatch.setenv("APP_MODEL__LR", "0.5") + assert get_sources(parser.parse_args([])) == { + "val": "environment variable APP_VAL", + "model.lr": "environment variable APP_MODEL__LR", + "model.encoder.layers": "default", + "model.encoder.dropout": "default", + } + + +def test_config_file(parser, tmp_cwd): + parser.add_argument("--val", type=int, default=3) + parser.add_argument("--other", type=int, default=3) + Path("cfg.json").write_text('{"other": 4, "val": 5}') + assert get_sources(parser.parse_args(["--config=cfg.json"])) == { + "val": "config file cfg.json", + "other": "config file cfg.json", + } + + +def test_config_string(parser): + parser.add_argument("--val", type=int, default=3) + assert get_sources(parser.parse_args(['--config={"val": 5}'])) == {"val": "config string"} + + +def test_default_config_file(parser, tmp_cwd): + parser.add_argument("--val", type=int, default=3) + Path("defaults.json").write_text('{"val": 9}') + parser.default_config_files = ["defaults.json"] + assert get_sources(parser.parse_args([])) == {"val": "default config file defaults.json"} + + +def test_config_file_inside_config_file(parser, tmp_cwd): + parser.add_argument("--a", type=int, default=0) + parser.add_argument("--b", type=int, default=0) + Path("inner.json").write_text('{"b": 2}') + Path("outer.json").write_text('{"a": 1, "config": "inner.json"}') + assert get_sources(parser.parse_args(["--config=outer.json"])) == { + "a": "config file outer.json", + "b": "config file inner.json", + } + + +def test_custom_loader_config_file_without_line_numbers(tmp_cwd): + with patch.dict("jsonargparse._loaders_dumpers.loaders"): + set_loader("provenance_custom", json.loads) + parser = ArgumentParser(exit_on_error=False, parser_mode="provenance_custom") + parser.add_argument("--config", action="config") + parser.add_argument("--val", type=int, default=0) + Path("cfg.custom").write_text('{"val": 2}') + assert get_sources(parser.parse_args(["--config=cfg.custom"])) == {"val": "config file cfg.custom"} + + +def test_subconfig_file_path_relative_to_working_directory(parser, tmp_cwd): + parser.add_argument("--model", type=Model, sub_configs=True) + Path("cfgs").mkdir() + Path("cfgs", "model.json").write_text('{"lr": 0.001}') + Path("cfgs", "main.json").write_text('{"model": "model.json"}') + sources = get_sources(parser.parse_args(["--config=" + str(Path("cfgs", "main.json"))])) + assert sources["model.lr"] == f"config file {Path('cfgs', 'model.json')}" + + +def test_config_file_path_outside_working_directory(parser, tmp_path, monkeypatch): + defaults = tmp_path / "defaults.json" + defaults.write_text('{"val": 9}') + (tmp_path / "work").mkdir() + monkeypatch.chdir(tmp_path / "work") + parser.add_argument("--val", type=int, default=3) + parser.default_config_files = [str(defaults)] + assert get_sources(parser.parse_args([])) == {"val": f"default config file {defaults}"} + + +# precedence between sources + + +def test_precedence(parser, tmp_cwd, monkeypatch): + parser.default_env = True + for key in "abcde": + parser.add_argument(f"--{key}", type=int, default=0) + Path("defaults.json").write_text('{"b": 1, "c": 1, "d": 1, "e": 1}') + Path("cfg.json").write_text('{"d": 3, "e": 3}') + parser.default_config_files = ["defaults.json"] + monkeypatch.setenv("APP_C", "2") + assert get_sources(parser.parse_args(["--config=cfg.json", "--e=4"])) == { + "a": "default", + "b": "default config file defaults.json", + "c": "environment variable APP_C", + "d": "config file cfg.json", + "e": "command line argument --e", + } + + +def test_value_equal_to_default(parser, tmp_cwd): + parser.add_argument("--val", type=int, default=10) + Path("cfg.json").write_text('{"val": 10}') + assert get_sources(parser.parse_args(["--config=cfg.json"])) == {"val": "config file cfg.json"} + + +def test_later_config_file_overrides(parser, tmp_cwd): + parser.add_argument("--a", type=int, default=0) + parser.add_argument("--b", type=int, default=0) + Path("one.json").write_text('{"a": 1, "b": 1}') + Path("two.json").write_text('{"b": 2}') + assert get_sources(parser.parse_args(["--config=one.json", "--config=two.json"])) == { + "a": "config file one.json", + "b": "config file two.json", + } + + +def test_parse_args_namespace_keeps_provenance(parser, tmp_cwd): + parser.add_argument("--a", type=int, default=0) + parser.add_argument("--b", type=int, default=0) + Path("cfg.json").write_text('{"a": 1}') + cfg = parser.parse_args(["--config=cfg.json"]) + cfg = parser.parse_args(["--b=2"], namespace=cfg) + assert get_sources(cfg) == {"a": "config file cfg.json", "b": "command line argument --b"} + + +# nested values + + +def test_nested_dataclass(parser, tmp_cwd): + parser.add_argument("--model", type=Model) + Path("cfg.json").write_text('{"model": {"lr": 0.001, "encoder": {"layers": 12}}}') + assert get_sources(parser.parse_args(["--config=cfg.json", "--model.encoder.dropout=0.3"])) == { + "model.lr": "config file cfg.json", + "model.encoder.layers": "config file cfg.json", + "model.encoder.dropout": "command line argument --model.encoder.dropout", + } + + +def test_optional_dataclass(parser, tmp_cwd): + parser.add_argument("--encoder", type=Optional[Encoder], default=None) + Path("cfg.json").write_text('{"encoder": {"layers": 12}}') + assert get_sources(parser.parse_args(["--config=cfg.json"])) == { + "encoder.layers": "config file cfg.json", + "encoder.dropout": "default", + } + + +def test_class_group_subconfig_file(parser, tmp_cwd): + parser.add_argument("--model", type=Model, sub_configs=True) + Path("model.json").write_text('{"lr": 0.001, "encoder": {"layers": 12}}') + Path("cfg.json").write_text('{"model": "model.json"}') + expected = { + "model.lr": "config file model.json", + "model.encoder.layers": "config file model.json", + "model.encoder.dropout": "default", + } + assert get_sources(parser.parse_args(["--model=model.json"])) == expected + assert get_sources(parser.parse_args(["--config=cfg.json"])) == expected + + +def test_subclass_from_config_file(parser, tmp_cwd): + parser.add_argument("--opt", type=Optimizer) + Path("cfg.json").write_text(json.dumps({"opt": {"class_path": sgd, "init_args": {"momentum": 0.5}}})) + assert get_sources(parser.parse_args(["--config=cfg.json"])) == { + "opt.class_path": "config file cfg.json", + "opt.init_args.lr": "default", + "opt.init_args.momentum": "config file cfg.json", + } + + +def test_subclass_init_arg_from_command_line(parser, tmp_cwd): + parser.add_argument("--opt", type=Optimizer) + Path("cfg.json").write_text(json.dumps({"opt": {"class_path": sgd, "init_args": {"momentum": 0.5}}})) + assert get_sources(parser.parse_args(["--config=cfg.json", "--opt.init_args.lr=0.9"])) == { + "opt.class_path": "config file cfg.json", + "opt.init_args.lr": "command line argument --opt.init_args.lr", + "opt.init_args.momentum": "config file cfg.json", + } + + +def test_subclass_init_args_in_later_config_file(parser, tmp_cwd): + parser.add_argument("--opt", type=Optimizer) + Path("one.json").write_text(json.dumps({"opt": {"class_path": sgd}})) + Path("two.json").write_text('{"opt": {"init_args": {"momentum": 0.5}}}') + assert get_sources(parser.parse_args(["--config=one.json", "--config=two.json"])) == { + "opt.class_path": "config file one.json", + "opt.init_args.lr": "default", + "opt.init_args.momentum": "config file two.json", + } + + +def test_subclass_from_command_line(parser): + parser.add_argument("--opt", type=Optimizer) + assert get_sources(parser.parse_args([f"--opt={sgd}", "--opt.init_args.momentum=0.5"])) == { + "opt.class_path": "command line argument --opt", + "opt.init_args.lr": "default", + "opt.init_args.momentum": "command line argument --opt.init_args.momentum", + } + + +def test_subclass_subconfig_file(parser, tmp_cwd): + parser.add_argument("--opt", type=Optimizer, sub_configs=True) + Path("sub.json").write_text(json.dumps({"class_path": sgd, "init_args": {"momentum": 0.5}})) + Path("cfg.json").write_text('{"opt": "sub.json"}') + expected = { + "opt.class_path": "config file sub.json", + "opt.init_args.lr": "default", + "opt.init_args.momentum": "config file sub.json", + } + assert get_sources(parser.parse_args(["--opt=sub.json"])) == expected + assert get_sources(parser.parse_args(["--config=cfg.json"])) == expected + + +def test_subcommand(parser, subparser, tmp_cwd): + subparser.add_argument("--config", action="config") + subparser.add_argument("--val", type=int, default=1) + subparser.add_argument("--other", type=int, default=1) + parser.add_subcommands().add_subcommand("fit", subparser) + Path("fit.json").write_text('{"val": 2}') + Path("cfg.json").write_text('{"fit": {"val": 2}}') + command_line = {"subcommand": "command line argument subcommand", "fit.other": "command line argument --other"} + sources = get_sources(parser.parse_args(["fit", "--config=fit.json", "--other=3"])) + assert sources == {**command_line, "fit.val": "config file fit.json"} + sources = get_sources(parser.parse_args(["--config=cfg.json", "fit", "--other=3"])) + assert sources == {**command_line, "fit.val": "config file cfg.json"} + + +def test_subcommand_environment_variable(parser, subparser, monkeypatch): + parser.default_env = True + subparser.add_argument("--val", type=int, default=1) + parser.add_subcommands().add_subcommand("fit", subparser) + monkeypatch.setenv("APP_SUBCOMMAND", "fit") + monkeypatch.setenv("APP_FIT__VAL", "2") + assert get_sources(parser.parse_args([])) == { + "subcommand": "environment variable APP_SUBCOMMAND", + "fit.val": "environment variable APP_FIT__VAL", + } + + +def test_list_of_subclasses(parser, tmp_cwd): + parser.add_argument("--opts", type=list[Optimizer]) + config = {"opts": [{"class_path": sgd, "init_args": {"momentum": 0.5}}, {"class_path": optimizer}]} + Path("cfg.json").write_text(json.dumps(config)) + assert get_sources(parser.parse_args(["--config=cfg.json"])) == { + "opts[0].class_path": "config file cfg.json", + "opts[0].init_args.lr": "default", + "opts[0].init_args.momentum": "config file cfg.json", + "opts[1].class_path": "config file cfg.json", + "opts[1].init_args.lr": "default", + } + + +def test_list_of_subclasses_appended_from_command_line(parser, tmp_cwd): + parser.add_argument("--opts", type=list[Optimizer]) + Path("cfg.json").write_text(json.dumps({"opts": [{"class_path": sgd, "init_args": {"momentum": 0.5}}]})) + args = ["--config=cfg.json", f"--opts+={optimizer}", "--opts.init_args.lr=0.7"] + assert get_sources(parser.parse_args(args)) == { + "opts[0].class_path": "config file cfg.json", + "opts[0].init_args.lr": "default", + "opts[0].init_args.momentum": "config file cfg.json", + "opts[1].class_path": "command line argument --opts+", + "opts[1].init_args.lr": "command line argument --opts.init_args.lr", + } + + +def test_list_of_subclasses_item_from_subconfig_file(parser, tmp_cwd): + parser.add_argument("--opts", type=list[Optimizer], sub_configs=True) + Path("item.json").write_text(json.dumps({"class_path": sgd, "init_args": {"momentum": 0.5}})) + Path("cfg.json").write_text('{"opts": ["item.json"]}') + assert get_sources(parser.parse_args(["--config=cfg.json"])) == { + "opts[0].class_path": "config file item.json", + "opts[0].init_args.lr": "default", + "opts[0].init_args.momentum": "config file item.json", + } + + +def test_list_of_subclasses_from_subconfig_file(parser, tmp_cwd): + parser.add_argument("--opts", type=list[Optimizer], sub_configs=True) + Path("opts.json").write_text(json.dumps([{"class_path": sgd, "init_args": {"momentum": 0.5}}])) + assert get_sources(parser.parse_args(["--opts=opts.json"])) == { + "opts[0].class_path": "config file opts.json", + "opts[0].init_args.lr": "default", + "opts[0].init_args.momentum": "config file opts.json", + } + + +def test_list_of_subclasses_in_init_args(parser, tmp_cwd): + parser.add_argument("--trainer", type=Trainer) + config = {"trainer": {"class_path": f"{__name__}.Trainer", "init_args": {"opts": [{"class_path": sgd}]}}} + Path("cfg.json").write_text(json.dumps(config)) + assert get_sources(parser.parse_args(["--config=cfg.json"])) == { + "trainer.class_path": "config file cfg.json", + "trainer.init_args.opts[0].class_path": "config file cfg.json", + "trainer.init_args.opts[0].init_args.lr": "default", + "trainer.init_args.opts[0].init_args.momentum": "default", + } + + +def test_dict_of_subclasses(parser, tmp_cwd): + parser.add_argument("--opts", type=dict[str, Optimizer]) + Path("cfg.json").write_text(json.dumps({"opts": {"a": {"class_path": sgd, "init_args": {"momentum": 0.5}}}})) + assert get_sources(parser.parse_args(["--config=cfg.json", f"--opts.b={optimizer}"])) == { + "opts.a.class_path": "config file cfg.json", + "opts.a.init_args.lr": "default", + "opts.a.init_args.momentum": "config file cfg.json", + "opts.b.class_path": "command line argument --opts.b", + "opts.b.init_args.lr": "default", + } + + +def test_list_and_dict_values(parser, tmp_cwd): + parser.add_argument("--items", type=list[int], default=[]) + parser.add_argument("--mapping", type=dict[str, int], default={}) + Path("cfg.json").write_text('{"items": [1], "mapping": {"a": 1}}') + assert get_sources(parser.parse_args(["--config=cfg.json", "--items+=2"])) == { + "items": "command line argument --items+", + "mapping": "config file cfg.json", + } + + +@pytest.mark.skipif(not omegaconf_support, reason="omegaconf package is required") +def test_omegaconf_interpolation(tmp_cwd): + parser = ArgumentParser(exit_on_error=False, parser_mode="omegaconf+") + parser.add_argument("--config", action="config") + parser.add_argument("--a", type=str, default="x") + parser.add_argument("--b", type=str, default="y") + Path("cfg.yaml").write_text("a: z\nb: ${a}\n") + assert get_sources(parser.parse_args(["--config=cfg.yaml"])) == { + "a": "config file cfg.yaml:1", + "b": "config file cfg.yaml:2", + } + + +# provenance is not visible otherwise + + +def test_provenance_not_part_of_equality(parser): + parser.add_argument("--val", type=int, default=1) + from_command_line = parser.parse_args(["--val=1"]) + from_default = parser.parse_args([]) + assert get_sources(from_command_line) != get_sources(from_default) + assert from_command_line == from_default + + +def test_deepcopy_parsed_namespace(parser, tmp_cwd): + parser.add_argument("--opts", type=list[Optimizer]) + Path("cfg.json").write_text(json.dumps({"opts": [{"class_path": sgd, "init_args": {"momentum": 0.5}}]})) + cfg = parser.parse_args(["--config=cfg.json"]) + copied = copy.deepcopy(cfg) + assert copied == cfg + assert get_sources(copied) == get_sources(cfg) + + +def test_pickle_parsed_namespace(parser, tmp_cwd): + parser.add_argument("--opt", type=Optimizer) + Path("cfg.json").write_text(json.dumps({"opt": {"class_path": sgd}})) + cfg = parser.parse_args(["--config=cfg.json"]) + unpickled = pickle.loads(pickle.dumps(cfg)) + assert unpickled == cfg + assert get_sources(unpickled) == get_sources(cfg) + + +# error messages + + +def get_error(parser: ArgumentParser, args: list[str]) -> str: + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(args) + return str(ctx.value) + + +def test_error_config_file(parser, tmp_cwd): + parser.add_argument("--a", type=int, default=0) + parser.add_argument("--val", type=int, default=0) + Path("cfg.json").write_text('{"a": 1, "val": "abc"}') + assert get_error(parser, ["--config=cfg.json"]).endswith("Got value: abc\n Source: config file cfg.json") + + +def test_error_later_config_file(parser, tmp_cwd): + parser.add_argument("--val", type=int, default=0) + Path("one.json").write_text('{"val": 1}') + Path("two.json").write_text('{"val": "abc"}') + assert get_error(parser, ["--config=one.json", "--config=two.json"]).endswith("\n Source: config file two.json") + + +def test_error_config_string(parser): + parser.add_argument("--val", type=int, default=0) + assert get_error(parser, ['--config={"val": "abc"}']).endswith("\n Source: config string") + + +def test_error_config_file_syntax(parser, tmp_cwd): + parser.add_argument("--val", type=int, default=0) + Path("cfg.json").write_text('{"val": [1, 2}') + error = get_error(parser, ["--config=cfg.json"]) + assert error.startswith("Problems parsing config:") + assert error.endswith("\n Source: config file cfg.json") + + +def test_error_default_config_file(parser, tmp_cwd): + parser.add_argument("--val", type=int, default=0) + Path("defaults.json").write_text('{"val": "abc"}') + parser.default_config_files = ["defaults.json"] + assert get_error(parser, []).endswith("\n Source: default config file defaults.json") + + +def test_error_environment_variable(parser, monkeypatch): + parser.default_env = True + parser.add_argument("--val", type=int, default=0) + monkeypatch.setenv("APP_VAL", "abc") + assert get_error(parser, []).endswith("\n Source: environment variable APP_VAL") + + +def test_error_unknown_key_in_config_file(parser, tmp_cwd): + parser.add_argument("--val", type=int, default=0) + Path("cfg.json").write_text('{"val": 1, "unknown": 2}') + expected = "Option 'unknown' is not accepted\n Source: config file cfg.json" + assert get_error(parser, ["--config=cfg.json"]) == expected + + +def test_error_nested_value_in_config_file(parser, tmp_cwd): + parser.add_argument("--opt", type=Optimizer) + Path("cfg.json").write_text(json.dumps({"opt": {"class_path": sgd, "init_args": {"momentum": "abc"}}})) + error = get_error(parser, ["--config=cfg.json"]) + assert 'Parser key "momentum"' in error + assert error.count("Source:") == 1 + assert error.endswith("\n Source: config file cfg.json") + + +def test_error_class_group_subconfig_file(parser, tmp_cwd): + parser.add_argument("--model", type=Model, sub_configs=True) + Path("model.json").write_text('{"lr": "abc"}') + assert get_error(parser, ["--model=model.json"]).endswith("\n Source: config file model.json") + + +def test_error_config_file_path_relative_to_working_directory(parser, tmp_cwd): + parser.add_argument("--val", type=int, default=0) + Path("cfgs").mkdir() + Path("cfgs", "cfg.json").write_text('{"val": "abc"}') + error = get_error(parser, ["--config=" + str(Path("cfgs", "cfg.json"))]) + assert error.endswith(f"\n Source: config file {Path('cfgs', 'cfg.json')}") + + +def test_error_subconfig_file_path_relative_to_working_directory(parser, tmp_cwd): + parser.add_argument("--model", type=Model, sub_configs=True) + Path("cfgs").mkdir() + Path("cfgs", "model.json").write_text('{"lr": "abc"}') + Path("cfgs", "main.json").write_text('{"model": "model.json"}') + error = get_error(parser, ["--config=" + str(Path("cfgs", "main.json"))]) + assert error.endswith(f"\n Source: config file {Path('cfgs', 'model.json')}") + + +def test_error_command_line(parser): + parser.add_argument("--val", type=int, default=0) + assert get_error(parser, ["--val=abc"]).endswith("Got value: abc\n Source: command line argument --val") + + +def test_error_command_line_nested_option(parser): + parser.add_argument("--opt", type=Optimizer) + error = get_error(parser, [f"--opt={sgd}", "--opt.init_args.momentum=abc"]) + assert error.count("Source:") == 1 + assert error.endswith("Source: command line argument --opt.init_args.momentum") + + +def test_error_from_argparse_without_source(parser): + parser.add_argument("--val", type=int, default=0) + parser.add_argument("--other", type=int, default=0) + error = get_error(parser, ["--val=1", "--other"]) + assert "expected one argument" in error + assert "Source:" not in error + + +def test_error_parse_object_without_source(parser): + parser.add_argument("--val", type=int, default=0) + with pytest.raises(ArgumentError) as ctx: + parser.parse_object({"val": "abc"}) + assert "Source:" not in str(ctx.value) + + +def test_error_source_printed_to_stderr(parser, tmp_cwd): + parser.add_argument("--val", type=int, default=0) + Path("cfg.json").write_text('{"val": "abc"}') + err = get_parse_args_stderr(parser, ["--config=cfg.json"]) + assert "Got value: abc\n Source: config file cfg.json\n" in err + + +# line numbers + + +@skip_if_no_pyyaml +def test_config_file_line_numbers(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--val", type=int, default=0) + yaml_parser.add_argument("--model", type=Model) + Path("cfg.yaml").write_text("val: 1\nmodel:\n lr: 0.001\n encoder:\n layers: 12\n") + assert get_sources(yaml_parser.parse_args(["--config=cfg.yaml"])) == { + "val": "config file cfg.yaml:1", + "model.lr": "config file cfg.yaml:3", + "model.encoder.layers": "config file cfg.yaml:5", + "model.encoder.dropout": "default", + } + + +@skip_if_no_pyyaml +def test_list_of_subclasses_line_numbers(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--opts", type=list[Optimizer]) + content = f"opts:\n - class_path: {sgd}\n init_args:\n momentum: 0.5\n - class_path: {optimizer}\n" + Path("cfg.yaml").write_text(content) + assert get_sources(yaml_parser.parse_args(["--config=cfg.yaml"])) == { + "opts[0].class_path": "config file cfg.yaml:2", + "opts[0].init_args.lr": "default", + "opts[0].init_args.momentum": "config file cfg.yaml:4", + "opts[1].class_path": "config file cfg.yaml:5", + "opts[1].init_args.lr": "default", + } + + +@skip_if_no_pyyaml +def test_json_config_file_line_numbers(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--model", type=Model) + Path("cfg.json").write_text(json.dumps({"model": {"lr": 0.001, "encoder": {"layers": 12}}}, indent=2)) + assert get_sources(yaml_parser.parse_args(["--config=cfg.json"])) == { + "model.lr": "config file cfg.json:3", + "model.encoder.layers": "config file cfg.json:5", + "model.encoder.dropout": "default", + } + + +@skip_if_no_pyyaml +def test_default_config_file_line_numbers(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--a", type=int, default=0) + yaml_parser.add_argument("--val", type=int, default=0) + Path("defaults.yaml").write_text("a: 1\nval: 9\n") + yaml_parser.default_config_files = ["defaults.yaml"] + assert get_sources(yaml_parser.parse_args([])) == { + "a": "default config file defaults.yaml:1", + "val": "default config file defaults.yaml:2", + } + + +@skip_if_no_pyyaml +def test_subclass_line_numbers(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--opt", type=Optimizer) + Path("cfg.yaml").write_text(f"opt:\n class_path: {sgd}\n init_args:\n momentum: 0.5\n") + assert get_sources(yaml_parser.parse_args(["--config=cfg.yaml"])) == { + "opt.class_path": "config file cfg.yaml:2", + "opt.init_args.lr": "default", + "opt.init_args.momentum": "config file cfg.yaml:4", + } + + +@skip_if_no_pyyaml +def test_subclass_implicit_init_args_line_numbers(yaml_parser, tmp_cwd): + """Keys that are not literally in the file, like the class_path, get the line of their closest ancestor.""" + 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.init_args.lr": "config file cfg.yaml:2", + } + + +@skip_if_no_pyyaml +def test_subclass_subconfig_file_line_numbers(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--opt", type=Optimizer, sub_configs=True) + Path("sub.yaml").write_text(f"class_path: {sgd}\ninit_args:\n momentum: 0.5\n") + Path("implicit.yaml").write_text("lr: 0.5\n") + assert get_sources(yaml_parser.parse_args(["--opt=sub.yaml"])) == { + "opt.class_path": "config file sub.yaml:1", + "opt.init_args.lr": "default", + "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.init_args.lr": "config file implicit.yaml:1", + } + + +@skip_if_no_pyyaml +def test_subcommand_line_numbers(yaml_parser, subparser, tmp_cwd): + subparser.add_argument("--val", type=int, default=1) + yaml_parser.add_subcommands().add_subcommand("fit", subparser) + Path("cfg.yaml").write_text("fit:\n val: 2\n") + assert get_sources(yaml_parser.parse_args(["--config=cfg.yaml", "fit"])) == { + "subcommand": "command line argument subcommand", + "fit.val": "config file cfg.yaml:2", + } + + +@skip_if_no_pyyaml +@pytest.mark.skipif( + "JSONARGPARSE_OMEGACONF_FULL_TEST" in os.environ, + reason="the omegaconf yaml loader does not accept duplicate keys", +) +def test_duplicate_key_line_number(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--val", type=int, default=0) + Path("cfg.yaml").write_text("val: 1\nval: 2\n") + assert get_sources(yaml_parser.parse_args(["--config=cfg.yaml"])) == {"val": "config file cfg.yaml:2"} + + +@skip_if_no_pyyaml +def test_error_line_number(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--a", type=int, default=0) + yaml_parser.add_argument("--val", type=int, default=0) + Path("cfg.yaml").write_text("a: 1\nval: abc\n") + expected = "Got value: abc\n Source: config file cfg.yaml:2\n 2 | val: abc" + assert get_error(yaml_parser, ["--config=cfg.yaml"]).endswith(expected) + + +@skip_if_no_pyyaml +def test_error_line_of_single_line_json_is_shortened(yaml_parser, tmp_cwd): + config = {f"a{num}": num for num in range(20)} | {"val": "abc"} | {f"b{num}": num for num in range(20)} + for key in config: + yaml_parser.add_argument(f"--{key}", type=int, default=0) + Path("cfg.json").write_text(json.dumps(config)) + snippet = get_error(yaml_parser, ["--config=cfg.json"]).splitlines()[-1] + assert snippet.startswith(" 1 | ...") + assert snippet.endswith("...") + assert '"val": "abc"' in snippet + assert len(snippet) < 100 + + +@skip_if_no_pyyaml +def test_error_syntax_without_line_number(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--val", type=int, default=0) + Path("cfg.yaml").write_text("val: [1, 2\n") + assert get_error(yaml_parser, ["--config=cfg.yaml"]).endswith("\n Source: config file cfg.yaml") + + +@skip_if_no_pyyaml +def test_dump_without_provenance(yaml_parser): + yaml_parser.add_argument("--val", type=int, default=1) + cfg = yaml_parser.parse_args(["--val=2"]) + assert yaml_parser.dump(cfg) == "val: 2\n" + + +# print config provenance flag + + +def test_help_lists_provenance_flag(parser): + assert "provenance" in get_parser_help(parser) + + +@skip_if_no_ruamel +def test_print_config_provenance(yaml_parser, tmp_cwd, monkeypatch): + yaml_parser.default_env = True + yaml_parser.add_argument("--val", type=int, default=0) + yaml_parser.add_argument("--model", type=Model) + yaml_parser.add_argument("--encoder", type=Optional[Encoder], default=None) + yaml_parser.add_argument("--opt", type=Optimizer) + Path("cfg.yaml").write_text( + f"model:\n lr: 0.001\n encoder:\n layers: 12\nencoder:\n layers: 3\nopt:\n class_path: {sgd}\n" + ) + monkeypatch.setenv("APP_VAL", "7") + out = get_parse_args_stdout( + yaml_parser, ["--config=cfg.yaml", "--model.encoder.dropout=0.3", "--print_config=provenance"] + ) + assert out == ( + "val: 7 # environment variable APP_VAL\n" + "model:\n" + " lr: 0.001 # config file cfg.yaml:2\n" + " encoder:\n" + " layers: 12 # config file cfg.yaml:4\n" + " dropout: 0.3 # command line argument --model.encoder.dropout\n" + "encoder:\n" + " layers: 3 # config file cfg.yaml:6\n" + " dropout: 0.0 # default\n" + "opt:\n" + f" class_path: {sgd} # config file cfg.yaml:8\n" + " init_args:\n" + " lr: 0.2 # default\n" + " momentum: 0.9 # default\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]) + Path("cfg.yaml").write_text(f"opts:\n - class_path: {sgd}\n init_args:\n momentum: 0.5\n") + out = get_parse_args_stdout(yaml_parser, ["--config=cfg.yaml", "--print_config=provenance"]) + assert out == ( + "opts:\n" + f"- class_path: {sgd} # config file cfg.yaml:2\n" + " init_args:\n" + " lr: 0.2 # default\n" + " momentum: 0.5 # config file cfg.yaml:4\n" + ) + + +@skip_if_no_ruamel +def test_print_config_provenance_list_and_dict(yaml_parser, tmp_cwd): + yaml_parser.add_argument("--items", type=list[int], default=[]) + yaml_parser.add_argument("--mapping", type=dict[str, int], default={}) + Path("cfg.yaml").write_text("items: [1]\nmapping:\n a: 1\n") + out = get_parse_args_stdout(yaml_parser, ["--config=cfg.yaml", "--items+=2", "--print_config=provenance"]) + assert out == "items: # command line argument --items+\n- 1\n- 2\nmapping: # config file cfg.yaml:2\n a: 1\n" + + +@skip_if_no_ruamel +def test_print_config_provenance_with_comments(yaml_parser): + yaml_parser.add_argument("--val", type=int, default=1, help="Value.") + out = get_parse_args_stdout(yaml_parser, ["--val=2", "--print_config=comments,provenance"]) + assert "# Value. (type: int, default: 1)\nval: 2 # command line argument --val\n" in out + + +@skip_if_no_ruamel +def test_print_config_provenance_with_skip_default(yaml_parser): + yaml_parser.add_argument("--a", type=int, default=1) + yaml_parser.add_argument("--b", type=int, default=1) + out = get_parse_args_stdout(yaml_parser, ["--b=2", "--print_config=provenance,skip_default"]) + assert out == "b: 2 # command line argument --b\n" + + +@skip_if_no_ruamel +def test_print_config_provenance_multiline_string(yaml_parser): + yaml_parser.add_argument("--text", type=str, default="multi\nline") + yaml_parser.add_argument("--val", type=int, default=1) + out = get_parse_args_stdout(yaml_parser, ["--val=2", "--print_config=provenance"]) + assert out == "# default\ntext: |-\n multi\n line\nval: 2 # command line argument --val\n"