Skip to content

Commit

Permalink
fix: nargs + list as default value (PR #213, fixes #212)
Browse files Browse the repository at this point in the history
  • Loading branch information
neithere committed Dec 25, 2023
2 parents 6d9f96f + 185b9a4 commit 1d03e1b
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 9 deletions.
13 changes: 13 additions & 0 deletions CHANGES.rst
Expand Up @@ -2,6 +2,19 @@
Changelog
~~~~~~~~~

Version 0.30.5 (2023-12-25)
---------------------------

Bugs fixed:

- A combination of `nargs` with a list as default value would lead to the
values coming from CLI being wrapped in another list (issue #212).

Enhancements:

- Argspec guessing: if `nargs` is not specified but the default value
is a list, `nargs="*"` is assumed and passed to argparse.

Version 0.30.4 (2023-11-04)
---------------------------

Expand Down
24 changes: 21 additions & 3 deletions docs/source/cookbook.rst
Expand Up @@ -10,10 +10,11 @@ Use `nargs` from argparse by amending the function signature with the
.. code-block:: python
@argh.arg("-p", "--patterns", nargs="*")
def cmd(patterns: list[str] | None = None) -> list:
def cmd(*, patterns: list[str] | None = None) -> list:
distros = ("abc", "xyz")
return [d for d in distros if not patterns
or any(p in d for p in patterns)]
return [
d for d in distros if not patterns or any(p in d for p in patterns)
]
Resulting CLI::

Expand All @@ -34,3 +35,20 @@ Resulting CLI::

Note that you need to specify both short and long names of the argument because
`@arg` turns off the "smart" mechanism.

In fact, you don't even need to use `nargs` if you specify a list as the
default value (and provided that you're using the name mapping policy which
will be eventually the default one):

.. code-block:: python
def cmd(patterns: list[str] = ["default-pattern"]) -> list:
distros = ("abc", "xyz")
return [d for d in distros if any(p in d for p in patterns)]
if __name__ == "__main__":
parser = argh.ArghParser()
parser.set_default_command(
cmd, name_mapping_policy=argh.assembling.NameMappingPolicy.BY_NAME_IF_KWONLY
)
argh.dispatch(parser)
2 changes: 1 addition & 1 deletion pyproject.toml
Expand Up @@ -4,7 +4,7 @@ build-backend = "flit_core.buildapi"

[project]
name = "argh"
version = "0.30.4"
version = "0.30.5"
description = "An unobtrusive argparse wrapper with natural syntax"
readme = "README.rst"
requires-python = ">=3.8"
Expand Down
14 changes: 10 additions & 4 deletions src/argh/assembling.py
Expand Up @@ -277,10 +277,16 @@ def guess_extra_parser_add_argument_spec_kwargs(
# (not applicable to positionals: _StoreAction doesn't accept `nargs`)
guessed["action"] = "store_false" if default_value else "store_true"
elif other_add_parser_kwargs.get("type") is None:
# infer type from default value
# (make sure that action handler supports this keyword)
if other_add_parser_kwargs.get("action", "store") in TYPE_AWARE_ACTIONS:
guessed["type"] = type(default_value)
if isinstance(default_value, (list, tuple)):
if "nargs" not in other_add_parser_kwargs:
# the argument has a default value, so it doesn't have to
# be passed; "zero or more" is a reasonable guess
guessed["nargs"] = ZERO_OR_MORE
else:
# infer type from default value
# (make sure that action handler supports this keyword)
if other_add_parser_kwargs.get("action", "store") in TYPE_AWARE_ACTIONS:
guessed["type"] = type(default_value)

# guess type from choices (first item)
if other_add_parser_kwargs.get("choices") and "type" not in list(guessed) + list(
Expand Down
64 changes: 63 additions & 1 deletion tests/test_regressions.py
Expand Up @@ -3,7 +3,7 @@
~~~~~~~~~~~~~~~~
"""
import sys
from typing import TextIO
from typing import List, Optional, TextIO

import pytest

Expand Down Expand Up @@ -180,3 +180,65 @@ def func(foo_bar):

parser = DebugArghParser()
parser.set_default_command(func)


def test_regression_issue212_orig_use_case():
"""
Issue #212: a combination of nargs with list as default value would result
in a nested list instead of a flat list.
Variation: original use case (default value via decorator).
"""

@argh.arg("paths", nargs="*", default=["one", "two"])
def func(paths: List[str]):
return f"{paths}"

parser = DebugArghParser()
parser.set_default_command(func)

assert run(parser, "").out == "['one', 'two']\n"
assert run(parser, "alpha").out == "['alpha']\n"
assert run(parser, "alpha beta gamma").out == "['alpha', 'beta', 'gamma']\n"


def test_regression_issue212_funcsig_centric_positional():
"""
Issue #212: a combination of nargs with list as default value would result
in a nested list instead of a flat list.
Variation: default value via function signature (positional).
"""

@argh.arg("paths", nargs="*")
def func(paths: Optional[List[str]] = ["one", "two"]):
return f"{paths}"

parser = DebugArghParser()
parser.set_default_command(
func, name_mapping_policy=argh.assembling.NameMappingPolicy.BY_NAME_IF_KWONLY
)

assert run(parser, "").out == "['one', 'two']\n"
assert run(parser, "alpha").out == "['alpha']\n"
assert run(parser, "alpha beta gamma").out == "['alpha', 'beta', 'gamma']\n"


def test_regression_issue212_funcsig_centric_named():
"""
Issue #212: a combination of nargs with list as default value would result
in a nested list instead of a flat list.
Variation: default value via function signature (named).
"""

@argh.arg("--paths", nargs="*")
def func(*, paths: Optional[List[str]] = ["one", "two"]):
return f"{paths}"

parser = DebugArghParser()
parser.set_default_command(func)

assert run(parser, "").out == "['one', 'two']\n"
assert run(parser, "--paths alpha").out == "['alpha']\n"
assert run(parser, "--paths alpha beta gamma").out == "['alpha', 'beta', 'gamma']\n"

0 comments on commit 1d03e1b

Please sign in to comment.