larryhastings / appeal

Command-line parsing library for Python 3.
Other
122 stars 7 forks source link

Passing global options to commands #4

Closed encukou closed 2 years ago

encukou commented 2 years ago

Hello,

Say I want to handle the command line from the README example:

% ./script.py --debug add --flag ro -v -xz myfile.txt

What's the intended way for the add command get info about the --debug option?

(Is too early to ask questions like this?)

larryhastings commented 2 years ago

What's the intended way for the add command get info about the --debug option?

I consider that out-of-band for Appeal. The obvious way is to store it in some global state somewhere. That, or, call app.command on bound methods of an instance of a user class, and store the information gleaned from global commands in that instance.

Fundamentally, Appeal is something you interact with once per process; you wouldn't expect to process multiple command-lines in a single process, much less multiple command-lines simultaneously.

(That said, I am working to clean up where Appeal stores its information, permitting you to re-use an Appeal object to parse more than one command-line. Hopefully I could even permit multiple simultaneous uses of an Appeal object. I don't expect users to need this, I just think it's good runtime hygiene. And it'll be helpful for testing.)

If you think Appeal needs a mechanism for passing in / exposing a user context object, feel free to propose something. What would it look like?

One idea off the top of my head: Appeal could add an AppealContext abstract base class. You subclass it, instantiate it, and assign the instance to app.context. If your command function takes a parameter annotated with a type that is a subclass of AppealContext, Appeal passes in app.context.

(Is too early to ask questions like this?)

Not at all. The most important thing is to figure out the API.

encukou commented 2 years ago

I consider that out-of-band for Appeal.

To me it seems like global options aren't very useful without it. It might be orthogonal to Appeal, but at least a "best practice" example (or a few hints) in the docs would be helpful.

What would it look like?

One idea did occur to me, though I don't know how it would mesh with the rest of the API: the super-command could return a (non-int) object, and the subcommand could receive it in a specially annotated argument.

larryhastings commented 2 years ago

To me it seems like global options aren't very useful without it.

I generally use global options to set global state. Either that, or, I'm writing a command that doesn't have any (sub-)commands, so all my logic is in the global command function anyway.

One idea did occur to me, though I don't know how it would mesh with the rest of the API: the super-command could return a (non-int) object, and the subcommand could receive it in a specially annotated argument.

The return value of every "command" is a process return code. A false value indicates success, and any true value stops processing and exits immediately. Appeal.process() will return that value; Appeal.main() will pass it into sys.exit().

larryhastings commented 2 years ago

I prototyped this and got it working. There's an AppealContext ABC, and a context argument on Appeal.main() and Appeal.process(). If a converter takes a parameter annotated with AppealContext or a subclass (etc), that parameter will sidestep the usual Appeal processing, and instead Appeal will simply pass in the context argument when calling that function. That works.

I ran into some trouble though. Appeal already had some fuzzy wishful thinking when dealing with with self parameters. Appeal mostly ignored them, assuming that they'd get filled in later... somehow. But if you annotated a self parameter with AppealContext--a pretty obvious use case, and probably the use case you originally had in mind--suddenly Appeal wanted both to see the parameter and ignore the parameter, depending on the context. I got it to run, but my hacks got hackier.

Also, in the back of my mind, I could forsee users needing more than one "context". I had been telling myself YAGNI, but I could still see how different contexts could be useful. For example, someone might want a main class to represent their app overall, but also use a child class to encapsulate all the logic for a particular sub-command. This would be possible with a single context--that gives you the lever and the place to stand, so to speak--but it would be clumsy. You'd be working around Appeal, not with it.

So after a long re-think on how Appeal should approach this, I've hit on a new approach that I think I'll prefer, as follows:

First, Appeal is getting out of the business of ignoring self. Appeal will process every argument it sees--"Special cases aren't special enough to break the rules." (I never liked this anyway; it was simultaneously wishy-washy and too magical. I want a bare minimum of magic baked in to Appeal.)

Second, we still need a nice way to use bound methods as Appeal commands. You could do this today, of course; just create your class, instantiate it, then call app.command() on the bound methods. But this is a pain. What we want is a mechanism that lets us pass in a late-binding self parameter, and for Appeal to otherwise ignore the parameter.

I think this sounds like a job for functools.partial. This lets us sidestep Appeal processing the parameter. The problem is that it isn't late-binding. We have to bind the parameter to something, but if this is before we wanted to create our instance, now we have a chicken-and-egg problem.

That's not hard to solve though. It's easy to rebind partial objects. Here's some code that does just that:

def partial_replace_map(partial, map):
    func = partial.func
    if isinstance(func, functools.partial):
        func = partial_replace_map(func, map)
    new_args = []
    for value in partial.args:
        new_args.append(map.get(value, value))
    new_kwargs = {}
    for key, value in partial.keywords.items():
        new_kwargs[key] = map.get(value, value)
    return functools.partial(func, *new_args, **new_kwargs)

def partial_replace(partial, **kwargs):
    return partial_replace_map(partial, kwargs)

If you call partial_replace(partialfn, old=new), every place where partialfn had a parameter bound to old, it will be rebound with new. The idea here is, we create a sentinel value as a placeholder for the self value, then we later use partial_replace(partial_bound_method, sentinel=instance) to swap in the actual instance for the placeholder.

All we need now is for Appeal to cooperate with this. I'm gonna call it the prepare step. You pass in a function to Appeal.main or Appeal.process, and Appeal calls that function, passing in each command function and converter, and calls the result. (Note that this prepare function isn't allowed to change the signature of the function.)

