argparse: mixing optional and positional arguments... not again

[monkeyman79]

BPO	42973
Nosy	@bitdancer, @monkeyman79
PRs	bpo-42973: argparse: mixing optional and positional #24259 bpo-42973: argparse: mixing optional and positional #24367

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-01-19.22:21:34.677>
labels = ['type-feature', 'library', '3.10']
title = 'argparse: mixing optional and positional arguments... not again'
updated_at = <Date 2021-01-29.13:29:51.018>
user = 'https://github.com/monkeyman79'

bugs.python.org fields:

activity = <Date 2021-01-29.13:29:51.018>
actor = 'monkeyman79'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-01-19.22:21:34.677>
creator = 'monkeyman79'
dependencies = []
files = []
hgrepos = []
issue_num = 42973
keywords = ['patch']
message_count = 21.0
messages = ['385301', '385369', '385386', '385388', '385659', '385675', '385683', '385684', '385690', '385691', '385699', '385727', '385732', '385733', '385734', '385770', '385804', '385809', '385810', '385827', '385829']
nosy_count = 4.0
nosy_names = ['v+python', 'r.david.murray', 'paul.j3', 'monkeyman79']
pr_nums = ['24259', '24367']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue42973'
versions = ['Python 3.10']

[monkeyman79]

I have following use case for argparse:

./prog --tracks=80 image1 --tracks=40 image2 --tracks=80 image3 ...

I expected that the following parser would be able process that command line:

parser = argparse.ArgumentParser()
add = parser.add_argument
add('--tracks', choices=['40', '80'])
add('image', nargs='*', action=_CustomAction)
parser.parse_args()

But it stops with 'unrecognized arguments: image2' error. It turns out that there is no way to prepare parser which could handle this command line.

There was a long discussion about this issue in bpo-14191. There were two approaches proposed:

• Allow 'image' action to trigger multiple times with additional encountered parameters.
• Parse all optionals first and all positionals in second pass.

The first approach was represented by a patch by user guilherme-pg. This patch was slightly criticized (undeservedly IMO) and pretty much ignored.

The discussion then focused on the second approach which ended up with introduction of parse_intermixed_args into codebase.

The problem with parse_intermixed_args is that it defeats the purpose of having intermixed arguments. When the arguments are effectively reordered, there is no way to match "optionals" with "positionals" with they relate to.

In my option the Guilherme's approach was the correct way to go, I would say it was elegant. The only complaint against that patch is that it unnecessarily modified _StoreAction class - instead of that, the way to go is to use "extend" or custom action.

I allowed myself to simplify the changes a bit for readability. The patch doesn't change default functionality, for the cost of changing public API by adding new argument to ArgumentParser constructor.

With the changes applied, I can parse my command line with this code:

parser = argparse.ArgumentParser(greedy_star=True)
add = parser.add_argument
add('--tracks', choices=['40', '80'])
add('image', nargs='*', action=_CustomAction)
parser.parse_args()

[paulj3]

Your patch is incomplete, without documentation or tests.

Your example is incomplete. That is _CustomAction? What namespace does your patch produce.

It's been a while since I worked on the intermixed patch, but as I recall the OP was happy with the fix. Also I don't recall any talk about 'matching "optionals" with "positionals" with they relate to. Was that part of the discussion?

On Stackoverflow I have seen questions about pairing arguments, and answered some. I don't recall the best answers. The tough version is when some of the grouped inputs may be missing

In your example I'd suggest making '--tracks' a nargs=2 and 'append' option. 'choices' would have to be replaced with a either a custom 'type', a custom Action, or post-parsing testing.

I haven't had time yet to test your patch.

One question I frequently ask posters who want to parse complex inputs is: 'how are going to explain this to your users?' 'What kind of usage and help do you want see?'

[monkeyman79]

> Your example is incomplete. That is _CustomAction? What namespace does your patch produce.

> In your example I'd suggest making '--tracks' a nargs=2 and 'append' option. 'choices' would have to be replaced with a either a custom 'type', a custom Action, or post-parsing testing.

My use case and example were oversimplified. The real life use case would be a program that processes multiple sources, with various processing parameters which may change between sources and writes result to destination:

example.py:

#! /usr/bin/python3

import argparse

class _AddSourceAction(argparse.Action):
    def __init__(self, option_strings, dest, nargs=None, const=None,
                 default=None, type=None, choices=None, required=False,
                 help=None, metavar=None):
        super(_AddSourceAction, self).__init__(
            option_strings=option_strings, dest=dest, nargs=nargs,
            const=const, default=default, type=type,
            choices=choices, required=required, help=help,
            metavar=metavar)

    def __call__(self, parser, namespace, values, option_string=None):
        for source in values:
            namespace.sources.append({'name': source,
                'frobnicate': namespace.frobnicate,
                'massage_level': namespace.massage_level})

