diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2d208e0c..557ab7e6 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -41,8 +41,8 @@ Added ``add_method_arguments``, and links applied on instantiate can target the parameters of these groups (`#986 `__). -- New ``__include__`` key accepted at any level of a config, which merges into it - the configs that it gives, so that a config can be composed from others. +- New ``__include__`` key accepted at any level of a config, which merges into + it the configs that it gives, so that a config can be composed from others. Enabled with the new ``config_include_enabled`` of ``set_parsing_settings`` (`#990 `__). - Support for ``TypeAliasType`` aliases whose value is a string, e.g. @@ -56,13 +56,11 @@ Added ``JSONARGPARSE_ALLOW_ABBREV`` environment variable, to accept abbreviated options in parsers not given ``allow_abbrev`` (`#993 `__). -- New ``parser_mode`` setting in ``set_parsing_settings`` and - ``JSONARGPARSE_PARSER_MODE`` environment variable, to change the mode of - parsers not given ``parser_mode`` (`#997 - `__). - Support for ``datetime``, ``date`` and ``time`` (ISO 8601), ``re.Pattern`` (str only) and ``Fraction`` types (`#1002 `__). +- New ``json_or_yaml`` parser mode, which parses as JSON and if that fails as + YAML (`#1004 `__). Fixed ^^^^^ @@ -114,16 +112,16 @@ Fixed - ``--print_config`` failing when a required subcommand is not given, unlike other required arguments (`#984 `__). -- ``instantiate``, ``auto_cli`` and ``from_config`` failing when a signature - has positional-only parameters (`#986 +- ``instantiate``, ``auto_cli`` and ``from_config`` failing when a signature has + positional-only parameters (`#986 `__). - AST resolver failing for a method whose ``self`` is positional-only (`#986 `__). - A typed positional with ``nargs="*"`` set in a config file being reset to empty when no values for it are given in the command line (`#986 `__). -- A positional-only parameter that has a default being required, even though - the call can omit it (`#986 +- A positional-only parameter that has a default being required, even though the + call can omit it (`#986 `__). - Parse errors logged multiple times when raised from a nested parse, e.g. an invalid value in a config given as argument (`#989 @@ -145,14 +143,17 @@ Fixed - Recursive type aliases, e.g. ``type Json = int | list[Json]``, failing with ``RecursionError`` (`#991 `__). -- Sentinel defaults, e.g. ``UNSET`` for ``x: int | UnsetType = UNSET``, shown - in the help as ````, and failing to serialize when - the sentinel is not defined in the module of its class (`#991 +- Sentinel defaults, e.g. ``UNSET`` for ``x: int | UnsetType = UNSET``, shown in + the help as ````, and failing to serialize when the + sentinel is not defined in the module of its class (`#991 `__). - ``TypedDict`` with ``extra_items`` (PEP 728) rejecting the keys it doesn't declare, failing with ``Unexpected keys`` instead of validating them against the ``extra_items`` type (`#1000 `__). +- ``jsonnet`` parser mode hiding Jsonnet evaluation errors when the config also + parses as YAML, e.g. ``{"a": error "boom"}`` giving ``a`` the string ``error + "boom"`` (`#1004 `__). Changed ^^^^^^^ @@ -178,8 +179,8 @@ Changed the only way to strip it (`#969 `__). - ``pyyaml`` is no longer a required dependency, install the ``yaml`` extra to - have it. Without it, using ``yaml`` raises an informative ``ImportError`` (`#970 - `__). + have it. Without it, using ``yaml`` raises an informative ``ImportError`` + (`#970 `__). - The ``json`` dump format is now indented, so that the print config argument gives a more readable output. Use the new ``json_compact`` format for the previous single line output (`#970 @@ -193,7 +194,8 @@ Changed `__). - 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 `__). + is not installed (`#975 + `__). - Relative paths in config files are now resolved without changing the process working directory (`#979 `__). @@ -226,14 +228,16 @@ Changed `__). - ``from_config`` now fails with ``ArgumentError`` for an invalid config, including a missing required parameter, instead of ``TypeError`` or - ``ImportDenied`` (`#990 `__). + ``ImportDenied`` (`#990 + `__). - ``allow_abbrev`` now defaults to ``False``, so abbreviated options, e.g. ``--max`` for ``--max_epochs``, fail instead of being expanded (`#993 `__). -- ``parser_mode`` now defaults to ``json`` instead of ``yaml``, so YAML configs - and values with YAML syntax, e.g. ``--list=[a, b]`` or ``--flag=yes``, fail - unless ``parser_mode="yaml"`` is given. The ``jsonnet`` parser mode no longer - accepts YAML (`#997 `__). +- ``parser_mode`` now defaults to the new ``json_or_yaml`` mode instead of + ``yaml``, so values are parsed as JSON and only if that fails as YAML, which + without the ``yaml`` extra makes YAML configs fail. Load errors now say which + format failed to parse (`#1004 + `__). Removed ^^^^^^^ diff --git a/DOCUMENTATION.rst b/DOCUMENTATION.rst index 113d32cc..e817c14a 100644 --- a/DOCUMENTATION.rst +++ b/DOCUMENTATION.rst @@ -1354,13 +1354,11 @@ Configuration files jsonargparse can parse configuration files (config files). The dot notation hierarchy of the arguments (see :ref:`nested-namespaces`) defines the structure -expected in these files. The default format is JSON. To change it, use the -``parser_mode`` parameter of the parser, e.g. -``ArgumentParser(parser_mode="yaml")``, or set it globally, which also applies -to parsers created by other code, with ``set_parsing_settings(parser_mode=...)`` -or the environment variable ``JSONARGPARSE_PARSER_MODE``. The examples in this -documentation use ``yaml``, which requires the ``yaml`` extra, see -:ref:`installation`. +expected in these files. The default ``parser_mode`` is ``json_or_yaml``, which +parses as JSON and, if that fails, as YAML. YAML requires the ``yaml`` extra, +see :ref:`installation`, so without it only JSON is accepted. To change the +mode, use the ``parser_mode`` parameter of the parser, e.g. +``ArgumentParser(parser_mode="json")``. The :py:attr:`.ArgumentParser.default_config_files` property holds patterns of config files to search for, e.g. @@ -1521,7 +1519,8 @@ From Python, a config object is serialized with the :meth:`dump <.ArgumentParser.dump>` and :meth:`save <.ArgumentParser.save>` methods. The supported formats are ``yaml``, ``toml``, ``json``/``json_indented``, ``json_compact`` and ``parser_mode``, the default, which uses the format of the -parser, or ``json`` when the parser mode has no dumper. The ``yaml`` format +parser, ``yaml`` for ``json_or_yaml`` when the ``yaml`` extra is installed, or +``json`` when the parser mode has no dumper. The ``yaml`` format dumps with a subclass of `yaml.SafeDumper `__ that writes multi-line strings as literal blocks, i.e. ``|``, instead of escaping the line breaks. More @@ -3173,9 +3172,9 @@ The parser then uses these variables, unless the command line overrides them: 'from env 2' Note the ``default_env=True`` given to the parser. By default :meth:`parse_args -<.ArgumentParser.parse_args>` does not parse environment variables. If -``default_env`` is left unset, they can also be enabled by setting -``JSONARGPARSE_DEFAULT_ENV=true`` in the shell. +<.ArgumentParser.parse_args>` does not parse environment variables. Setting +``JSONARGPARSE_DEFAULT_ENV`` to ``true`` or ``false`` in the shell overrides +``default_env`` for all parsers, even ones given it explicitly. The :meth:`parse_env <.ArgumentParser.parse_env>` method parses only environment variables, useful when there is no command line call. @@ -3782,7 +3781,8 @@ globally, which also applies to parsers created by other code: set_parsing_settings(allow_abbrev=False) Without changing Python code, the same is achieved by setting the environment -variable ``JSONARGPARSE_ALLOW_ABBREV=true``. +variable ``JSONARGPARSE_ALLOW_ABBREV=true``. Neither applies to parsers given +``allow_abbrev`` explicitly. User defined types ------------------ diff --git a/README.rst b/README.rst index 831228d9..894aaacb 100644 --- a/README.rst +++ b/README.rst @@ -190,11 +190,12 @@ specifying one or more of the following extras (optional dependencies): ``signatures``, ``docstrings``, ``typeshed``, ``yaml``, ``jsonschema``, ``jsonnet``, ``urls``, ``fsspec``, ``toml``, ``ruamel``, ``omegaconf``, ``shtab``, and ``argcomplete``. The ``yaml`` extra installs `PyYAML -`__, required for ``parser_mode="yaml"``. The -``signatures`` extra includes ``docstrings`` and ``typeshed``, which can also be -installed individually. Additionally, the ``all`` extras can be used to enable -all optional features (excluding tab completion ones). To install -``jsonargparse`` with extras, use the following syntax: +`__, without which config files are parsed +only as JSON. The ``signatures`` extra includes ``docstrings`` and +``typeshed``, which can also be installed individually. Additionally, the +``all`` extras can be used to enable all optional features (excluding tab +completion ones). To install ``jsonargparse`` with extras, use the following +syntax: .. code-block:: bash diff --git a/jsonargparse/_actions.py b/jsonargparse/_actions.py index a6e281cf..7f541cb7 100644 --- a/jsonargparse/_actions.py +++ b/jsonargparse/_actions.py @@ -130,12 +130,19 @@ def apply_config(parser, cfg, dest, value) -> None: cfg_path: Path | None = Path(value, mode=_get_config_read_mode()) except TypeError as ex_path: try: - if isinstance(load_value(value), str): + try: + loaded = load_value(value) + except get_loader_exceptions() as ex_load: + if value.strip()[:1] in {"[", "{"}: # clearly not intended as a path + raise ex_load + errors = f"- {ex_path}\n- {indent_text(str(ex_load), first_line=False)}" + raise TypeError(f"Not a valid path nor a config string:\n{errors}") from ex_load + if isinstance(loaded, str): raise ex_path cfg_path = None cfg_file = parser.parse_string(value, **kwargs) except (TypeError, ValueError) + get_loader_exceptions() as ex_str: - raise TypeError(f'Parser key "{dest}": {ex_str}') from ex_str + raise TypeError(f'Parser key "{dest}": {indent_text(str(ex_str), first_line=False)}') from ex_str else: cfg_file = parser.parse_path(value, **kwargs) cfg_merged = merge_config(parser, cfg_file, cfg) diff --git a/jsonargparse/_common.py b/jsonargparse/_common.py index 6b4980c0..315affa5 100644 --- a/jsonargparse/_common.py +++ b/jsonargparse/_common.py @@ -281,7 +281,6 @@ class ImportDenied(ImportError, ValueError): parsing_settings: dict = { "allow_abbrev": False, - "parser_mode": "json", "validate_defaults": False, "validate_subclass_spec_in_any": False, "instantiate_subclass_spec_in_any": False, @@ -361,7 +360,6 @@ def get_env_var_bool(name: str) -> bool: def set_parsing_settings( *, allow_abbrev: bool | None = None, - parser_mode: str | None = None, validate_defaults: bool | None = None, validate_subclass_spec_in_any: bool | None = None, instantiate_subclass_spec_in_any: bool | None = None, @@ -388,9 +386,6 @@ def set_parsing_settings( ``--max`` for ``--max_epochs``. Default is ``False``, unlike argparse. Parsers given ``allow_abbrev`` take precedence, and the ``JSONARGPARSE_ALLOW_ABBREV`` environment variable over this. - parser_mode: Mode for parsing values and config files of parsers not - given ``parser_mode``. Default is ``json``. The - ``JSONARGPARSE_PARSER_MODE`` environment variable takes precedence. validate_defaults: Whether default values must be valid according to the argument type. Defaults are always validated to normalize them, but with the default ``False`` an invalid default is kept as is, like @@ -484,12 +479,6 @@ class when a value for a type that accepts any value, i.e. ``Any``, parsing_settings[name] = value elif value is not None: raise ValueError(f"{name} must be a boolean, but got {value}.") - # parser_mode - if parser_mode is not None: - from ._loaders_dumpers import check_parser_mode - - check_parser_mode(parser_mode) - parsing_settings["parser_mode"] = parser_mode # unset_sentinel if isinstance(unset_sentinel, bool): parsing_settings["unset_sentinel"] = Unset if unset_sentinel else None @@ -518,7 +507,6 @@ class when a value for a type that accepts any value, i.e. ``Any``, parsing_settings_env_vars = { "allow_abbrev": "JSONARGPARSE_ALLOW_ABBREV", - "parser_mode": "JSONARGPARSE_PARSER_MODE", "add_print_completion_argument": "JSONARGPARSE_ADD_PRINT_COMPLETION_ARGUMENT", } @@ -528,23 +516,10 @@ def get_parsing_setting(name: str): raise ValueError(f"Unknown parsing setting {name}.") var_name = parsing_settings_env_vars.get(name, "") if var_name in os.environ: - if name == "parser_mode": - return get_env_var_parser_mode(var_name) return get_env_var_bool(var_name) return parsing_settings[name] -def get_env_var_parser_mode(name: str) -> str: - from ._loaders_dumpers import check_parser_mode - - value = os.environ[name] - try: - check_parser_mode(value) - except ValueError as ex: - raise ValueError(f"Invalid value for environment variable {name}: {ex}") from ex - return value - - def validate_default(container: ActionsContainer, action: argparse.Action, logger: logging.Logger) -> None: if action.default is get_parsing_setting("unset_sentinel") or not hasattr(action, "_check_type"): return diff --git a/jsonargparse/_core.py b/jsonargparse/_core.py index 6ad2dda6..cd059611 100644 --- a/jsonargparse/_core.py +++ b/jsonargparse/_core.py @@ -45,11 +45,11 @@ from ._jsonschema import ActionJsonSchema from ._link_arguments import ActionLink, ArgumentLinking from ._loaders_dumpers import ( - check_parser_mode, check_valid_dump_format, dump_using_format, get_loader_exceptions, load_value, + loaders, ) from ._namespace import ( Namespace, @@ -70,6 +70,8 @@ _get_config_read_mode, fsspec_support, import_fsspec, + import_jsonnet, + import_pyyaml, omegaconf_apply, pydantic_model_accepts_extra, pydantic_support, @@ -109,6 +111,7 @@ get_argument_group_class, get_private_kwargs, identity, + indent_text, load_config_path_context, merge_config, resolve_config_includes, @@ -297,7 +300,7 @@ def __init__( logger: logging.Logger | bool | str | dict = False, version: str | None = None, print_config: str | None = "--print_%s", - parser_mode: str | None = None, + parser_mode: str = "json_or_yaml", dump_header: list[str] | None = None, default_config_files: list[str | os.PathLike] | None = None, default_env: bool = False, @@ -316,8 +319,8 @@ def __init__( logger: Logger to use or configuration for logger. version: Program version which will be printed by the ``--version`` argument. print_config: Name for print config argument, ``%s`` is replaced by config dest, set ``None`` to disable. - parser_mode: Mode for parsing values: ``json``, ``yaml``, ``jsonnet`` or added via :func:`.set_loader`, - ``None`` to use the global setting. + parser_mode: Mode for parsing values: ``json_or_yaml``, ``json``, ``yaml``, ``jsonnet`` or added via + :func:`.set_loader`. dump_header: Header to include as comment when dumping a config object. default_config_files: Default config file locations, e.g. ``['~/.config/myapp/*.yaml']``. default_env: Set the default value on whether to parse environment variables. @@ -800,7 +803,7 @@ def _load_config_parser_mode( try: cfg_dict = load_value(content, path=path, ext_vars=ext_vars) except get_loader_exceptions() as ex: - raise TypeError(f"Problems parsing config: {ex}") from ex + raise TypeError(f"Problems parsing config:\n{indent_text(str(ex))}") from ex if not isinstance(cfg_dict, dict): raise TypeError(f"Unexpected config: {content}") return self._apply_actions(resolve_config_includes(cfg_dict), prev_cfg=prev_cfg) @@ -1712,24 +1715,25 @@ def env_prefix(self, env_prefix: bool | str): @property def parser_mode(self) -> str: - """Mode for parsing config files, ``json``, ``yaml``, ``jsonnet`` or ones added via :func:`.set_loader`. - - When set to ``None``, the ``parser_mode`` setting of :func:`.set_parsing_settings` is used. + """Mode for parsing config files, ``json_or_yaml``, ``json``, ``yaml``, ``jsonnet`` or via :func:`.set_loader`. :getter: Returns the current parser mode. - :setter: Sets the parser mode or ``None``. + :setter: Sets the parser mode. Raises: ValueError: If an invalid value is given. """ - if self._parser_mode is None: - return get_parsing_setting("parser_mode") return self._parser_mode @parser_mode.setter - def parser_mode(self, parser_mode: str | None): - if parser_mode is not None: - check_parser_mode(parser_mode) + def parser_mode(self, parser_mode: str): + accepted = set(loaders).union({"omegaconf", "omegaconf+"}) + if parser_mode not in accepted: + raise ValueError(f"The only accepted values for parser_mode are {accepted}.") + if parser_mode == "jsonnet": + import_jsonnet("parser_mode=jsonnet") + elif parser_mode == "yaml": + import_pyyaml("parser_mode=yaml") self._parser_mode = parser_mode if self._subcommands_action: for subparser in self._subcommands_action._name_parser_map.values(): diff --git a/jsonargparse/_formatters.py b/jsonargparse/_formatters.py index 1427a97e..6e7b6503 100644 --- a/jsonargparse/_formatters.py +++ b/jsonargparse/_formatters.py @@ -32,7 +32,7 @@ ) from ._link_arguments import ActionLink from ._namespace import Namespace -from ._optionals import import_pyyaml, import_ruamel +from ._optionals import import_pyyaml, import_ruamel, pyyaml_available from ._subcommands import ActionSubCommands, find_action from ._type_checking import ArgumentParser, ruamelCommentedMap from ._typehints import ( @@ -337,7 +337,7 @@ def _set_provenance_comments(cfg, provenance: dict, key_lines: dict, prefix: str _set_provenance_comments(item, provenance, key_lines, f"{full_key}[{num}].", depth + 1) -line_number_modes = {"yaml", "omegaconf", "omegaconf+"} +line_number_modes = {"yaml", "omegaconf", "omegaconf+"} | ({"json_or_yaml"} if pyyaml_available else set()) snippet_width = 80 @@ -416,7 +416,11 @@ def collect(node, key): 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()), "") + try: + node = yaml.compose(content, Loader=get_yaml_default_loader()) + except yaml.YAMLError: # e.g. json that yaml does not accept, so positions are unknown + return positions + collect(node, "") return positions diff --git a/jsonargparse/_jsonnet.py b/jsonargparse/_jsonnet.py index 242f6014..0f01b3bb 100644 --- a/jsonargparse/_jsonnet.py +++ b/jsonargparse/_jsonnet.py @@ -11,7 +11,6 @@ get_jsonschema_exceptions, import_jsonnet, import_jsonschema, - pyyaml_available, ) from ._subcommands import find_action from ._typehints import ActionTypeHint @@ -47,11 +46,10 @@ def __init__( if schema is not None: jsonvalidator = import_jsonschema("ActionJsonnet")[1] if isinstance(schema, str): - mode = "yaml" if pyyaml_available else "json" - with parser_context(load_value_mode=mode): + with parser_context(load_value_mode="json_or_yaml"): try: schema = load_value(schema) - except get_loader_exceptions(mode) as ex: + except get_loader_exceptions("json_or_yaml") as ex: raise ValueError(f"Problems parsing schema: {ex}") from ex jsonvalidator.check_schema(schema) self._validator = ActionJsonSchema._extend_jsonvalidator_with_default(jsonvalidator)(schema) @@ -165,7 +163,7 @@ def parse( fname = jsonnet(absolute=False) if isinstance(jsonnet, Path) else jsonnet # type: ignore[operator] snippet = fpath.read_text() try: - with parser_context(load_value_mode="yaml" if pyyaml_available else "json"): + with parser_context(load_value_mode="json_or_yaml"): values = load_value(_jsonnet.evaluate_snippet(fname, snippet, ext_vars=ext_vars, ext_codes=ext_codes)) except RuntimeError as ex: raise argument_error(f"Problems evaluating Jsonnet '{fname}': {ex}") from ex diff --git a/jsonargparse/_jsonschema.py b/jsonargparse/_jsonschema.py index 1596885f..69e8ceec 100644 --- a/jsonargparse/_jsonschema.py +++ b/jsonargparse/_jsonschema.py @@ -9,7 +9,6 @@ from ._optionals import ( get_jsonschema_exceptions, import_jsonschema, - pyyaml_available, ) from ._util import check_no_composed_config, parse_value_or_config @@ -33,11 +32,10 @@ def __init__(self, schema: str | dict | None = None, sub_config: bool = True, wi """ if schema is not None: if isinstance(schema, str): - mode = "yaml" if pyyaml_available else "json" - with parser_context(load_value_mode=mode): + with parser_context(load_value_mode="json_or_yaml"): try: schema = load_value(schema) - except get_loader_exceptions(mode) as ex: + except get_loader_exceptions("json_or_yaml") as ex: raise ValueError(f"Problems parsing schema: {ex}") from ex jsonvalidator = import_jsonschema("ActionJsonSchema")[1] jsonvalidator.check_schema(schema) diff --git a/jsonargparse/_loaders_dumpers.py b/jsonargparse/_loaders_dumpers.py index 653bcd2d..68630d11 100644 --- a/jsonargparse/_loaders_dumpers.py +++ b/jsonargparse/_loaders_dumpers.py @@ -3,6 +3,7 @@ import inspect import os import re +import textwrap from argparse import HelpFormatter from collections.abc import Callable from contextlib import suppress @@ -15,6 +16,7 @@ import_toml_dumps, import_toml_loads, omegaconf_support, + pyyaml_available, ruamel_support, ) from ._paths import current_local_dir @@ -32,6 +34,10 @@ yaml_default_dumper = None +class LoadError(ValueError): + """Raised when a value fails to be parsed, saying which format was attempted.""" + + def load_basic(value): value = value.strip() if value == "true": @@ -118,6 +124,21 @@ def toml_load(value): return toml_loads(value) +def json_or_yaml_load(value): + import json + + try: + return json_load(value) + except json.JSONDecodeError as ex: + if not pyyaml_available: + raise + json_ex = ex + try: + return yaml_load(value) + except get_loader_exceptions("yaml") as yaml_ex: + raise LoadError(f"{parse_failure('as JSON', json_ex)}\n{parse_failure('as YAML', yaml_ex)}") from yaml_ex + + def jsonnet_load(stream, path="", ext_vars=None): from ._jsonnet import ActionJsonnet @@ -130,6 +151,9 @@ def jsonnet_load(stream, path="", ext_vars=None): try: val = _jsonnet.evaluate_snippet(path, stream, ext_vars=ext_vars, ext_codes=ext_codes) except RuntimeError as ex: + if str(ex).startswith("STATIC ERROR"): # not jsonnet syntax, so maybe json or yaml + with suppress(*get_loader_exceptions("json_or_yaml")): + return json_or_yaml_load(stream) raise ValueError(str(ex)) from ex return json_load(val) @@ -137,15 +161,26 @@ def jsonnet_load(stream, path="", ext_vars=None): loaders: dict[str, Callable] = { "yaml": yaml_load, "json": json_load, + "json_or_yaml": json_or_yaml_load, "toml": toml_load, } loader_exceptions: dict[str, tuple[type[Exception], ...]] = {} loader_json_superset: dict[str, bool] = { "yaml": True, "json": True, + "json_or_yaml": True, "toml": False, } loader_params: dict[str, set[str]] = {} +format_names = { + "yaml": "YAML", + "json": "JSON", + "json_or_yaml": "JSON", + "toml": "TOML", + "jsonnet": "Jsonnet", + "omegaconf": "YAML with OmegaConf", + "omegaconf+": "YAML with OmegaConf", +} def get_load_value_mode() -> str: @@ -163,23 +198,30 @@ def get_loader_exceptions(mode: str | None = None) -> tuple[type[Exception], ... if mode not in loader_exceptions: if mode == "yaml": loader_exceptions[mode] = (import_pyyaml("get_loader_exceptions").YAMLError,) - elif mode == "json": + elif mode in {"json", "json_or_yaml"}: loader_exceptions[mode] = (__import__("json").JSONDecodeError,) + if mode == "json_or_yaml" and pyyaml_available: + loader_exceptions[mode] += (import_pyyaml("get_loader_exceptions").YAMLError,) elif mode == "toml": loader_exceptions[mode] = (import_toml_loads("get_loader_exceptions")[1],) elif mode == "jsonnet": loader_exceptions[mode] = (ValueError,) - return loader_exceptions[mode] + return (LoadError,) + loader_exceptions[mode] -def check_parser_mode(parser_mode: str) -> None: - accepted = set(loaders).union({"omegaconf", "omegaconf+"}) - if parser_mode not in accepted: - raise ValueError(f"The only accepted values for parser_mode are {accepted}.") - if parser_mode == "jsonnet": - import_jsonnet("parser_mode=jsonnet") - elif parser_mode == "yaml": - import_pyyaml("parser_mode=yaml") +def parse_failure(name: str, ex: Exception) -> str: + return f"Failed to parse {name}: " + textwrap.indent(str(ex), " ").lstrip() + + +def load_error_message(mode: str, value: str, ex: Exception) -> str: + """Error message for a value that failed to load, saying which format was attempted.""" + name = f"as {format_names[mode]}" if mode in format_names else f"with parser_mode={mode!r}" + message = parse_failure(name, ex) + if mode == "json" and pyyaml_available: + with suppress(*get_loader_exceptions("yaml")): + if isinstance(yaml_load(value), (dict, list)): + message += "\nThe value parses as YAML, to accept it use parser_mode='json_or_yaml'" + return message def load_list_or_dict(value: str): @@ -210,7 +252,12 @@ def load_value(value: str, simple_types: bool = False, **kwargs): if kwargs and mode in loader_params: params = loader_params[mode] load_kwargs = {k: v for k, v in kwargs.items() if k in params} - loaded_value = loader(value, **load_kwargs) + try: + loaded_value = loader(value, **load_kwargs) + except LoadError: + raise + except get_loader_exceptions(mode) as ex: + raise LoadError(load_error_message(mode, value, ex)) from ex if not simple_types and isinstance(loaded_value, (int, float, bool, str)): loaded_value = value @@ -333,8 +380,9 @@ def dump_using_format( provenance: dict | None = None, ) -> str: if dump_format == "parser_mode": - default_format = "yaml" if parser.parser_mode.startswith("omegaconf") else "json" - dump_format = parser.parser_mode if parser.parser_mode in dumpers else default_format + mode = parser.parser_mode + is_yaml = mode.startswith("omegaconf") or (mode == "json_or_yaml" and pyyaml_available) + dump_format = mode if mode in dumpers else ("yaml" if is_yaml else "json") if with_comments or provenance is not None: if f"{dump_format}_comments" not in dumpers: if dump_format == "yaml": diff --git a/jsonargparse/_subcommands.py b/jsonargparse/_subcommands.py index 2ec18fda..54984f1a 100644 --- a/jsonargparse/_subcommands.py +++ b/jsonargparse/_subcommands.py @@ -128,7 +128,7 @@ def add_subcommand(self, name: str, parser: ArgumentParser, **kwargs) -> Argumen parser.env_prefix = f"{self.env_prefix}{name}_" parser.default_env = self.parent_parser.default_env parser.parent_parser = self.parent_parser # type: ignore[attr-defined] - parser.parser_mode = self.parent_parser._parser_mode + parser.parser_mode = self.parent_parser.parser_mode parser.exit_on_error = self.parent_parser.exit_on_error parser.formatter_class = self.parent_parser.formatter_class parser.logger = self.parent_parser.logger diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py index 474d14e1..b9d55e06 100644 --- a/jsonargparse/_typehints.py +++ b/jsonargparse/_typehints.py @@ -723,13 +723,16 @@ def _check_type(self, value, append=False, cfg=None, mode=None): if not islist: value = [value] for num, val in enumerate(value): + load_error = None try: orig_val = val enable_path = self._enable_path and not isinstance(val, NestedArg) try: val, config_path = parse_value_or_config(val, enable_path=enable_path) - except get_loader_exceptions(): + except get_loader_exceptions() as ex: config_path = None + if isinstance(val, str) and val.strip()[:1] in {"[", "{"}: + load_error = ex # included in the error, since it seems intended as a list or dict path_meta = val.pop("__path__", None) if isinstance(val, dict) else None # a single sub-config appended to a list becomes one more list item appended_subconfig = append and config_path is not None and not isinstance(val, list) @@ -790,7 +793,7 @@ def _check_type(self, value, append=False, cfg=None, mode=None): value[num] = val else: elem = "" if not islist else f" element {num + 1}" - error = indent_text(str(ex)) + error = indent_text(str(ex) if load_error is None else f"{ex}\n{load_error}") raise TypeError(f'Parser key "{self.dest}"{elem}:\n{error}') from ex return value if islist else value[0] diff --git a/jsonargparse_tests/conftest.py b/jsonargparse_tests/conftest.py index 449569cf..c37fa9df 100644 --- a/jsonargparse_tests/conftest.py +++ b/jsonargparse_tests/conftest.py @@ -32,10 +32,6 @@ set_parsing_settings(docstring_parse_style=DocstringStyle.GOOGLE) -# most tests write yaml configs when pyyaml is installed, while the default parser_mode is json -if pyyaml_available: - set_parsing_settings(parser_mode="yaml") - columns = "200" @@ -120,7 +116,7 @@ def parser_modes(test_function): if "JSONARGPARSE_OMEGACONF_FULL_TEST" in os.environ: parser_modes = ["yaml"] else: - parser_modes = ["json"] + parser_modes = ["json", "json_or_yaml"] if toml_load_available: parser_modes += ["toml"] if pyyaml_available: diff --git a/jsonargparse_tests/test_jsonnet.py b/jsonargparse_tests/test_jsonnet.py index c797466b..63c3bf61 100644 --- a/jsonargparse_tests/test_jsonnet.py +++ b/jsonargparse_tests/test_jsonnet.py @@ -18,6 +18,7 @@ get_parser_help, json_or_yaml_load, skip_if_jsonschema_unavailable, + skip_if_no_pyyaml, ) @@ -95,6 +96,31 @@ def test_parser_mode_jsonnet(tmp_path): pytest.raises(ArgumentError, lambda: parser.parse_args(["--cfg", "{}}"])) +@skip_if_no_pyyaml +def test_parser_mode_jsonnet_yaml_fallback(parser): + parser.parser_mode = "jsonnet" + parser.add_argument("--cfg", action="config") + parser.add_argument("--list", type=list) + assert parser.parse_args(["--cfg=list: [a, b]"]).list == ["a", "b"] + + +def test_parser_mode_jsonnet_runtime_error_no_yaml_fallback(parser): + parser.parser_mode = "jsonnet" + parser.add_argument("--cfg", action="config") + parser.add_argument("--a", type=str) + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(['--cfg={"a": error "boom"}']) + ctx.match("Failed to parse as Jsonnet: RUNTIME ERROR: boom") + + +def test_parser_mode_jsonnet_error(parser): + parser.parser_mode = "jsonnet" + parser.add_argument("--cfg", action="config") + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(["--cfg={}}"]) + ctx.match("Failed to parse as Jsonnet: STATIC ERROR") + + def test_parser_mode_jsonnet_import_libsonnet(parser, tmp_cwd): parser.parser_mode = "jsonnet" parser.add_argument("--cfg", action="config") diff --git a/jsonargparse_tests/test_loaders_dumpers.py b/jsonargparse_tests/test_loaders_dumpers.py index a12e1622..08a718a9 100644 --- a/jsonargparse_tests/test_loaders_dumpers.py +++ b/jsonargparse_tests/test_loaders_dumpers.py @@ -10,7 +10,7 @@ import pytest -from jsonargparse import ArgumentParser, get_loader, set_dumper, set_loader +from jsonargparse import ArgumentError, ArgumentParser, get_loader, set_dumper, set_loader from jsonargparse._common import parser_context from jsonargparse._loaders_dumpers import load_value from jsonargparse._optionals import pyyaml_available, toml_dump_available, toml_load_available @@ -60,10 +60,129 @@ def test_invalid_parser_mode(): pytest.raises(ValueError, lambda: ArgumentParser(parser_mode="invalid")) -def test_default_parser_mode_json(monkeypatch): - monkeypatch.delenv("JSONARGPARSE_PARSER_MODE", raising=False) +def test_parser_mode_none(): + with pytest.raises(ValueError, match="accepted values for parser_mode"): + ArgumentParser(parser_mode=None) + + +def test_default_parser_mode_json_or_yaml(): code = "from jsonargparse import ArgumentParser; print(ArgumentParser().parser_mode)" - assert subprocess.check_output([sys.executable, "-c", code], text=True) == "json\n" + out = subprocess.check_output([sys.executable, "-c", code], text=True) + assert out == "json_or_yaml\n" + + +# json_or_yaml parser mode + + +def test_json_or_yaml_loads_json(parser): + parser.parser_mode = "json_or_yaml" + parser.add_argument("--cfg", action="config") + parser.add_argument("--list", type=List[str]) + parser.add_argument("--flag", type=bool) + cfg = parser.parse_args(['--cfg={"list": ["a", "b"], "flag": true}']) + assert cfg.list == ["a", "b"] + assert cfg.flag is True + + +@skip_if_no_pyyaml +def test_json_or_yaml_loads_yaml(parser, tmp_cwd): + parser.parser_mode = "json_or_yaml" + parser.add_argument("--cfg", action="config") + parser.add_argument("--list", type=List[str]) + parser.add_argument("--flag", type=bool) + Path("cfg.yaml").write_text("list: [a, b]\n") + cfg = parser.parse_args(["--cfg=cfg.yaml", "--flag=yes"]) + assert cfg.list == ["a", "b"] + assert cfg.flag is True + + +@skip_if_no_pyyaml +def test_json_or_yaml_both_fail_error(parser): + parser.parser_mode = "json_or_yaml" + parser.add_argument("--cfg", action="config") + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(['--cfg={"a": [1}']) + ctx.match("Failed to parse as JSON: Expecting ',' delimiter") + ctx.match("Failed to parse as YAML: while parsing a flow sequence") + + +@pytest.mark.skipif(pyyaml_available, reason="PyYAML package should not be installed") +def test_json_or_yaml_without_pyyaml_error(parser): + parser.parser_mode = "json_or_yaml" + parser.add_argument("--cfg", action="config") + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(["--cfg=a: 1"]) + ctx.match("Failed to parse as JSON: Expecting value") + + +def test_json_or_yaml_dump(parser): + parser.parser_mode = "json_or_yaml" + parser.add_argument("--int", type=int, default=1) + expected = "int: 1\n" if pyyaml_available else '{\n "int": 1\n}\n' + assert parser.dump(parser.get_defaults()) == expected + + +# load error messages + + +@skip_if_no_pyyaml +def test_json_mode_config_file_yaml_error(parser, tmp_cwd): + parser.parser_mode = "json" + parser.add_argument("--cfg", action="config") + parser.add_argument("--a", type=int) + Path("cfg.yaml").write_text("a: 1\n") + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(["--cfg=cfg.yaml"]) + ctx.match("Failed to parse as JSON: Expecting value") + ctx.match("parses as YAML, to accept it use parser_mode='json_or_yaml'") + + +@skip_if_no_pyyaml +def test_json_mode_list_yaml_error(parser): + parser.parser_mode = "json" + parser.add_argument("--list", type=List[str]) + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(["--list=[a, b]"]) + ctx.match("Failed to parse as JSON: Expecting value") + ctx.match("parses as YAML") + + +def test_json_mode_invalid_error_no_yaml_hint(parser): + parser.parser_mode = "json" + parser.add_argument("--cfg", action="config") + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(['--cfg={"a": [1}']) + ctx.match("Failed to parse as JSON: Expecting ',' delimiter") + assert "YAML" not in str(ctx.value) + + +def test_config_path_not_found_error(parser, tmp_cwd): + parser.parser_mode = "json" + parser.add_argument("--cfg", action="config") + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(["--cfg=missing.json"]) + ctx.match("Not a valid path nor a config string") + ctx.match("missing.json") + ctx.match("Failed to parse as JSON") + + +@skip_if_no_pyyaml +def test_yaml_mode_error(parser): + parser.parser_mode = "yaml" + parser.add_argument("--list", type=List[int]) + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(["--list=[1, 2"]) + ctx.match("Failed to parse as YAML: while parsing a flow sequence") + + +def test_custom_mode_error(parser): + parser.add_argument("--cfg", action="config") + with patch.dict("jsonargparse._loaders_dumpers.loaders"): + set_loader("custom", json.loads, (json.JSONDecodeError,)) + parser.parser_mode = "custom" + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(["--cfg={x"]) + ctx.match("Failed to parse with parser_mode='custom': Expecting property name") @pytest.mark.skipif(pyyaml_available, reason="PyYAML package should not be installed") @@ -235,3 +354,12 @@ def test_toml_print_config(parser): parser.add_argument("--group.child2", type=List[float], default=[3.0, 4.5]) out = get_parse_args_stdout(parser, ["--print_config"]) assert out.strip() == toml_config.strip() + + +@pytest.mark.skipif(not toml_load_available, reason="tomllib or tomli package is required") +def test_toml_parse_error(parser): + parser.parser_mode = "toml" + parser.add_argument("--cfg", action="config") + with pytest.raises(ArgumentError) as ctx: + parser.parse_args(["--cfg=a = "]) + ctx.match("Failed to parse as TOML") diff --git a/jsonargparse_tests/test_parsing_settings.py b/jsonargparse_tests/test_parsing_settings.py index b9ac65f9..48a64f6c 100644 --- a/jsonargparse_tests/test_parsing_settings.py +++ b/jsonargparse_tests/test_parsing_settings.py @@ -15,7 +15,6 @@ get_parse_args_stdout, get_parser_help, json_or_yaml_load, - skip_if_no_pyyaml, ) from jsonargparse_tests.test_typehints import Optimizer @@ -77,57 +76,6 @@ def test_allow_abbrev_invalid(parser): parser.allow_abbrev = "invalid" -# parser_mode - - -def test_set_parser_mode_failure(): - with pytest.raises(ValueError, match="accepted values for parser_mode"): - set_parsing_settings(parser_mode="invalid") - - -@skip_if_no_pyyaml -def test_parser_mode_setting(parser): - parser.add_argument("--list", type=List[str]) - set_parsing_settings(parser_mode="json") - assert parser.parser_mode == "json" - with pytest.raises(ArgumentError, match='key "list"'): - parser.parse_args(["--list=[a, b]"]) - set_parsing_settings(parser_mode="yaml") - assert parser.parse_args(["--list=[a, b]"]).list == ["a", "b"] - - -@skip_if_no_pyyaml -def test_parser_mode_setting_subcommands(parser, subparser): - parser.add_subcommands().add_subcommand("sub", subparser) - set_parsing_settings(parser_mode="json") - assert subparser.parser_mode == "json" - set_parsing_settings(parser_mode="yaml") - assert subparser.parser_mode == "yaml" - - -@skip_if_no_pyyaml -def test_parser_mode_env_var(parser, monkeypatch): - set_parsing_settings(parser_mode="json") - monkeypatch.setenv("JSONARGPARSE_PARSER_MODE", "yaml") - assert parser.parser_mode == "yaml" - - -def test_parser_mode_env_var_invalid(parser, monkeypatch): - monkeypatch.setenv("JSONARGPARSE_PARSER_MODE", "invalid") - with pytest.raises(ValueError, match="JSONARGPARSE_PARSER_MODE"): - parser.parser_mode - - -@skip_if_no_pyyaml -def test_parser_mode_parser_takes_precedence(monkeypatch): - set_parsing_settings(parser_mode="yaml") - monkeypatch.setenv("JSONARGPARSE_PARSER_MODE", "yaml") - parser = ArgumentParser(parser_mode="json") - assert parser.parser_mode == "json" - parser.parser_mode = None - assert parser.parser_mode == "yaml" - - # validate_defaults diff --git a/jsonargparse_tests/test_provenance.py b/jsonargparse_tests/test_provenance.py index 6115a9c6..74968acb 100644 --- a/jsonargparse_tests/test_provenance.py +++ b/jsonargparse_tests/test_provenance.py @@ -501,6 +501,19 @@ def test_list_and_dict_values(parser, tmp_cwd): } +@skip_if_no_pyyaml +def test_json_or_yaml_line_numbers(tmp_cwd): + parser = ArgumentParser(exit_on_error=False, parser_mode="json_or_yaml") + parser.add_argument("--config", action="config") + parser.add_argument("--a", type=int) + parser.add_argument("--b", type=int) + Path("cfg.yaml").write_text("a: 1\nb: 2\n") + assert get_sources(parser.parse_args(["--config=cfg.yaml"])) == { + "a": "config file cfg.yaml:1", + "b": "config file cfg.yaml:2", + } + + @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+") @@ -829,6 +842,15 @@ def test_error_syntax_without_line_number(yaml_parser, tmp_cwd): assert get_error(yaml_parser, ["--config=cfg.yaml"]).endswith("\n Source: config file cfg.yaml") +@skip_if_no_pyyaml +def test_error_json_config_not_composable_as_yaml(tmp_cwd): + parser = ArgumentParser(exit_on_error=False, parser_mode="json_or_yaml") + parser.add_argument("--config", action="config") + parser.add_argument("--val", type=int, default=0) + Path("cfg.json").write_text('{"val": "abc"}\n\t\n') # valid json, but yaml rejects the tab + assert get_error(parser, ["--config=cfg.json"]).endswith("Got value: abc\n Source: config file cfg.json") + + @skip_if_no_pyyaml def test_dump_without_provenance(yaml_parser): yaml_parser.add_argument("--val", type=int, default=1) diff --git a/sphinx/conf.py b/sphinx/conf.py index e8c8ed79..a01766a0 100644 --- a/sphinx/conf.py +++ b/sphinx/conf.py @@ -89,8 +89,6 @@ def check_output(self, want, got, optionflags): from jsonargparse.typing import * from jsonargparse._util import unresolvable_import_paths -set_parsing_settings(parser_mode="yaml") # documentation examples use yaml - def doctest_mock_class_in_main(cls): cls.__module__ = None setattr(sys.modules["__main__"], cls.__name__, cls) diff --git a/sphinx/migrate_v5.rst b/sphinx/migrate_v5.rst index 9aaf2132..9bed4e0b 100644 --- a/sphinx/migrate_v5.rst +++ b/sphinx/migrate_v5.rst @@ -35,14 +35,12 @@ changes: - **``pyyaml`` is no longer a required dependency.** Gives no deprecation warning. If your code imports ``yaml``, or you want YAML configs, install with the ``yaml`` extra (``pip install "jsonargparse[yaml]"``). -- **``parser_mode`` defaults to ``json``.** Gives no deprecation warning. Config - files and command line values are loaded as JSON instead of YAML, so YAML - syntax, e.g. ``key: value`` configs, ``--list=[a, b]`` or ``--flag=yes``, - fails. To keep YAML, give ``parser_mode="yaml"`` to the parser, which also - works in v4, or set it globally with - ``set_parsing_settings(parser_mode="yaml")`` or the - ``JSONARGPARSE_PARSER_MODE=yaml`` environment variable. The ``jsonnet`` parser - mode no longer accepts YAML either. +- **``parser_mode`` defaults to ``json_or_yaml``.** Gives no deprecation + warning. Config files and command line values are parsed as JSON, and only if + that fails as YAML, so with the ``yaml`` extra installed YAML keeps working. + Without it, YAML syntax, e.g. ``key: value`` configs or ``--list=[a, b]``, + fails. For strict JSON, give ``parser_mode="json"`` to the parser, which also + works in v4. - **``json`` dumps are now indented.** Gives no deprecation warning. ``dump(format="json")``, ``save`` and the print config argument produce indented JSON instead of a single line. Use ``format="json_compact"`` for the @@ -77,12 +75,13 @@ changes: ``instantiate`` gives a :func:`functools.partial` with the arguments bound, or an :func:`operator.methodcaller` for a method that is called with an instance, instead of the parsed namespace. Call it, e.g. ``init.fn()`` instead of - ``fn(**init.fn)``, or give ``instantiate=False`` to keep the namespace. Warns - only with ``JSONARGPARSE_DEPRECATION_WARNINGS=all``. + ``fn(**init.fn)``, or in v5 give ``instantiate=False`` to keep the namespace. + Warns only with ``JSONARGPARSE_DEPRECATION_WARNINGS=all``. - **Abbreviated options are disabled.** ``allow_abbrev`` defaults to ``False``, so e.g. ``--max`` no longer matches ``--max_epochs``. Use full option names, - or keep abbreviations with ``allow_abbrev=True`` in the parser, or globally - with ``set_parsing_settings(allow_abbrev=True)`` or the + or keep abbreviations with ``allow_abbrev=True`` in the parser, which also + works in v4, or in v5 globally with + ``set_parsing_settings(allow_abbrev=True)`` or the ``JSONARGPARSE_ALLOW_ABBREV=true`` environment variable.