List-type arg values

[bitprophet]

Strongly related to #9, how to handle list values.

In #9, I assumed they would be contiguous values on the CLI with a separator, e.g. inv mytask --listarg list;val;here.

However, a common pattern for existing arg parsing libs is to give a flag >1 time and have the results tallied into a list, e.g. inv mytask -l list -l val -l here. E.g. this is how argparse/optparse do it, IIRC.

Pros of that latter approach:

• No need to deal with overridden meaning for shell meta-characters (many candidate delimiters have shell meaning.)
  • Counterpoint: commas work pretty good...
• No need to deal with escaping (always a PITA.)
• Minority use case: aping/wrapping pre-existing tools which work this way (e.g. using Invoke as a lightweight argument preprocessor before calling a Java process.)

Cons:

• Harder to implement, requires the parser to be more complicated :(
• Some parsers treat multiple values for the same flag as overrides, so in the above example we'd get -l here "winning" as a string, not list, value. Some users may expect or desire this behavior, e.g. when stitching together args from various "levels" of an invoking program.
• When you are intending to give a list in a single invocation, typing out mytask --listarg a;b;c is much faster than mytask -l a -l b -l c.

Either way:

• Do we perform sub-list transformation, i.e. do we turn mytask --listarg 1,2,3 into a list of ints automatically? Which again ties back into CLI arg typecasting #9.

[sophacles]

I'm strictly opposed to the ; separator. Bash just doesn't like it and quotes start getting nasty (; is assumed to be "end of command" and e.g.:

inv -c gah -r /tmp task1 --arg=foo;bar;baz
this is a task
-bash: bar: command not found
-bash: baz: command not found

)

Other than that, I have a preference towards the mytask -la -lb -l c style.

[drakeguan]

I also prefer the repeating arguments way, or the argparse/optparse way, to handle list. It's more intuitive to work with from viewpoint of commandline. Just wondering how you would like to handle this then.

[bitprophet]

@sophacles Yea that was just a dumb example, if I did go the "sub-parsing" route I'd probably use commas (again) as it's one of the few without common shell meaning.

But it sounds like going for correctness (-l a -l b -l c) over usability (-l a,b,c) may be the right call in this case, esp given how important the parser is as a feature set. We could also add both eventually, though that can and should wait. Thanks for the feedback, guys.

[drakeguan]

@bitprophet Just wondering how would you like to do this. I am willing to provide any help, even though some code snippets as long as you're comfortable with that.

[bitprophet]

@drakeguan I'll take care of it sometime soon, have been on vacation until today.

[drakeguan]

Really excited to know that. Just let us (I believe there are lots developers interested in this feature) know if there is anything need help or discussion, then. Thank you!

[bitprophet]

(3 years later...) I've got a POC of this for the base case (list of strings) working, where one manually indicates which arguments should be list-type instead of string-type - similar to how positional and optional args are configured. E.g.:

@task(iterable=['listy'])
def mytask(c, listy=None):
    print(listy)

and then:

$ inv mytask --listy foo --listy bar
['foo', 'bar']

Need to backfill more tests & figure out some of the corner/edge cases:

• I've only got a high level CLI test atm so need a handful more for Parser, Argument at least
• Needs explicit mention in the docs (the autodoc for @task is not sufficient.)
• Make sure it meshes well with no-default-value task function arguments
• Worth trying to handle other types of list members? I can't see lists of ints or bools, for example, being a very common use case so it's probably okay to punt on that for now...
  • Counterpoint: -vvv, which is arguably a list of booleans. (I actually don't know offhand how argparse & friends represent this behind the scenes, if they do...)
• This may exist already for the positional/optional features (in which case I may punt), but, the case where a user typos the argument name in @task(iterable=[names...]) - right now it'll probably explode messily instead of usefully?
• Is iterable the best name for that @task arg? it's a bit overloaded (could mean a single iterable, could imply "is iterable y/n?", etc) and this is a user-facing API. It's not awful but it's not super.
  • lists or iterables are clearer but still not great (esp since the established pattern has been to use the singular, i.e. optional=[name, ...] and positional=[name, ...])
  • FTR, argparse folds this into the more complex/nuanced nargs but that's not a good name here.
  • Comedy option: make this kinds={mapping}, e.g. kinds={'listy': list}, which simply feeds into Argument(kind=...) (versus requiring translation within Task as currently.)
    • More forward-looking, but breaks consistency with optional/positional (it implies those ought to be part of an overall "how to build Arguments out of this function" mapping that includes kind and the rest)
    • And at this point we're sorta in territory owned by Explicit control over a task's parser level flags #174 or *args and **kwds support in tasks #378.

[bitprophet]

That base case is all doc'd up now. Wondering if I can cram in the -vvv use case while the patient's entabled. Thoughts:

• The simplest/most naive approach would be to add another kwarg to @task like counters that otherwise acts a lot like iterable, incrementing instead of appending, resulting in an int.
  • counters highlights the name issue, since they "seem" to be out of whack, even though both are used the same in my head: "args which are iterable", "args which are counters". Meh.
  • Maybe increments? That sounds nice.
• I'd like to avoid runaway growth in @task's argument set so this continues to push me towards "have an arbitrary way for @task to associate Argument objects with the function's signature"; how would that look here, solving perhaps both use cases:
  • Maybe @task(arguments={'name': Argument(), ...}). Not sure what else really works here so let's assume we're doing it that way - user can explicitly make an Argument themselves to override the implicit ones we normally make.
  • Then perhaps we augment (or replace) Argument.kind with something more flexible that can imply (or explicitly implement) not only the usual string/int/bool but also list/increment/etc.
    • E.g. allow a callable of the form func(current_value, new_value) which returns updated_value; for normal single-value args this would simply return new_value (possibly excepting when current_value is truthy, if one doesn't want to allow overwriting), for iterables it returns the union, for counters it returns the increment.
    • However we really need two things here - both the behavior of "setting" the value, but also the hints to the parser itself about whether a flag takes a value or not (i.e. is a boolean flag). This latter is currently driven solely by kind and whether it's bool or other (or now list).
    • Might be possible to update the parser to be intentionally dumber about this, right now the only kind check is the new one for list, and the related bit is Argument.takes_value, which is what needs figuring out.
• Another angle is where one may want the same casting as is done with single-value arguments, for iterable ones, so one can e.g. mytask --numeric 1 --numeric 7 --numeric 15 to arrive at numeric==[1, 7, 15] instead of numeric==['1', '7', '15'].
  • In the above "arbitrary callable" approach, this still "works" insofar as one could manually provide a setter that both appends and casts.
  • But it definitely makes me wonder if we just want to go the subclassing route here, and have Integer, String, StringList etc. Which would be extremely small subclasses just overriding takes_value and set_value.

[bitprophet]

One worry about going the direct-argument-link route is it makes me want to fold the existing positional and optional @task kwargs into it. Technically anything goes before 1.0 but I still want to avoid making such changes unless they're seriously necessary. May just go the 'deprecated' route and say "not actually nuking these rn but they'll go away at 1.0"?

---

Also, as I glance at how this would work, there's a bigger issue - parser.Argument includes the argument name(s), which clashes with an attempt at describing "everything but the name" as was in my head. E.g. @task(arguments={'mylist': Argument(kind=list)}) - this doesn't actually work because the Argument object has no name info. Heck, even if we were to do something like tack the names onto the Argument objects inside of @task/Task, this still has a hole where multiple names/aliases would want to go.

Probably better is the idea I had earlier which was for this mapping to just be "arg args", i.e. kwargs to be given to the Argument to override the implied info. So e.g.:

@task(arg_kwargs={'mylist': {'kind': list}})
def mytask(c, mylist):
    pass

Downside is that there's no good name for this due to the clash between parser/cli "args" and function/class "args/kwargs". Blah. This may be something that only gets fully developed under Python 3 since it now allows for actual type annotations (including, I believe, lists-of-kinds e.g. list[str].)

[bitprophet]

Anyway that stuff all really belongs in #174 / #378 as noted earlier.

For now, I think I am gonna go with the increments/counters approach, because -vvv and friends are honestly one of the most common use cases for "give an arg >1 time" and it feels very awkward to have --list of --list items --list here and not -vvv.

[bitprophet]

Naming of that arg still problematic since the current ones are all "these arguments are ", i.e. "these arguments are positional|optional|iterable. I guess the true match would be incrementable? Sure.

[bitprophet]

That's what it ended up being. This is done! It's kinda first-draft:

• for one thing, there are many ways for less-than-attentive users to receive presumably confusing errors by misusing these new @task kwargs - e.g. trying to use incrementable with a non-integer-default function argument will probably get you an actually-pretty-straightforward TypeError.
• for another, the actual inner implementation is slightly hacky. Nothing visibly gross unless one is manually trying to use invoke.parser.Argument by itself, which seems rather unlikely.

But it definitely works for me as documented and tackles both of the related use cases, so, hooray!