argh
argh copied to clipboard
Adding toggling decorators and testing
Adds decorators that enable the creation of pairs of mutually exclusive boolean arguments.
(See #72 for details.)
At the moment I cannot accept this PR as it doesn't pass tests:
- Python 2.6 doesn't understand
'{}'
('{0}'
is required). - The tests for Python 2.7 had failed because of a connection problem so I've restarted the build. Hopefully it will pass. Travis is soooooo slow these days.
The Python 2.6 formatting issue has been fixed.
Cool, thanks. I'm now reading your pull request thoroughly which also results in light refactoring and adding more detailed tests. And it seems that we've dug up a small problem which was given not enough attention before.
The question is: what should we do if the default value of the toggleable argument is True
? Let's take this example:
def cmd(a=True, b=False):
print('a', a, 'b', b)
How does it behave? The store_x
actions are assigned by negating the default values, so the defaults are kept intact when the related argument is missing but inverted when the argument is supplied:
$ cmd
a True b False
$ cmd -a -b
a False b True
Note that this is purely on the CLI level; Python does not support the notion of switches in function calls and requires a value along with the argument name, so consistency with Python notation is irrelevant here.
Alright, now if we mark these arguments as toggleables, how should they behave? I would expect them to behave exactly the same way with the only difference being that --no-foo
arguments are added (mutually exclusive with their positive counterparts). That is, the overall behaviour should be consistent regardless of whether the argument was added to the list of toggleables or not.
But then two of your tests (multiarg
and multitoggle
) would fail because they suppose a different behaviour: --foo
implies store_true
and --no-foo
implies store_false
regardless of the argument's default value.
And the more I think about which is right and which is wrong, the more I lean towards your version.
Without toggleables it was pretty evident that cmd(x=False)
should be 'store_true'
and cmd(x=True)
should be 'store_false'
as there was no other way to invert the value from CLI. But now there is one, and the benefit of having a cmd(foo=True)
with a --foo
that yields False
becomes highly questionable.
It is indeed very strange for a --not-foo
to yield a True
value for foo
argument. Maybe it's up to the developer to decide if the argument should deviate from the common idiom foo=False
for a switch; however, I cannot come up with a sensible use case.
Another example:
def cmd(table=True):
if table:
print('table rows')
else:
print('lines')
The corresponding CLI:
$ cmd
table rows
$ cmd --table
lines
WTF?! Of course we'd have to rename the argument to no_table
/--no-table
, but then the code becomes overcomplicated with these double negations. The only sensible version is this one:
$ cmd
table rows
$ cmd --table
table rows
$ cmd --no-table
lines
...And it should correspond to a function signature cmd(table=True)
.
So, it seems to me that when argument metadata is inferred from the function and the default value is a bool
, the action should always become 'store_true'
; but if that bool
value is also True
, we should also add a "counterargument" --no-x
. This would move your feature a bit closer to the core of the library. This requires a bit more thinking and refactoring but it seems that writing this comment helped me understand a lot about the issue :)
Anyway, what I mentioned as preferable implies backward incompatibility; we may simply add the decorator and then deprecate it for certain cases when the desired behaviour could be triggered automatically.
Thanks for the response. I created these decorators because of the problems you mentioned -- nonintuitive behavior (also with using argparse) that can result when there is a disconnect between the names of boolean parameters and their default behavior, and the increased mental load that places on the developer when invoking a program. After using the feature for a few months myself, I've found that I'm always using @argh.set_all_toggleable()
Basically, there is never a case when I don't want a mutually exclusive pair for boolean arguments, because then I can always be guaranteed to get what I want from my programs by specifying one of --foo
or --no-foo
The behavior you mentioned for "counterargument" creation is interesting, but it might be confusing if both the decorators and that exist.
Another option could be to make adding the toggling pairs the default behavior, and instead make a separate decorator to turn it off, in which case, all inference of parameter meaning should be left to the developer who turned off the toggling for a(ny) parameter(s). However, that may or may not be too strict and biased toward my own use preferences >:)
make adding the toggling pairs the default behavior, and instead make a separate decorator to turn it off, in which case, all inference of parameter meaning should be left to the developer who turned off the toggling for a(ny) parameter(s)
This is exactly what I meant. I think the treatment of booleans should be smarter than it is (and it already is smarter than it was with barebones argparse, so we shouldn't be afraid of it in general).
I would even think of a simplified default behaviour:
-
cmd(foo=False)
→cmd [--foo]
-
cmd(foo=True)
→cmd [--no-foo]
...and a method (dedicated decorator or a keyword in a decorator) to enable redundant arguments (that don't have effect) to make these mutually exclusive pairs of --foo
/--no-foo
.
It would look like this:
# negative defaults, action 'store_true'
# --foo is autotoggled
@toggle('--bar') # use redundant negative counterargument
@toggle('--quux', 'not') # same, also specify custom prefix
def cmd_a(foo=False, bar=False, quux=False):
"""
usage: cmd-a [--foo] [--bar | --no-bar] [--quux | --not-quux]
"""
pass
# positive defaults, action 'store_false'
# --foo is autotoggled
@toggle('--bar') # use redundant positive counterargument
@toggle('--quux', 'not') # same, also specify custom prefix
@toggle('--whiz', 'never', redundant=False) # custom prefix but no redundancy
def cmd_b(foo=True, bar=True, quux=True, whiz=True):
"""
usage: cmd-b [--no-foo] [--bar | --no-bar] [--quux | --not-quux] [--never-whiz]
"""
pass
I think cmd(foo=True)
mapped to cmd --foo
is a marginal (and probably even harmful) case which is safe to omit.
Of course a @toggle_all
decorator would also make sense.