Open jordan-schneider opened 4 years ago
I agree the API provided by defopt is not so great when multiple commands share the same parameters. The solution I've been thinking about is to support dataclasses, e.g.
@dataclass
class Foo:
x: int
y: float
def somefunc(foo: Foo): ...
def otherfunc(foo: Foo): ...
defopt.run([somefunc, otherfunc])
The main issue how to decide on exact argument name conversion rules (if foo
is positional-or-keyword, do we just splat x
and y
as positional CLI arguments? if foo
is keyword-only, are the flags named --foo-x
/--foo-y
or just --x
/--y
? what if dataclasses support some day keyword-only args (https://bugs.python.org/issue33493)?). I haven't really had the opportunity to think what's best here.
I guess instead of dataclasses we could also support "any class whose constructor is type-hinted in a defopt-compatible way", similarly to how we already support "any class whose constructor takes a single str" (but the latter just drops the name of the parameter in the class constructor, so we're going to have some inconsistencies then...).
I don't know anything about fire, but from a quick look there's a lot of things that I would definitely consider as out of scope (e.g. the "grouping commands" or "accessing properties" or "chaining function calls" sections of the guide). Can you clarify what parts you are exactly looking for?
dataclasses would be an improvement, but I would still have to repeat the setup and teardown. Let me be more specific. I currently have to do this:
def f(x: int, y: str, z: pathlib.Path) -> None:
state = read_state(path)
# Do some processing
write(x, y, path)
def g(x: int, path: pathlib.Path) -> None:
state = read_state(path)
# Do something else
write(x, path)
if __name__ == "__main__":
defopt.run([f, g])
The syntax that fire supports, which removes all this repetition, is
class Command:
def __init__(self, path):
self.state = read_state(path)
self.path = path
def f(x: int, y: str) -> None:
# stufff
def g(x: int) -> None:
# other stuff
def __del__(self):
write(self.state, self.path)
if __name__ == "__main__":
defopt.run(Command)
And then execution would just use both the __init__
arguments and the function arguments
python file.py f --x 5 --y hello --path ~/mypath/myfile
Is that clear?
Thank you for your promptness.
I got that from fire's doc, but if that's what you want, why don't you just use fire? In other words, what benefits do you expect from using defopt instead?
defopt handles list arguments/options sanely. If you want to have a list option in fire, you need to write it as a python style string
In fire:
python file.py --list '[a,b,c]'
In defopt:
python file.py --list a b c
AFAICT the only other API CLI generator that does this well is argh, but argh requires you to manually annotate list parameters.
I guess what you can do right now is something like
from typing import Literal
class Command:
cmd: Literal["foo", "bar"]
x: int
y: float
def __init__(self, cmd, *, x, y):
self.x = x
self.y = y
getattr(self, cmd)()
def foo(self):
print("foo", self.x, self.y)
def bar(self):
print("bar", self.x, self.y)
if __name__ == "__main__":
import defopt
defopt.run(Command)
(which works).
The problem I have with the class-based API is that you're basically writing a class solely for the purpose of providing a command-line API, whereas I feel that defopt's philosophy is that the entire module should still make sense (the functions should be callable with a normal pattern from python code, etc.) if defopt was removed (this should only remove the CLI) (Yes, I know, with class-based APIs, you could write Command(x, y).foo()
but... the API feels clunky). (There's also the additional point that defopt is rather fundamentally oriented around mapping keyword-only args to CLI flags and postitional(-or-keyword) args to positional CLI args.)
(I have switch to using argh because I decided I really wanted the full power of argparse and am willing to pay the cost of writing decorators. Feel free to close this issue if you don't care about setup and teardown.)
Yeah, I can how forcing the user to write a class just for defopt kind of breaks the philosophy of the thing. I think you could get the same results by only using functions if you allowed people to specify multiple sets of functions to dispatch.
def setup(...):
...
def teardown(...):
...
def f(...):
...
def g(...):
...
if __name__ == "__main__"
import defopt
defopt.run(setup)
defopt.run([f, g])
teardown()
And then the CLI would have arguments/flags for both setup
and one of f
or g
. The main difficulty I see here is how to handle getting the results of setup to f or g. You'd have to allow some kind of piping or people would be forced to shove things into global variables.
The case of teardown can also be handled by an atexit handler (although again you'd need globals to pass things around -- but once you're architecting your module for CLI use anyways I don't think globals are that bad as you're effectively assuming a single-use pattern).
I'll leave this open mostly because I would personally like some kind of dataclasses support, but again haven't managed to decide on an API yet.
I just found this library and I love its general approach, also how it extracts docs from the docstring for the argument. Much better approach I feel than typer! But common arguments are still a gap I see and classes would be great to fill it.
You mentioned earlier that there would be issues with deciding on what to name the shared arguments and how to handle them. I think one solution would be to require that all shared arguments are specified before the subcommand (this is how typer does it, just more clunkier with 'callback' and 'ctx' objects). Then all standard rules for functions that you already have apply for the shared args, and the signature of the subcommand doesn´t have to change at all.
Anyway, great library, I will switch to it from using typer as it feels like easier to re-use things.
P.S. Writing a small class just for the sake of defining the API does not feel bad. That can be kept very short.
Passing common arguments before the subcommand is an interesting idea. Can you be more explicit as to the API you propose, though?
All of my commands have some common setup and teardown. One paradigm for this is to have a class with an
__init__()
and__del__()
method, and then have each command by a method of that class. fire supports this, for example. I don't know how difficult this would be, but it would save me a lot of repetition if I could pass a class todefopt.run()
instead of a list of functions.