def process_files(dest, sources):
    for source in sources:
        frob = source["frobnicate"]
        print("processing %s, %sfrobnication, massage level %d " %
              (source["name"],
              "default " if frob is None else "no " if frob == False else "",
              source["massage_level"]))
    print("saving output to %s" % dest)

parser = argparse.ArgumentParser()
if hasattr(parser, "greedy_star"):
    setattr(parser, "greedy_star", True)
parser.add_argument('destination')
parser.add_argument('--frobnicate', action='store_true', dest='frobnicate')
parser.add_argument('--no-frobnicate', action='store_false', dest='frobnicate')
parser.add_argument('--massage-level', type=int, default=5)
parser.add_argument('sources', metavar="source", nargs='*', action=_AddSourceAction)
parser.set_defaults(sources=[])
parser.set_defaults(frobnicate=None)
args = parser.parse_args()

process_files(args.destination, args.sources)

Without patch:

$ ./example.py output.txt --massage-level 4 input1.txt input2.txt --massage-level 5 input3.txt --frobnicate input4.txt input5.txt
example.py: error: unrecognized arguments: input1.txt input2.txt input3.txt input4.txt input5.txt

With patch:

$ PYTHONPATH=patch ./example.py output.txt --massage-level 4 input1.txt input2.txt --massage-level 5 input3.txt --frobnicate input4.txt input5.txt
processing input1.txt, default frobnication, massage level 4
processing input2.txt, default frobnication, massage level 4
processing input3.txt, default frobnication, massage level 5
processing input4.txt, frobnication, massage level 5
processing input5.txt, frobnication, massage level 5
saving output to output.txt

Note that I avoided passing greedy_star to constructor, to avoid error with unpatched argparse. Maybe that should be recommended way to enable this feature? N.b. I don't quite like this name 'greedy_star' seems to informal for me, but I didn't come up with anything better.

In this example frobnicate and massage_level apply to all following sources, but _AddSourceAction could be modified to reset their values, so they would only apply to first following source.

> It's been a while since I worked on the intermixed patch, but as I recall the OP was happy with the fix. Also I don't recall any talk about 'matching "optionals" with "positionals" with they relate to. Was that part of the discussion?
> On Stackoverflow I have seen questions about pairing arguments, and answered some. I don't recall the best answers. The tough version is when some of the grouped inputs may be missing

The use case above seems quite basic and common to me. I'm surprises that I actually didn't find any question on stackoverflow about this exact use case.

> One question I frequently ask posters who want to parse complex inputs is: 'how are going to explain this to your users?' 'What kind of usage and help do you want see?'

