-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
argparse claims '*' positional argument is required in error output #72795
Comments
In Python 3.5.2, with a positional argument with nargs='*', running the program with no arguments gives an error like this: usage: caffeinate [-h] [-V] COMMAND [ARGUMENT [ARGUMENT ...]] Here it is clear from the (correct, though redundant) syntax that "ARGUMENT" is optional, but the error message claims, incorrectly, that it is required. The add_argument calls used were: parser.add_argument('COMMAND', help='command to run')
parser.add_argument('ARGUMENT', nargs='*', help='arguments to COMMAND')
parser.add_argument('-V', '--version', action='version', version=PROGRAM_NAME + ' ' + VERSION) |
I agree that the message is slightly misleading. Uploading one possible solution. I'm sure someone more familiar with the library might have a better approach... |
Thanks very much for this. It would be great if the redundancy I referred to in the usage message could also be removed, so that instead of "[ARGUMENT [ARGUMENT ...]]" it just said "[ARGUMENT ...]". (Similarly, for a '+' argument, it could say just ARGUMENT ... ) |
The current error message is the result of http://bugs.python.org/issue10424 and http://bugs.python.org/issue12776 Before the test was just: if positionals:
self.error(_('too few arguments')) The 2nd patch reworked the test to include the revised handling of defaults. So the current error message just lists all the positionals which haven't been consumed. ARGUMENT wasn't consumed because COMMAND wasn't consumed. And technically that would be true even if ARGUMENT required arguments. Well, to be pickier, it as a 're' pattern like 'AA*' that failed. The proposed patch looks like it would work, but I haven't tested or looked at the unittests. But I wonder if such a patch is really needed. Are users really misled by the the current message? =============== As for the usage, the current version allows you to give a tuple METAVAR, producing lines like:
This display pattern is generated in HelpFormater._format_args, with these lines
You could subclass HelpFormatter, and replace this method with one that performs as you want, (I just tried this)
I wouldn't recommend this as default change, but if there was a enough demand it could added as another HelpFormatter subclass. METAVAR lets me approximate your shorter version:
It still has the [], but the name is gone. |
Try Including your ARGUMENT action in the error message is intentional. The test for this error message is: required_actions = []
for action in self._actions:
if action not in seen_actions:
if action.required: Originally the code just checked if For optionals,
So for '?' argument, required is False. But for '*', it must also have a 'default' parameter (not just the default None). So the proposed patch is overriding the 'required' value that was set during 'add_argument'. And issuing this error message is the main purpose of the 'required' attribute. I would not implement this patch. But it would be a good idea to check if this method of setting the required attribute has been discussed in other bug/issues. (There is an open issue concerning how the 'required' is set/or not for the subparsers positional.) Off hand I don't see anything about this in the documentation. Maybe that's what needs to be patched. (It's easier and safer to change the documentation than the working code. Backward compatibility is a major concern when changing the code.) |
Why would I do that? I want 0 or more arguments, and there's no default value.
It will likely confuse the user. The syntax message says that it's optional, the error suggests (wrongly) that it is required. Patching the Python documentation will not help in this case, because the user of my program will not be reading the Python documentation to understand how it works, only the program's own documentation. Note that I did not suggest that the behavior be changed, only the message that is output. The analysis of why the message is output is useful, but it does not demonstrate that the error message is correct; the error message, as I've already demonstrated, is undeniably wrong. In no sense is "ARGUMENT" missing, because 0 occurrences are entirely legal. Therefore although in terms of the API the argument is "required", it makes no sense to tell the user this (the user will assume that "required" has its colloquial sense, not a technical sense). I entirely sympathise with the argument that the behavior of argparse should not change; I see nothing wrong with the behavior, in any case. The problems are purely cosmetic:
These are serious annoyances from the developer's point of view (in this case, from my point of view), because they mean that in order to release a production-quality tool, I have to hack around argparse's shortcomings. So in fact, you're quite right that the documentation should be fixed; only in this case it is the documentation generated by argparse, not the documentation of argparse (which, again, is fine as far as I can see). |
Simply including a
You shouldn't see any other changes in behavior (except maybe if the positional is in a mutually_exclusive_group). The code that sets 'required' for positionals only looks for the presence of the parameter in kwargs, not its value: An alternative is to change the value of 'required' after creation:
Anyways I remain convinced that changing the 'required' attribute is the correct way to change the error message, not adding more tests to the message generator. |
My suggestion to use http://bugs.python.org/issue14074 The tuple metavar does not work right for positionals. That's a old issue that should have been fixed long ago. So streamlining the usage has to be done with a custom HelpFormatter subclass. |
Thanks, that's a simple, robust workaround. I'll duck out now and leave the Python experts to sort out the underlying problem, if they can; I think it's still well worth sorting out, though documenting the problem and workaround would be much better than nothing. |
I definitely agree with Reuben here -- I just ran into this issue while creating a "production quality" tool, and the help message produced by argparse with nargs='*' confused me too. It's pretty clear that this is a simple bug in argparse's production of that usage message: it says ARGUMENT is required, but it isn't. However, the workaround of specifying an (unused) default is a reasonable workaround in the meantime -- thanks Paul. |
nargs='*' being marked as I was also very confused by this behavior, the only reason I found this bug was to search before opening new, and had a patch prepared that is nearly identical to the one here.
|
Reproduced on 3.11. >>> parser.parse_args([])
usage: [-h] [-V] COMMAND [ARGUMENT ...]
: error: the following arguments are required: COMMAND, ARGUMENT |
I would like to say that I also bumped into this, the usage text / error message is self-contradictory. The end result is very confusing to users. I understand there is a workaround, but I couldn't find any mention of it anywhere in the documentation, and Googling for this buggy argparse behavior didn't come up with this GitHub issue. |
Mimic what 'ssh' does more closely, by invoking a shell inside the container and using it to run the specified command. To do this, concatenate the full argument list into a space-separated list and pass it to the shell via its '-c' argument, e.g.: /bin/sh -c 'command arg1 arg2' Callers of the 'ssh' binary expect this behavior, and it really helps when using the tool interactively, e.g., one can specify shell pipelines directly. But it diverges from what 'kubectl exec' supports, so add an argument, --no-shell, to disable this default behavior. Also disable prefix matching when passing arguments, and work around a longstanding upstream bug with Python's argparse, see: python/cpython#72795 Signed-off-by: Vangelis Koukis <[email protected]>
… non-required This allows to use positional argument with nargs='*' and without default in mutually exclusive group and improves error message about required arguments.
I agree that the current error message that requires an argument with It is even worse for from argparse import *
parser = ArgumentParser()
parser.add_argument('COMMAND', help='command to run')
parser.add_argument('ARGUMENT', nargs=REMAINDER, help='arguments to COMMAND')
parser.parse_args([]) Output:
I think that it is an error to mark such positional arguments as required. I guess that the reason of marking As for |
…equired (GH-124306) This allows to use positional argument with nargs='*' and without default in mutually exclusive group and improves error message about required arguments.
… non-required (pythonGH-124306) This allows to use positional argument with nargs='*' and without default in mutually exclusive group and improves error message about required arguments. (cherry picked from commit 3c83f99) Co-authored-by: Serhiy Storchaka <[email protected]>
… non-required (pythonGH-124306) This allows to use positional argument with nargs='*' and without default in mutually exclusive group and improves error message about required arguments. (cherry picked from commit 3c83f99) Co-authored-by: Serhiy Storchaka <[email protected]>
…R non-required (GH-124306) (GH-124422) This allows to use positional argument with nargs='*' and without default in mutually exclusive group and improves error message about required arguments. (cherry picked from commit 3c83f99) Co-authored-by: Serhiy Storchaka <[email protected]>
…R non-required (GH-124306) (#124421) This allows to use positional argument with nargs='*' and without default in mutually exclusive group and improves error message about required arguments. (cherry picked from commit 3c83f99) Co-authored-by: Serhiy Storchaka <[email protected]>
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:
bugs.python.org fields:
Linked PRs
The text was updated successfully, but these errors were encountered: