DanCardin / cappa

Declarative CLI argument parser
Apache License 2.0
135 stars 8 forks source link

bug: Cappa tries to pass dataclass classvar as keyword argument #181

Closed pawamoy closed 1 week ago

pawamoy commented 1 week ago
from __future__ import annotations

import sys
import cappa
from dataclasses import dataclass
from typing import ClassVar

@cappa.command(name="command")
@dataclass(kw_only=True)
class Command:
    _CONFIG: ClassVar[str] = "config.toml"

    arg: int = 0

    def __call__(self):
        print("Command called")

def main(args: list[str] | None = None) -> int:
    return cappa.invoke(Command, argv=args)

if __name__ == "__main__":
    sys.exit(main())
Traceback (most recent call last):
  File "/media/data/dev/insiders-project/repro.py", line 26, in <module>
    sys.exit(main())
             ^^^^^^
  File "/media/data/dev/insiders-project/repro.py", line 22, in main
    return cappa.invoke(Command, argv=args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/media/data/dev/insiders-project/.venv/lib/python3.12/site-packages/cappa/base.py", line 131, in invoke
    command, parsed_command, instance, concrete_output = parse_command(
                                                         ^^^^^^^^^^^^^^
  File "/media/data/dev/insiders-project/.venv/lib/python3.12/site-packages/cappa/base.py", line 247, in parse_command
    command, parsed_command, instance = Command.parse_command(
                                        ^^^^^^^^^^^^^^^^^^^^^^
  File "/media/data/dev/insiders-project/.venv/lib/python3.12/site-packages/cappa/command.py", line 188, in parse_command
    result = command.map_result(command, prog, parsed_args)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/media/data/dev/insiders-project/.venv/lib/python3.12/site-packages/cappa/command.py", line 239, in map_result
    return command.cmd_cls(**kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Command.__init__() got an unexpected keyword argument '_CONFIG'
pawamoy commented 1 week ago

Looks like the class variable is shifting everything:

cappa-shift

(look at short flags in the watch pane, and the subcommand being assigned to _CONFIG in the variables pane)

pawamoy commented 1 week ago

My investigation tells me that DataclassField should maybe use dataclasses.fields(typ) instead of typ.__dataclasses_fields__, as the former correctly ignores classvars:

"_CONFIG" in [f.name for f in dataclasses.fields(command.cmd_cls)]    # False
"_CONFIG" in command.cmd_cls.__dataclass_fields__                     # True

https://docs.python.org/3/library/dataclasses.html#dataclasses.fields

Just tried the fix and it works :slightly_smiling_face: If you believe that is a proper solution, I'm happy to send a PR.

DanCardin commented 1 week ago

It seems to pass tests, i cannot for the life of me tell why I would have used that field instead of the fields function...

Yea feel free to send a PR! If it's accompanied by a classvar test and a CHANGELOG entry, feel free to also bump the patch version and i can release it directly. (but i can also follow up with those, or the pr myself. up to you!)

pawamoy commented 1 week ago

Working on it!

pawamoy commented 1 week ago

I'll leave the changelog update and version bump to you this time :smile: