Repository: DanCardin/cappa

Issue:
bug: Positional argument with inferred num_args=-1 doesn't call default
```python
from dataclasses import dataclass
from typing import Annotated

import cappa


def never_called():
    print("CALLED!")
    return [42]


@cappa.command(name="main")
@dataclass
class Command:
    positional: Annotated[
        list[str],
        cappa.Arg(
            short=False,
            long=False,
            default=cappa.ValueFrom(never_called),
        ),
    ]

    def __call__(self):
        print(self.positional)


cappa.invoke(Command)
```

```console
% python repro.py
[]
```

(I was expecting `CALLED!` and `[42]` here :point_up:)

```console
% python repro.py 7
['7']
```

If I set `required=True` on the arg:

```console
% python repro.py
Usage: main POSITIONAL ... [-h] [--completion COMPLETION]

Error: Argument 'positional' requires at least one values, found 0
```

If I set `num_args=1`, it's better, but then I can only pass *one* argument, not more:

```console
% python repro.py
CALLED!
[42]
```

```console
% python repro.py 10
['10']
```

```console
% python repro.py 10 11
Usage: main [POSITIONAL] [-h] [--completion COMPLETION]

Error: Unrecognized arguments: 11
```

Hints from issue comments:
@DanCardin: Somewhat amusingly, this fails in both argparse and cappa parsers for different reasons.

cappa is the reason you see above. i should be able to early return for unbounded args to avoid the action.

but in argparse, this is the only case that seems to trigger a need for `num_args='*'`. it doesn't respect defaults when using `'+'`, which i've had hardcoded as the only option for lists (and just dealt with optionality elsewhere).

---

It's actually also a problem in both cases if you do `tuple[str, str]`, although argparse doesnt appear to support optional num_args=2...? I guess you must just be forced to add two individually optional positional arguments and combine them yourself. @pawamoy: So did we find a bug in `argparse` :thinking:?

Man, parsing arguments is such a complex thing to do right :sweat_smile: Thank you so much for all the fixes :bow: @DanCardin: Idk that it's a bug in argparse so much as it's got relatively inflexible error handling, and (for all that it **is** fairly flexible generally), it is (imo) overly strict about the allowed inputs when using certain features. Like `nargs=2` will not allow you to specify `required=False`, and a number of other combinations of inputs like that that would be useful to cappa for simplicity.

In this case, if you reported a bug, i assume they'd just say to use two separate optional positional arguments rather than a nargs=2.

> Man, parsing arguments is such a complex thing to do right 😅 Thank you so much for all the fixes 🙇

Heh, you're telling me. the native cappa parser started out at probably half it's current length. Thank **you** for providing all the edge cases :D

Solution patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5f7bfd8..00a115f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## 0.26
 
+### 0.26.2
+
+- fix: Optional positional arguments should not get a default value from their action.
+
 ### 0.26.1
 
 - fix: Default formatting when value is None/Empty and formatter is supplied.
diff --git a/pyproject.toml b/pyproject.toml
index 4d171e4..2e90147 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,6 +1,6 @@
 [project]
 name = "cappa"
-version = "0.26.1"
+version = "0.26.2"
 description = "Declarative CLI argument parser."
 
 urls = {repository = "https://github.com/dancardin/cappa"}
diff --git a/src/cappa/argparse.py b/src/cappa/argparse.py
index 0531ebf..7136842 100644
--- a/src/cappa/argparse.py
+++ b/src/cappa/argparse.py
@@ -229,7 +229,7 @@ def add_argument(
 
     is_positional = not names
 
-    num_args = backend_num_args(arg.num_args)
+    num_args = backend_num_args(arg.num_args, assert_type(arg.required, bool))
 
     kwargs: dict[str, typing.Any] = {
         "dest": dest_prefix + assert_type(arg.field_name, str),
@@ -295,12 +295,14 @@ def add_subcommands(
         )
 
 
-def backend_num_args(num_args: int | None) -> int | str | None:
+def backend_num_args(num_args: int | None, required: bool) -> int | str | None:
     if num_args is None or num_args == 1:
         return None
 
     if num_args == -1:
-        return "+"
+        if required:
+            return "+"
+        return "*"
 
     return num_args
 
diff --git a/src/cappa/parser.py b/src/cappa/parser.py
index 3fead99..808fa81 100644
--- a/src/cappa/parser.py
+++ b/src/cappa/parser.py
@@ -597,13 +597,16 @@ def consume_arg(
                 arg=arg,
             )
     else:
-        missing_arg_requirement = (
-            # Positive fixed-arg amount
-            (orig_num_args > 0 and len(result) != orig_num_args)
-            # Unbounded but required arg
-            or (orig_num_args < 0 and arg.required and not result)
-        )
-        if missing_arg_requirement:
+        required_arg = arg.required
+        fixed_arg_mismatch = orig_num_args > 0 and len(result) != orig_num_args
+        unbounded_missing = orig_num_args < 0 and not result
+        if fixed_arg_mismatch or unbounded_missing:
+            option_requiring_values = option and requires_values
+            if not (required_arg or option_requiring_values or result):
+                # Lacking a consumed value for an optional positional arg should avoid
+                # hitting the argument's action, so as to apply it's default handling.
+                return
+
             quoted_result = [f"'{r}'" for r in result]
             names_str = arg.names_str("/")
 


Test patch:
diff --git a/tests/arg/test_default_value_from.py b/tests/arg/test_default_value_from.py
index 66d606d..7ed943f 100644
--- a/tests/arg/test_default_value_from.py
+++ b/tests/arg/test_default_value_from.py
@@ -51,3 +51,50 @@ def test_not_parsed(backend):
 
     result = parse(Command, backend=backend)
     assert result == Command(0)
+
+
+def called():
+    return ["42"]
+
+
+@backends
+def test_positional_unbound_num_args(backend):
+    @dataclass
+    class Command:
+        positional: Annotated[
+            list[str],
+            cappa.Arg(
+                short=False,
+                long=False,
+                default=cappa.ValueFrom(called),
+            ),
+        ]
+
+    result = parse(Command, "7", backend=backend)
+    assert result == Command(["7"])
+
+    result = parse(Command, backend=backend)
+    assert result == Command(["42"])
+
+
+def two_tuple():
+    return ("42", "0")
+
+
+def test_optional_positional_2_tuple():
+    @dataclass
+    class Command:
+        positional: Annotated[
+            tuple[str, str],
+            cappa.Arg(
+                short=False,
+                long=False,
+                default=cappa.ValueFrom(two_tuple),
+            ),
+        ]
+
+    result = parse(Command, "7", "8")
+    assert result == Command(("7", "8"))
+
+    result = parse(Command)
+    assert result == Command(("42", "0"))