Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
<https://github.com/mauvilsa/jsonargparse/pull/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
<https://github.com/mauvilsa/jsonargparse/pull/978>`__).

Changed
^^^^^^^
Expand Down
7 changes: 4 additions & 3 deletions jsonargparse/_completions_jsonschema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 17 additions & 2 deletions jsonargparse/_subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)]

Expand All @@ -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:
Expand Down
8 changes: 6 additions & 2 deletions jsonargparse_tests/test_completions_jsonschema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand All @@ -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"})
Expand All @@ -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}})

Expand Down
25 changes: 23 additions & 2 deletions jsonargparse_tests/test_subcommands.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,32 @@


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:

Check warning on line 137 in jsonargparse_tests/test_subcommands.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this exception test to have only one invocation possibly throwing an exception.

See more on https://sonarcloud.io/project/issues?id=mauvilsa_jsonargparse&issues=AaCrwstdSxzUcpQ1zB8A&open=AaCrwstdSxzUcpQ1zB8A&pullRequest=978
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()
Expand Down