Skip to content

argparse claims '*' positional argument is required in error output #72795

Closed

Description

@rrt
mannequin
BPO 28609
Nosy @benhoyt, @charlierproctor, @iritkatriel
Files
  • optional_arguments.patch: Check that action.nargs not in [OPTIONAL, ZERO_OR_MORE]
  • 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 2016-11-04.15:19:23.095>
    labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
    title = "argparse claims '*' positional argument is required in error output"
    updated_at = <Date 2021-12-10.17:07:21.760>
    user = 'https://bugs.python.org/rrt'

    bugs.python.org fields:

    activity = <Date 2021-12-10.17:07:21.760>
    actor = 'iritkatriel'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Library (Lib)']
    creation = <Date 2016-11-04.15:19:23.095>
    creator = 'rrt'
    dependencies = []
    files = ['45357']
    hgrepos = []
    issue_num = 28609
    keywords = ['patch']
    message_count = 12.0
    messages = ['280052', '280072', '280074', '280161', '280169', '280171', '280180', '280181', '280230', '288644', '343614', '408225']
    nosy_count = 6.0
    nosy_names = ['benhoyt', 'paul.j3', 'charlie.proctor', 'rrt', 'svenevs', 'iritkatriel']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue28609'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    Linked PRs

    Activity

    1. rrt commented on Nov 4, 2016

      rrtmannequin
      MannequinAuthor

      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 ...]]
      caffeinate: error: the following arguments are required: COMMAND, 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)
    2. added
      stdlibStandard Library Python modules in the Lib/ directory
      type-bugAn unexpected behavior, bug, or error
      on Nov 4, 2016
    3. charlierproctor commented on Nov 4, 2016

      charlierproctormannequin
      Mannequin

      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...

    4. rrt commented on Nov 4, 2016

      rrtmannequin
      MannequinAuthor

      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 ...

      )

    5. paulj3 commented on Nov 6, 2016

      paulj3mannequin
      Mannequin

      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:

      In [359]: a.metavar=('A','B')
      In [360]: parser.print_usage()
      usage: ipython3 [-h] [A [B ...]]
      In [361]: a.nargs='+'
      In [362]: parser.print_usage()
      usage: ipython3 [-h] A [B ...]
      

      This display pattern is generated in HelpFormater._format_args, with these lines

          elif action.nargs == ZERO_OR_MORE:
              result = '[%s [%s ...]]' % get_metavar(2)
          elif action.nargs == ONE_OR_MORE:
              result = '%s [%s ...]' % get_metavar(2)
          elif action.nargs == REMAINDER:
              result = '...'
      

      You could subclass HelpFormatter, and replace this method with one that performs as you want, (I just tried this)

      result = '[%s ...]' % get_metavar(1)
      

      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:

      In [4]: p.print_usage()
      usage: ipython3 [-h] [pos [pos ...]]
      In [5]: a.metavar=('pos','')
      In [6]: p.print_usage()
      usage: ipython3 [-h] [pos [...]]
      In [7]: a.nargs='+'
      In [8]: p.print_usage()
      usage: ipython3 [-h] pos [...]
      

      It still has the [], but the name is gone.

    6. paulj3 commented on Nov 6, 2016

      paulj3mannequin
      Mannequin

      Try nargs='?' or try providing a default along with the '*'.

      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 positionals was empty. That was the list of positional Actions. Actions were popped as they were parsed. Now it maintains a set seen_actions, and checks the required attribute. This test applies to both positionals and optionals.

      For optionals, required is set by the programmer. But for positionals it is set with:

      def _get_positional_kwargs
           ...
          # mark positional arguments as required if at least one is
          # always required
          if kwargs.get('nargs') not in [OPTIONAL, ZERO_OR_MORE]:
              kwargs['required'] = True
          if kwargs.get('nargs') == ZERO_OR_MORE and 'default' not in kwargs:
              kwargs['required'] = True
      

      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.)

    7. rrt commented on Nov 6, 2016

      rrtmannequin
      MannequinAuthor

      Try nargs='?' or try providing a default along with the '*'.

      Why would I do that? I want 0 or more arguments, and there's no default value.

      Including your ARGUMENT action in the error message is intentional.

      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:

      1. The syntax message says, redundantly and confusingly, "[ARGUMENT [ARGUMENT ...]]" where it should say just "[ARGUMENT ...]".

      2. The error message says that ARGUMENT is "required", whereas, from the user's point of view, it clearly is not.

      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).

    8. paulj3 commented on Nov 7, 2016

      paulj3mannequin
      Mannequin

      Simply including a default parameter, even with the default default None, changes the error message

       In [395]: parser=argparse.ArgumentParser()
       In [396]: parser.add_argument('cmd');
       In [397]: a=parser.add_argument('args',nargs='*',default=None)
       In [398]: a.required
       Out[398]: False
       In [399]: parser.parse_args([])
      
       usage: ipython3 [-h] cmd [args [args ...]]
       ipython3: error: the following arguments are required: cmd
      

      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: 'default' not in kwargs.

      An alternative is to change the value of 'required' after creation:

       a.required = False
      

      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.

    9. paulj3 commented on Nov 7, 2016

      paulj3mannequin
      Mannequin

      My suggestion to use metavar=('A','') to streamline the usage creates problems with the help code

      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.

    10. rrt commented on Nov 7, 2016

      rrtmannequin
      MannequinAuthor

      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.

    11. benhoyt commented on Feb 27, 2017

      benhoytmannequin
      Mannequin

      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.

    12. svenevs commented on May 27, 2019

      svenevsmannequin
      Mannequin

      For optionals, required is set by the programmer. But for positionals it is set with: ...
      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.

      nargs='*' being marked as required is incorrect though, isn't it?

      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.

      1. It's really helpful to know about explicitly setting default=None|[] depending on use case. Would a docs PR briefly explaining at the bottom of the nargs [a] docs explaining how to change the error message via default be welcome? This is a slightly challenging problem to search for.

      2. My understanding is the ultimate rejection of the patch is because it's bypassing the required attribute. So to fix this adequately, changing ? and * to not show up as required (when no default provided) should be done?

      [a] https://docs.python.org/3/library/argparse.html#nargs

    13. iritkatriel commented on Dec 10, 2021

      @iritkatriel
      Member

      Reproduced on 3.11.

      >>> parser.parse_args([])
      usage: [-h] [-V] COMMAND [ARGUMENT ...]
      : error: the following arguments are required: COMMAND, ARGUMENT
    14. 7 remaining items

    15. added a commit that references this issue on Sep 21, 2024
    16. serhiy-storchaka commented on Sep 21, 2024

      @serhiy-storchaka
      Member

      I agree that the current error message that requires an argument with nargs='*' is misleading.

      It is even worse for nargs=REMAINDER, because it refers to argument name not mentioned in the "usage" line:

      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:

      usage: [-h] COMMAND ...
      : error: the following arguments are required: COMMAND, ARGUMENT
      

      I think that it is an error to mark such positional arguments as required.

      I guess that the reason of marking nargs='*' without default as required lies in the way how conflicts in mutually exclusive groups were detected. The code simply checked if the value is the same as the default value, and this obviously does not work for []. Fixing this code allows to make all positional arguments with nargs='*' non-required.

      As for nargs=REMAINDER, I think it was just an omission.

    17. added
      3.12only security fixes
      3.13only security fixes
      3.14bugs and security fixes
      and removed
      3.11only security fixes
      on Sep 21, 2024
    18. added a commit that references this issue on Sep 24, 2024
    19. added 2 commits that reference this issue on Sep 24, 2024
    20. added a commit that references this issue on Sep 24, 2024
    21. moved this from Bugs to Doc issues in Argparse issueson Sep 24, 2024
    22. added a commit that references this issue on Oct 7, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.12only security fixes3.13only security fixes3.14bugs and security fixesstdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

      Projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions