diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cf477e42..6a047044 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -41,6 +41,11 @@ Fixed from the remote, leaking the secret. Types that include ``SecretStr`` in a union now only resolve relative paths locally (`#977 `__). +- Subcommand aliases, given in command line or config, were set in the namespace + instead of the subcommand name, and were accepted by the ``jsonschema`` + completion. A config that has settings for both a subcommand name and one of + its aliases now fails, instead of one of them being silently discarded (`#978 + `__). Changed ^^^^^^^ diff --git a/jsonargparse/_completions_jsonschema.py b/jsonargparse/_completions_jsonschema.py index b5dc9178..481986c4 100644 --- a/jsonargparse/_completions_jsonschema.py +++ b/jsonargparse/_completions_jsonschema.py @@ -336,10 +336,11 @@ def set_dest( def add_subcommands(self, action, schema: dict) -> None: # the subcommand key is never required: it is implied when the config has a single subcommand # block, and when there are several the chosen one can be given as a command line argument - names = list(action._name_parser_map.keys()) + # aliases are left out, only the subcommand names are canonical + subparsers = {n: p for n, p in action._name_parser_map.items() if n == p.subcommand} properties = schema.setdefault("properties", {}) - properties[action.dest] = {"enum": names, "description": subcommand_description} - for name, subparser in action._name_parser_map.items(): + properties[action.dest] = {"enum": list(subparsers), "description": subcommand_description} + for name, subparser in subparsers.items(): subcommand_schema = new_object(subparser.description) self.add_properties(subparser, subcommand_schema) properties[name] = subcommand_schema diff --git a/jsonargparse/_subcommands.py b/jsonargparse/_subcommands.py index 4f4095da..f0dd323c 100644 --- a/jsonargparse/_subcommands.py +++ b/jsonargparse/_subcommands.py @@ -153,13 +153,15 @@ def __call__(self, parser, namespace, values, option_string=None): """Adds subcommand dest and parses subcommand arguments.""" subcommand = values[0] arg_strings = values[1:] + subparser = self._name_parser_map.get(subcommand) + if subparser is not None: + subcommand = subparser.subcommand # replace alias with name # set the parser name namespace[self.dest] = subcommand # parse arguments - if subcommand in self._name_parser_map: - subparser = self._name_parser_map[subcommand] + if subparser is not None: subnamespace = namespace.get(subcommand).clone() if subcommand in namespace else None kwargs = dict(_skip_validation=True, _namespace_as_config=True, **parse_kwargs.get()) namespace[subcommand] = subparser.parse_args(arg_strings, namespace=subnamespace, **kwargs) @@ -184,6 +186,17 @@ def get_subcommands( require_single = single_subcommand.get() and not parsing_defaults.get() + # Replace alias settings keys with subcommand names + for key, subparser in action._name_parser_map.items(): + name = subparser.subcommand + if key != name and isinstance(cfg.get(prefix + key), Namespace): + if isinstance(cfg.get(prefix + name), Namespace): + raise ValueError( + f"Subcommand '{name}' settings given more than once, as '{prefix + name}' and " + f"alias '{prefix + key}'. Only one of the subcommand name or its aliases is accepted." + ) + cfg[prefix + name] = cfg.pop(prefix + key) + # Get subcommand settings keys subcommand_keys = [k for k in action.choices if isinstance(cfg.get(prefix + k), Namespace)] @@ -194,6 +207,8 @@ def get_subcommands( subcommand = cfg[dest] if parsing_defaults.get(): raise NSKeyError(f"A specific subcommand can't be provided in defaults, got '{subcommand}'") + if subcommand in action._name_parser_map: + cfg[dest] = subcommand = action._name_parser_map[subcommand].subcommand elif len(subcommand_keys) > 0 and (fail_no_subcommand or require_single): cfg[dest] = subcommand = subcommand_keys[0] if len(subcommand_keys) > 1: diff --git a/jsonargparse_tests/test_completions_jsonschema.py b/jsonargparse_tests/test_completions_jsonschema.py index 0ed0a6ac..12cab10d 100644 --- a/jsonargparse_tests/test_completions_jsonschema.py +++ b/jsonargparse_tests/test_completions_jsonschema.py @@ -689,9 +689,11 @@ def test_subcommands(parser, subparser, subsubparser): subsubparser.add_argument("--opt", type=int, default=1) subcommands = parser.add_subcommands() subcommands.add_subcommand("cmd1", subparser) - subcommands.add_subcommand("cmd2", subsubparser) + subcommands.add_subcommand("cmd2", subsubparser, aliases=["c2"]) schema = get_schema(parser) + # aliases are not canonical, so only the subcommand names are in the schema assert schema["properties"]["subcommand"]["enum"] == ["cmd1", "cmd2"] + assert "c2" not in schema["properties"] assert "can be omitted" in schema["properties"]["subcommand"]["description"] assert schema["properties"]["cmd1"]["description"] == "The first command." assert schema["properties"]["cmd1"]["properties"]["num"] == {"type": "integer"} @@ -713,7 +715,7 @@ def test_subcommands_validation(parser, subparser, subsubparser): subsubparser.add_argument("--opt", type=int, default=1) subcommands = parser.add_subcommands() subcommands.add_subcommand("cmd1", subparser) - subcommands.add_subcommand("cmd2", subsubparser) + subcommands.add_subcommand("cmd2", subsubparser, aliases=["c2"]) schema = get_schema(parser) validate(schema, {"subcommand": "cmd1", "cmd1": {"num": 1}}) validate(schema, {"subcommand": "cmd2"}) @@ -723,6 +725,8 @@ def test_subcommands_validation(parser, subparser, subsubparser): validate(schema, {}) assert iter_errors(schema, {"subcommand": "cmd1"}) assert iter_errors(schema, {"subcommand": "cmd3"}) + assert iter_errors(schema, {"subcommand": "c2"}) + assert iter_errors(schema, {"c2": {"opt": 2}}) assert iter_errors(schema, {"subcommand": "cmd1", "cmd1": {}}) assert iter_errors(schema, {"cmd1": {"bogus": 1}}) diff --git a/jsonargparse_tests/test_subcommands.py b/jsonargparse_tests/test_subcommands.py index e7fbcedf..7064fec9 100644 --- a/jsonargparse_tests/test_subcommands.py +++ b/jsonargparse_tests/test_subcommands.py @@ -113,11 +113,32 @@ def test_main_subcommands_help(subcommands_parser): def test_subcommands_parse_args_alias(subcommands_parser): - cfg = subcommands_parser.parse_args(["B"]) - assert cfg["subcommand"] == "B" + cfg = subcommands_parser.parse_args(["B", "--nums.val1=3"]) + assert cfg["subcommand"] == "b" + assert cfg["b.nums.val1"] == 3 + assert "B" not in cfg pytest.raises(ArgumentError, lambda: subcommands_parser.parse_args(["A"])) +@pytest.mark.parametrize("config", [{"subcommand": "B"}, {"B": {}}, {"subcommand": "B", "B": {}}]) +def test_subcommands_parse_config_alias(subcommands_parser, config): + subcommands_parser.add_argument("--cfg", action="config") + config = {k: {"nums": {"val1": 3}} if k == "B" else v for k, v in config.items()} + cfg = subcommands_parser.parse_args([f"--cfg={json.dumps(config)}"]) + assert cfg["subcommand"] == "b" + assert cfg["b.nums.val1"] == (3 if "B" in config else 1) + assert "B" not in cfg + + +@pytest.mark.parametrize("config", [{}, {"subcommand": "b"}, {"subcommand": "B"}]) +def test_subcommands_parse_config_alias_collision(subcommands_parser, config): + subcommands_parser.add_argument("--cfg", action="config") + config = {**config, "b": {"nums": {"val1": 2}}, "B": {"nums": {"val1": 3}}} + with pytest.raises(ArgumentError) as ctx: + subcommands_parser.parse_args([f"--cfg={json.dumps(config)}"]) + ctx.match("Subcommand 'b' settings given more than once, as 'b' and alias 'B'") + + def test_subcommands_parse_args_config(subcommands_parser): subcommands_parser.add_argument("--cfg", action="config") cfg = subcommands_parser.parse_args(['--cfg={"o1": "o1_arg"}', "a", "ap1_arg"]).as_dict()