diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6dd3f808..335bbb5c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -12,6 +12,17 @@ The semantic versioning only considers the public API as described in paths are considered internals and can change in minor and patch releases. +v4.44.0 (unreleased) +-------------------- + +Changed +^^^^^^^ +- Improved error messages when not accepted options are given, referencing which + parser/subcommand the option was given to. Also suggests running ``--help`` + for the corresponding parser/subcommand (`#809 + `__). + + v4.43.0 (2025-11-11) -------------------- diff --git a/jsonargparse/_core.py b/jsonargparse/_core.py index 3ed4120b..0dde003a 100644 --- a/jsonargparse/_core.py +++ b/jsonargparse/_core.py @@ -1084,7 +1084,15 @@ def error(self, message: str, ex: Optional[Exception] = None) -> NoReturn: elif debug_mode_active(): self._logger.debug("Debug enabled, thus raising exception instead of exit.") raise argument_error(message) from ex - self.print_usage(sys.stderr) + + parser = getattr(ex, "subcommand_parser", None) or self + parser.print_usage(sys.stderr) + + help_action = next((a for a in parser._actions if isinstance(a, argparse._HelpAction)), None) + if help_action: + prog = parser.prog.replace(" [options]", "") + sys.stderr.write(f"tip: For details of accepted options run: {prog} {help_action.option_strings[-1]}\n") + sys.stderr.write(f"error: {message}\n") self.exit(2) @@ -1123,7 +1131,7 @@ def check_required(cfg, parser, prefix): raise TypeError except (KeyError, TypeError) as ex: raise TypeError( - f'Key "{prefix}{reqkey}" is required but not included in config object or its value is None.' + f"Option '{prefix}{reqkey}' is required but not provided or its value is None." ) from ex subcommand, subparser = _ActionSubCommands.get_subcommand(parser, cfg, fail_no_subcommand=False) if subcommand is not None and subparser is not None: @@ -1156,24 +1164,25 @@ def check_values(cfg): else: if isinstance(parent_action, _ActionSubCommands) and "." in key: subcommand, subkey = split_key_root(key) - raise NSKeyError(f"Subcommand '{subcommand}' does not accept nested key '{subkey}'") + 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 nested key '{subkey}'") - raise NSKeyError(f"Key '{key}' is not expected") + 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") - try: - with parser_context(load_value_mode=self.parser_mode): - check_values(cfg) - if not skip_required and not lenient_check.get(): - check_required(cfg, self, prefix) - except (TypeError, KeyError) as ex: - prefix = "Validation failed: " - message = ex.args[0] - if prefix not in message: - message = prefix + message - raise type(ex)(message) from ex + with parser_context(load_value_mode=self.parser_mode): + check_values(cfg) + if not skip_required and not lenient_check.get(): + check_required(cfg, self, prefix) def add_instantiator( self, diff --git a/jsonargparse/_formatters.py b/jsonargparse/_formatters.py index 79e37269..11f7a316 100644 --- a/jsonargparse/_formatters.py +++ b/jsonargparse/_formatters.py @@ -201,8 +201,9 @@ def _get_help_string(self, action: Action) -> str: help_str += action.extra_help() return action_help + (" (" + help_str + ")" if help_str else "") - def _format_usage(self, *args, **kwargs) -> str: - usage = super()._format_usage(*args, **kwargs) + def _format_usage(self, usage, actions, *args, **kwargs) -> str: + actions = filter_non_parsing_actions(actions) + usage = super()._format_usage(usage, actions, *args, **kwargs) parser = parent_parser.get() if not parser: diff --git a/jsonargparse_tests/conftest.py b/jsonargparse_tests/conftest.py index 80fc5277..aad70abe 100644 --- a/jsonargparse_tests/conftest.py +++ b/jsonargparse_tests/conftest.py @@ -223,7 +223,7 @@ def get_parse_args_stdout(parser: ArgumentParser, args: List[str]) -> str: def get_parse_args_stderr(parser: ArgumentParser, args: List[str]) -> str: err = StringIO() with patch.object(parser, "exit_on_error", return_value=True): - with redirect_stderr(err), pytest.raises(SystemExit): + with patch.dict(os.environ, {"COLUMNS": columns}), redirect_stderr(err), pytest.raises(SystemExit): parser.parse_args(args) return err.getvalue() diff --git a/jsonargparse_tests/test_actions.py b/jsonargparse_tests/test_actions.py index bded4df2..d4ddca40 100644 --- a/jsonargparse_tests/test_actions.py +++ b/jsonargparse_tests/test_actions.py @@ -303,7 +303,7 @@ def test_action_parser_required_argument(parser, subparser): assert "1" == parser.parse_args(["--op2.op1=1"]).op2.op1 with pytest.raises(ArgumentError) as ctx: parser.parse_args([]) - ctx.match('"op2.op1" is required') + ctx.match("'op2.op1' is required") def test_action_parser_init_failures(parser, subparser): diff --git a/jsonargparse_tests/test_cli.py b/jsonargparse_tests/test_cli.py index 00093458..72f552b4 100644 --- a/jsonargparse_tests/test_cli.py +++ b/jsonargparse_tests/test_cli.py @@ -188,7 +188,7 @@ def test_single_class_missing_required_init(): err = StringIO() with redirect_stderr(err), pytest.raises(SystemExit): auto_cli(Class1, args=['--config={"method1": {"m1": 2}}']) - assert '"i1" is required' in err.getvalue() + assert "'i1' is required" in err.getvalue() def test_single_class_invalid_method_parameter(): diff --git a/jsonargparse_tests/test_core.py b/jsonargparse_tests/test_core.py index 1b60fb22..0e264e76 100644 --- a/jsonargparse_tests/test_core.py +++ b/jsonargparse_tests/test_core.py @@ -125,7 +125,7 @@ def test_parse_args_positional_nargs_questionmark(parser): parser.add_argument("pos2", nargs="?") with pytest.raises(ArgumentError) as ctx: parser.parse_args([]) - ctx.match('"pos1" is required') + ctx.match("'pos1' is required") assert parser.parse_args(["v1"]) == Namespace(pos1="v1", pos2=None) assert parser.parse_args(["v1", "v2"]) == Namespace(pos1="v1", pos2="v2") @@ -135,7 +135,7 @@ def test_parse_args_positional_nargs_plus(parser): parser.add_argument("pos2", nargs="+") with pytest.raises(ArgumentError) as ctx: parser.parse_args(["v1"]) - ctx.match('"pos2" is required') + ctx.match("'pos2' is required") assert parser.parse_args(["v1", "v2", "v3"]) == Namespace(pos1="v1", pos2=["v2", "v3"]) @@ -448,7 +448,7 @@ def test_non_positional_required(parser, subtests): with subtests.test("help"): help_str = get_parser_help(parser) - assert "[-h] --req1 REQ1 --lev1.req2 REQ2" in help_str + assert "--req1 REQ1 --lev1.req2 REQ2" in help_str assert "--lev1.req2 REQ2 (required)" in help_str with subtests.test("parse_env"): diff --git a/jsonargparse_tests/test_dataclasses.py b/jsonargparse_tests/test_dataclasses.py index d2a05be5..63b9414f 100644 --- a/jsonargparse_tests/test_dataclasses.py +++ b/jsonargparse_tests/test_dataclasses.py @@ -244,7 +244,7 @@ def test_add_argument_dataclass_unexpected_keys(parser): invalid = { "class_path": f"{__name__}.DataClassB", } - with pytest.raises(ArgumentError, match="Group 'b' does not accept nested key 'class_path'"): + with pytest.raises(ArgumentError, match="Group 'b' does not accept option 'class_path'"): parser.parse_args([f"--b={json.dumps(invalid)}"]) @@ -257,7 +257,7 @@ class DataRequiredAttr: def test_add_argument_dataclass_type_required_attr(parser): parser.add_argument("--b", type=DataRequiredAttr) assert Namespace(a1="v", a2=1.2) == parser.parse_args(["--b.a1=v"]).b - with pytest.raises(ArgumentError, match='Key "b.a1" is required'): + with pytest.raises(ArgumentError, match="Option 'b.a1' is required"): parser.parse_args([]) @@ -744,7 +744,7 @@ def test_dataclass_not_subclass(parser): assert "--data.help" not in help_str config = {"class_path": f"{__name__}.DataSub", "init_args": {"p2": "y"}} - with pytest.raises(ArgumentError, match="Group 'data' does not accept nested key 'init_args.p2'"): + with pytest.raises(ArgumentError, match="Group 'data' does not accept option 'init_args.p2'"): parser.parse_args([f"--data={json.dumps(config)}"]) @@ -854,7 +854,7 @@ def test_dataclass_nested_not_subclass(parser): } }, } - with pytest.raises(ArgumentError, match="Group 'data' does not accept nested key 'init_args.p1'"): + with pytest.raises(ArgumentError, match="Group 'data' does not accept option 'init_args.p1'"): parser.parse_args([f"--parent={json.dumps(config)}"]) diff --git a/jsonargparse_tests/test_formatters.py b/jsonargparse_tests/test_formatters.py index 0effd0f0..0145ae4f 100644 --- a/jsonargparse_tests/test_formatters.py +++ b/jsonargparse_tests/test_formatters.py @@ -170,4 +170,4 @@ def test_help_subcommands_with_default_env(parser): def test_format_usage(parser): parser.add_argument("--v1") with patch.dict(os.environ, {"COLUMNS": "200"}): - assert parser.format_usage() == "usage: app [-h] [--v1 V1]\n" + assert parser.format_usage() == "usage: app [--v1 V1]\n" diff --git a/jsonargparse_tests/test_signatures.py b/jsonargparse_tests/test_signatures.py index 91aefc80..656afd5f 100644 --- a/jsonargparse_tests/test_signatures.py +++ b/jsonargparse_tests/test_signatures.py @@ -124,7 +124,7 @@ def test_add_class_without_nesting(parser): with pytest.raises(ArgumentError) as ctx: parser.parse_args([]) - ctx.match('"c3_a0" is required') + ctx.match("Option 'c3_a0' is required") if docstring_parser_support: assert "Class3 short description" == parser.groups["Class3"].title @@ -196,7 +196,7 @@ def test_add_class_without_parameters(parser): config = {"no_params": {"class_path": f"{__name__}.NoParams"}} with pytest.raises(ArgumentError) as ctx: parser.parse_args([f"--cfg={json.dumps(config)}"]) - ctx.match("Group 'no_params' does not accept nested key 'class_path'") + ctx.match("Group 'no_params' does not accept option 'class_path'") class NestedWithParams: @@ -710,5 +710,5 @@ def test_add_function_positional_and_keyword_only_parameters(parser): assert cfg == Namespace(a=1, b=2, c=3, d=4) with pytest.raises(ArgumentError, match="Unrecognized arguments: --b=2"): parser.parse_args(["1", "--b=2", "--c=3", "--d=4"]) - with pytest.raises(ArgumentError, match='Key "c" is required'): + with pytest.raises(ArgumentError, match="Option 'c' is required"): parser.parse_args(["1", "2", "--d=4"]) diff --git a/jsonargparse_tests/test_subclasses.py b/jsonargparse_tests/test_subclasses.py index d4a78246..025e0910 100644 --- a/jsonargparse_tests/test_subclasses.py +++ b/jsonargparse_tests/test_subclasses.py @@ -364,10 +364,10 @@ def test_class_method_instantiator(parser): with pytest.raises(ArgumentError) as ctx: parser.parse_args([f"--cls={__name__}.ClassMethodInstantiator.from_p1"]) - ctx.match('Key "p1" is required') + ctx.match("Option 'p1' is required") with pytest.raises(ArgumentError) as ctx: parser.parse_args([f"--cls={__name__}.ClassMethodInstantiator.from_p1", "--cls.p1=2", "--cls.p3=-"]) - ctx.match("Key 'p3' is not expected") + ctx.match("Option 'p3' is not accepted") class FunctionInstantiator: @@ -392,7 +392,7 @@ def test_function_instantiator(parser): with pytest.raises(ArgumentError) as ctx: parser.parse_args([f"--cls={__name__}.function_instantiator", "--cls.p2=y", "--cls.p3=x"]) - ctx.match("Key 'p3' is not expected") + ctx.match("Option 'p3' is not accepted") def function_undefined_return(p1: int) -> "Undefined": # type: ignore[name-defined] # noqa: F821 @@ -663,7 +663,7 @@ def test_subclass_class_name_then_invalid_init_args(parser): parser.add_argument("--op", type=Union[Calendar, GzipFile]) with pytest.raises(ArgumentError) as ctx: parser.parse_args(["--op=TextCalendar", "--op=GzipFile", "--op.firstweekday=2"]) - ctx.match("Key 'firstweekday' is not expected") + ctx.match("Option 'firstweekday' is not accepted") # dict parameter tests @@ -1378,7 +1378,8 @@ def test_add_subclass_required_group(parser): parser.add_subclass_arguments(Calendar, "cal", required=True) pytest.raises(ArgumentError, lambda: parser.parse_args([])) help_str = get_parser_help(parser) - assert "[-h] [--cal.help [CLASS_PATH_OR_NAME]] --cal " in help_str + assert "--cal.help [CLASS_PATH_OR_NAME]" in help_str + assert "--cal CONFIG | CLASS_PATH_OR_NAME | .INIT_ARG_NAME VALUE" in help_str def test_add_subclass_not_required_group(parser): @@ -1676,7 +1677,7 @@ def test_subclass_print_config(parser): assert obtained == {"a1": {"class_path": "calendar.Calendar", "init_args": {"firstweekday": 0}}, "a2": 7} err = get_parse_args_stderr(parser, ["--g.a1=calendar.Calendar", "--g.a1.invalid=1", "--print_config"]) - assert "Key 'invalid' is not expected" in err + assert "Option 'invalid' is not accepted" in err class PrintConfigRequired: @@ -1754,7 +1755,7 @@ def test_subclass_error_unexpected_init_arg(parser): init_args = '"init_args": {"unexpected_arg": true}' with pytest.raises(ArgumentError) as ctx: parser.parse_args(["--op={" + class_path + ", " + init_args + "}"]) - ctx.match("Key 'unexpected_arg' is not expected") + ctx.match("Option 'unexpected_arg' is not accepted") def test_subclass_invalid_class_path_value(parser): @@ -1823,7 +1824,7 @@ def test_subclass_implicit_class_path(parser): assert cfg.implicit.init_args == Namespace(a=3, b="") with pytest.raises(ArgumentError) as ctx: parser.parse_args(['--implicit={"c": null}']) - ctx.match("Key 'c' is not expected") + ctx.match("Option 'c' is not accepted") # error messages tests diff --git a/jsonargparse_tests/test_subcommands.py b/jsonargparse_tests/test_subcommands.py index 77c383b5..9df470f1 100644 --- a/jsonargparse_tests/test_subcommands.py +++ b/jsonargparse_tests/test_subcommands.py @@ -71,7 +71,7 @@ def test_subcommands_not_given_when_many_subcommands(parser, subparser): def test_subcommands_missing_required_subargument(subcommands_parser): - with pytest.raises(ArgumentError, match='Key "a.ap1" is required'): + with pytest.raises(ArgumentError, match="Option 'a.ap1' is required"): subcommands_parser.parse_args(["a"]) @@ -118,13 +118,42 @@ def test_subcommands_parse_args_config(subcommands_parser): } +def test_subcommands_parse_args_error_config_before_subcommand(parser, subparser): + subparser.add_argument("--sp1") + subcommands = parser.add_subcommands(required=False) + subcommands.add_subcommand("a", subparser) + parser.add_argument("--cfg", action="config") + parser.prog = "app" + parser.exit_on_error = True + err = get_parse_args_stderr(parser, ['--cfg={"sp1": "x"}', "a"]) + assert "Option 'sp1' is not accepted before subcommand 'a'" in err + assert "For details of accepted options run: app --help" in err + err = get_parse_args_stderr(parser, ['--cfg={"sp1": "x"}']) + assert "Option 'sp1' is not accepted before subcommand {a,...}" in err + assert "For details of accepted options run: app --help" in err + assert "usage: app [--cfg CFG] {a} ..." in err + + +def test_subcommands_parse_args_error_config_after_subcommand(parser, subparser): + parser.prog = "app" + parser.exit_on_error = True + subparser.add_argument("--sp1") + subcommands = parser.add_subcommands(required=False) + subcommands.add_subcommand("a", subparser) + parser.add_argument("--cfg", action="config") + err = get_parse_args_stderr(parser, ['--cfg={"a": {"sp2": "x"}}']) + assert "Subcommand 'a' does not accept option 'sp2'" in err + assert "For details of accepted options run: app a --help" in err + assert "usage: app [options] a [--sp1 SP1]" in err + + def test_subcommands_parse_string_implicit_subcommand(subcommands_parser): cfg = subcommands_parser.parse_string('{"a": {"ap1": "ap1_cfg"}}').as_dict() assert cfg["subcommand"] == "a" assert cfg["a"] == {"ap1": "ap1_cfg", "ao1": "ao1_def"} with pytest.raises(ArgumentError) as ctx: subcommands_parser.parse_string('{"a": {"ap1": "ap1_cfg", "unk": "unk_cfg"}}') - ctx.match("Subcommand 'a' does not accept nested key 'unk'") + ctx.match("Subcommand 'a' does not accept option 'unk'") def test_subcommands_parse_string_first_implicit_subcommand(subcommands_parser):