Finally, we add a helper to make it easy in the general case--the user has one app class, and wants to substitute in one self value.

So the workflow when using a class should look something like this:

import appeal

app = appeal.Appeal()
my_app_command = app.method_command()

class MyApp:
    @my_app_command()
    def add(self, name, value): ...

    @my_app_command()
    def remove(self, name, value): ...

my_app = MyApp()
app.main(prepare=my_app_command.prepare(my_app))

The app.method_command() object would perform three functions:

You aren't required to use app.method_command() though. That will be a specific object implementing a defined protocol, and you could write your own objects conforming to that protocol, to handle things like multiple substitutions. (Appeal could maybe provide that too.)

Certainly this should satisfy your original use case, as stated in the message that opened the issue. Does it satisfy all the use cases you can think of?

larryhastings commented 2 years ago

I've played around with it some more, and here's the API I've hit on:

app = appeal.Appeal()
command_method = app.command_method()

class MyApp:
    def __init__(self, id):
        self.id = id

    def __repr__(self):
        return f"<MyApp id={self.id!r}>"

    @command_method()
    def add(self, a, b, c):
        print(f"MyApp add {self=} {a=} {b=} {c=}")

command_method2 = command_method.new_class()

# commands implemented using different instances? sure!
class MyApp2:
    def __init__(self, id):
        self.id = id

    def __repr__(self):
        return f"<MyApp2 id={self.id!r}>"

    @command_method2()
    def remove(self, a, b, c):
        print(f"MyApp2 remove {self=} {a=} {b=} {c=}")

my_app = MyApp("koogle")
my_app2 = MyApp2("froogle")

p = app.processor()
p.prepare(command_method, my_app)
p.prepare(command_method2, my_app2)
p.main()

This maps two commands on the command-line, add and remove, which are implemented using two different methods on two different instances of two different classes.

Appeal.processor() returns a Processor object--you haven't seen that before, that's new. I'm trying to split off all the runtime data from the Appeal object, letting you run multiple command-lines through the same Appeal object and tree--even concurrently. And I want to put the remapped arguments in the Processor object so you could use different instances, like if your commands were method calls on a database connection object or something. Anyway, Appeal.main() and Appeal.process() are now convenience functions that create a Processor object and call the equivalent method on that.

Appeal.command_method() returns a CommandMethodPreparer which allocates a placeholder object (literally an object()) and creates a functools.partial() passing in that placeholder as the first positional argument. Then, later, we pass in that CommandMethodPrepare object and the instance we want to substitute it with to Preparer.prepare(). This means that, every time Appeal builds a function call to a command or converter, just before calling the function, it calls into the CommandMethodPrepare object to prepare the function, which substitutes the actual instance for the placeholder.

I also made a variant of partial_replace_map() that I use behind the scenes here. It assumes the first positional argument is a self parameter, and uses getattr(self, fn.__name__) to pull out a proper bound method, rather than just passing in self as the first positional parameter. My thinking here: some classes like to reimplement the descriptor protocol (__getattribute__) and play wacky games under the covers; binding the method this way preserves compatibility with this style of magical code.

encukou commented 2 years ago

Certainly this should satisfy your original use case, as stated in the message that opened the issue. Does it satisfy all the use cases you can think of?

Getting back to you is on my TODO list, but I'm unfortunately swamped by other work right now. Thanks for considering this issue, though!

larryhastings commented 2 years ago

I tweaked the API just a little, now it looks like this:

app = appeal.Appeal()
command_method = app.command_method()

class MyApp:
    def __init__(self, id):
        self.id = id

    def __repr__(self):
        return f"<MyApp id={self.id!r}>"

    @command_method()
    def add(self, a, b, c):
        print(f"MyApp add {self=} {a=} {b=} {c=}")

my_app = MyApp("koogle")

p = app.processor()
p.preparer(command_method.bind(my_app))
p.main()

I like this fine, so I'm gonna check it in. If you have concerns, or this doesn't solve your use case, tell me about it and we'll just fix it.

larryhastings commented 2 years ago

I think this solution needs some embellishment / sophistication. The interface you want is:

This needs to have a nice interface on it. We mostly have the pieces we need here, though we might need to revive the "parameter decorated with a subclass of AppealSomething gets that field passed in automatically". Anyway I'm gonna think some more about it.

I think I want the interface to look something like this:

a = appeal.Appeal()
global_command_class, command_method = a.app_class()

@global_command_class()
class MyApp:
    def __init__(self, *, verbose=False):
        self.verbose = False

    @command_method()
    def add(self, a, b, c):
        print(f"MyApp add {self=} {self.verbose=} {a=} {b=} {c=}")

app.main()

(important! you must decorate the class declaration, not the declaration of __init__.)

larryhastings commented 2 years ago

Okay! It's checked in, and has basically the interface I wanted. Here's the actual sample code I used to test the implementation:

import appeal

app = appeal.Appeal()
app_class, command_method = app.app_class()

@app_class()
class MyApp:
    def __init__(self, *, verbose=False):
        print(f"MyApp init {verbose=}")
        self.verbose = verbose

    @command_method()
    def add(self, a, b, c):
        print(f"MyApp add {a=} {b=} {c=} {self.verbose=}")

app.main()

I'll update the docs and add a test later.

larryhastings commented 2 years ago

Okay, I'm pretty happy where this is now. Again, if this hasn't solved your problem, comment or reopen or open a new issue, whatever works for you! And thanks for the feedback--this was a good nudge for me to finally figure out Appeal should interact with classes.