I would (and I will, because I'm really working on a project which will do something like this) put all options which apply to source files into separate option group and add help string to this option group which explains that those options may be specified between source files and that they apply to all (or just first) following source files.

> Your patch is incomplete, without documentation or tests.

Yeah, I can work on that if there is any chance that this will go anywhere. This is an effort from my side however, because I have quite slow mobile link at this moment. I tried to checkout python sources, but even with -depth 1 it took too long for my patience and I gave up after 'du -sh .git' showed 2G.

[monkeyman79]

> I tried to checkout python sources, but even with -depth 1 it took too long for my patience and I gave up after 'du -sh .git' showed 2G.

Ignore that. I must have been doing something worng.

[monkeyman79]

I added tests and docs to the PR. How does it look now?

[vpython]

On 1/25/2021 12:43 PM, Tadek Kijkowski wrote:

Tadek Kijkowski <tkijkowski@gmail.com> added the comment:

I added tests and docs to the PR. How does it look now?
Could you send me the docs privately?

I'm trying to understand what you are suggesting, without reading the code.

I realize that you are trying to apply different values of optional
parameters to positional parameters... how are you saving that context?
Is it up to the special handler of the positional parameter to read and 
save the values of the optional parameters specified so far? What about
the ones unspecified so far? Can one even read and save the default value?

And where does it get stored? Is the value of the positional parameter
turned into a dict or class containing its own value + the saved
optional parameters?

I do agree that the even though the parse_intermixed_args was sufficient
for the use case I had at the time, and has been sufficient for me so
far, that it  not fully flexible, and I think I have seen Unix command
line programs that had rich semantics similar to what you are proposing,
where the sequence of repeated optional args could affect handling of
later positionals but not earlier ones.

So I applaud your efforts here, but simply reading the issue, and having
forgotten most of the internal workings of argument parser since getting
the compromise going, I think reading your docs would help clarify it
for me.

[monkeyman79]

> Is it up to the special handler of the positional parameter to read and save the values of the optional parameters specified so far?

Yes, in order to get anything more that just concatenated list of positional parameters, one has to provide specialized action class. Basic functionality is easy to implement, as you can see in the fruits.py example.

> ... how are you saving that context?

Entire context is stored in the namespace object. Action class has access to all previous parameters via the namespace and it can modify the namespace as needed. Standard 'store' action just adds value to the namespace, but specialized action can anything.

> What about the ones unspecified so far?

They are obviously not available for the action processing positional parameters preceeding them, but will be present in the resulting namespace and can be processed after parsing is done.

> Can one even read and save the default value?

I'm not sure what do you mean, but it all depend on how action class deals with it.

> And where does it get stored? Is the value of the positional parameter turned into a dict or class containing its own value + the saved optional parameters?

It all gets stored in the namespace. Turning positionals + optionals into a dict or class seems most reasonable, but it all depends on how action class will deal with them.

> I think I have seen Unix command line programs that had rich semantics similar to what you are proposing, where the sequence of repeated optional args could affect handling of later positionals but not earlier ones.

For example 'tar' with '-C' parameter which applies to all files following it.

[monkeyman79]

This is, I think, smallest functional example for matching optional parameters with positionals - fruits.py:

  import argparse
  
  DEFAULT_COLOR="plain"
  
  class AddFruitAction(argparse.Action):
     def __call__(self, parser, namespace, values, option_string=None):
        for fruit in values:
           namespace.fruits.append({'name': fruit, 'color': namespace.color})
           namespace.color = DEFAULT_COLOR
  
  def show_fruits(fruits):
     for fruit in fruits:
        print(f"{fruit['color']} {fruit['name']}")
  
  parser = argparse.ArgumentParser(greedy=True)
  parser.add_argument('--color', default=DEFAULT_COLOR)
  parser.add_argument('fruits', nargs='*', action=AddFruitAction, default=[])
  args = parser.parse_args()
  show_fruits(args.fruits)

It starts with 'namespace.color' set to 'DEFAULT_COLOR' - 'default=DEFAULT_COLOR' takes care of that, and with 'namespace.fruits' set to empty list - via 'default=[]'.

For each group of positional command-line arguments, AddFruitAction is called with one or more fruit names in 'value'. The method iterates over them adding series of dicts to the 'fruits' list. The 'namespace.color' is immediately reset to default value, because we want the 'color' to apply to one following 'fruit'. If we wanted the 'color' to apply to all following 'fruits' the action class could just leave it alone.

After parsing is done, we get our namespace assigned to args, with list of color + fruit name pairs in 'args.fruits'.

[vpython]

OK, I think I see what you are doing here. Thanks for your responses.

And probably it is the bare-bones minimum feature that allows user-implementation of "as complex as desired" combinations of optionals and context-sensitive positionals.

In the docs fruits example, though, I think the input and output are inconsistent: you have a brown banana reported as yellow?

I guess I could think of enhancements to simplify common cases. For example, each positional might want to track/save values for some number of optionals. So one could write custom code to do that, like your fruit basket, or one could imagine a canned approach, where the list of optionals was passed as a parameter when defining the positionals. This could be built as a wrapper for add_argument, that would call a more general version of the AddFruit Action. Perhaps a special parameter could also indicate saving the values of all optionals at the point of parsing the positional, giving each positional a complete set of optionals that apply.

Your feature of having the optional apply to only one positional would be a harder thing, especially if you wanted to have both kinds of optionals: that apply to the next positional, or that apply to all following positionals. But hmm. Maybe not much harder: just have two lists of optionals passed to the positional: a list of each kind.

Does the namespace still contain the default value for an optional after it has been used? It looks like it might, so your fruit example could be reduced to not needing the DEFAULT_COLOR variable, but rather change references to it to use

  self._get_default_metavar_for_positional(action) 

? That avoids using a global variable, and is for more helpful for the "list of optionals to be reset on each use" rather than re-creating the more complex data structure required to hold all the defaults.

I'm not sure how that plays with optionals that use append or append_const storage methods, nor do I fully understand the comment in "def _copy_items(items):"

I think if you could avoid the global variable, your implementation would be stronger. And if you provide, either as a sample subclass, or argparse new feature, the ability to do the things I mention here, it would be even stronger, as it would allow the user to deal with the more powerful capabilities without needing as much understanding of argparse internals.