Skip to content

argparse: positional arguments containing - in name not handled well #59330

Closed

Description

@nstiurca
mannequin
BPO 15125
Nosy @merwok, @bitdancer, @florentx, @vadmium, @khughitt, @shihai1991
Files
  • argparse-argument-names.diff
  • 15125-1.patch
  • 15125-2.patch
  • 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 2012-06-21.15:34:07.013>
    labels = ['3.11', 'type-bug', '3.9', '3.10', 'docs']
    title = 'argparse: positional arguments containing - in name not handled well'
    updated_at = <Date 2021-12-10.17:15:56.002>
    user = 'https://bugs.python.org/nstiurca'

    bugs.python.org fields:

    activity = <Date 2021-12-10.17:15:56.002>
    actor = 'iritkatriel'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation']
    creation = <Date 2012-06-21.15:34:07.013>
    creator = 'nstiurca'
    dependencies = []
    files = ['26286', '27860', '27863']
    hgrepos = []
    issue_num = 15125
    keywords = ['patch']
    message_count = 22.0
    messages = ['163340', '163383', '163456', '163457', '163461', '164789', '164792', '164808', '164968', '165060', '165067', '166181', '174655', '174656', '174660', '174677', '174725', '225527', '225628', '225744', '348622', '350811']
    nosy_count = 14.0
    nosy_names = ['bethard', 'sfllaw', 'eric.araujo', 'r.david.murray', 'flox', 'docs@python', 'tshepang', 'martin.panter', 'paul.j3', 'nstiurca', 'mapleoin', 'khughitt', 'Socob', 'shihai1991']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue15125'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    Linked PRs

    Activity

    1. nstiurca commented on Jun 21, 2012

      nstiurcamannequin
      MannequinAuthor

      To reproduce, try the following code:

      from argparse import ArgumentParser
      a = ArgumentParser()
      a.add_argument("foo-bar")
      args = a.parse_args(["biz"])
      print args, args.foo_bar

      Expected output:

      Namespace(foo_bar='biz') biz
      

      Actual output:

      Namespace(foo-bar='biz')
      Traceback (most recent call last):
        File "<stdin>", line 1, in <module>
      AttributeError: 'Namespace' object has no attribute 'foo_bar'

      Other comments:
      The positional argument 'foo-bar' becomes impossible to retrieve without explicitly passing keyword argument dest='foo_bar'. Hyphens in positional arguments should be automatically replaced with underscores just as with other arguments.

      I have not tested if this problem occurs in Python versions newer than 2.6.

    2. bitdancer commented on Jun 22, 2012

      @bitdancer
      Member

      It does.

    3. nstiurca commented on Jun 22, 2012

      nstiurcamannequin
      MannequinAuthor

      Re-selecting Python 2.6 under version (I think it was accidentally unselected).

      Here as another related error: if I try to add dest="baz" to the a.add_argument() call, it throws ValueError: dest supplied twice for positional argument

    4. bitdancer commented on Jun 22, 2012

      @bitdancer
      Member

      Nope, it was intentionally unselected. We use versions for the versions in which we will fix the bug, and 2.6 gets only security patches at this point. In any case, argparse isn't part of the stdlib in 2.6.

    5. bitdancer commented on Jun 22, 2012

      @bitdancer
      Member

      Oh, and to make sure your second report doesn't get lost, could you open another issue for the other bug, and give a complete example?

    6. mapleoin commented on Jul 7, 2012

      mapleoinmannequin
      Mannequin

      I'm working on this right now as part of EuroPython's CPython sprint.

    7. florentx commented on Jul 7, 2012

      florentxmannequin
      Mannequin

      I don't see a valid use case to support "-" in the name of the positional argument.

      IMHO, it should raise an error (probably a ValueError) for the add_argument in this case ...

      Or we keep it as-is and close as wont-fix: if the op wants to pass "foo-bar" for the name of the positional argument ... it is his problem.
      He can retrieve the value if he really want, with something like:
      getattr(args, 'foo-bar')

      In this case a single note in the documentation about using valid Python identifier for the names could be enough.

    8. mapleoin commented on Jul 7, 2012

      mapleoinmannequin
      Mannequin

      I agree with Florent that this is maybe just a documentation issue, since the argument is accessible via getattr().

    9. nstiurca commented on Jul 8, 2012

      nstiurcamannequin
      MannequinAuthor

      Florent, there are several reasons I think this is a valid use case. First, for optional arguments, '-' gets automatically replaced with '_' as the destination. I don't see any reason why optional and positional arguments should be treated differently when deciding the destination. This inconveniences the programmer who could naturally be inclined to follow some hyphenated optional arguments

      a.add_argument("--option-one")
      a.add_argument("--option-two")
      a.add_argument("--option-three")

      with hyphenated positional argument

      a.add_argument("positional-args")

      The programmer shouldn't have to pause and think about different naming requirements for optional and positional arguments.

      Second, persuading programmers to use underscores for positional args (eg, via proposed documentation patch) would have user-visible changes. Specifically, the automatically generated help/usage message would contain underscores instead of hyphens. Admittedly, this is fairly minor and inconsequential, but most programs use hyphens (not underscores) as delimiters in long options. I think it would be poor form to break user expectations in this regard.

      Last, I was apparently wrong (as Florent points out) that positional arguments whose names are invalid identifiers are impossible to retrieve. That's good to know, but that solution strikes me as just an ugly workaround. :/

    10. merwok commented on Jul 9, 2012

      @merwok
      Member

      Isn’t the obvious workaround to pass a dest argument which is a valid identifier? add_argument('spam-eggs', dest='spam_eggs')

      Maybe argparse in 3.4 could do this transformation automatically (see how namedtuple converts anything to valid identifiers).

    11. nstiurca commented on Jul 9, 2012

      nstiurcamannequin
      MannequinAuthor

      Eric:
      I agree, that would be the obvious workaround. However, it turns that the way dest is set differs for optional and positional arguments. I alluded to this in my earlier message http://bugs.python.org/issue15125#msg163456

      Specifically, it turns out that when the first argument to add_argument() starts with '-', it is interpreted as an optional argument and dest in _inferred_ from the first argument (eg, by stripping leading '-', and replacing remaining '-' with '_'). So supplying dest= to add_argument() overrides this heuristic.

      But when the first argument to add_argument() does not start with a '-', dest becomes exactly the first argument, and supplying dest as a keyword argument like you suggest yields the exception I reported earlier.

      Revisiting the documentation on dest (http://docs.python.org/dev/library/argparse.html#dest), this behavior is briefly and un-enlighteningly addressed in the first paragraph. The problem is that the paragraph suggests that dest can be explicitly set using keyword argument even for positional arguments; instead a ValueError is thrown as I have already mentioned.

      I suppose that patching argparse to replace '-' with '_' for positional arguments may break existing code for anyone that has figured out they can get hyphenated positional arguments with getattr(), which would make it a risky patch. But patching the module to allow explicitly setting dest via keyword argument shouldn't hurt anybody.

      For clarity, I recommend adding """(This conversion does NOT take place for positional arguments.)""" before the last sentence of paragraph 2 in the dest documentation.

      Since I had no idea about using getattr, I bet others wouldn't figure it out either. I suggest also adding a simple example in the documentation for anybody that wants to use an invalid Python identifier as an argument. (Please don't say this is an invalid use case because eg, ssh has options -1, -2, -4, and -6.)

    12. bethard commented on Jul 22, 2012

      bethardmannequin
      Mannequin

      I'm sympathetic to the idea that '-' should be translated similarly for optional and positional arguments, but as you've noted, it would be a risky patch because it's already possible for people to use getattr on hyphenated arguments. So I think this isn't possible without a long deprecation process first.

      But patching the module to allow explicitly setting dest via keyword
      argument shouldn't hurt anybody.

      I agree that it wouldn't hurt anybody. If you can find a way to do this, feel free to provide a patch.

      However, the correct way to have one name for the attribute (i.e. dest=) and one name for the help (i.e. metavar=) is to use the metavar argument like so:

          parser.add_argument('positional_args', metavar='positional-args')

      That said, this is not the first time I've seen someone run into this problem. I think the documentation could be improved in a few ways:

      (1) In the "name or flags" section, describe how the flags (for optional arguments) are translated into both a default "dest" (stripping initial '-' and converting '-' to '_') and into a default "metavar" (stripping initial '-' and uppercasing). Part of this is in the "dest" and "metavar" documentation, but probably belongs up in the "name or flags" documentation. Add cross-references to "dest" and "metavar" sections.

      (2) In the "name or flags" section, describe how the name (for positional arguments) are translated into the same default "dest" and "metavar" (no string munging at all). Again, move stuff from the "dest" and "metavar" sections as necessary. Add cross-references to "dest" and "metavar" sections.

      (3) In the "dest" section and somewhere in the "parse_args" section, describe how "getattr" can be used to get attributes whose names aren't valid Python identifiers. Maybe cross-reference this section from the edits in (2).

    13. 22 remaining items

    14. added
      3.12only security fixes
      stdlibStandard Library Python modules in the Lib/ directory
      and removed on May 3, 2023
    15. serhiy-storchaka commented on Sep 25, 2024

      @serhiy-storchaka
      Member

      There are issues with the proposed patches and #104092.

      • It changes also the default value for metavar.
      • It does not handle setattr and delattr.
      • And, most importantly, Namespace is only the default class for namespace. The user can use arbitrary object as a namespace. This cannot be fixed.

      So, if we still want to implement name mangling for positional parameters, I suggest the following 3-step plan:

      • Update the documentation.
      • Start emitting a FutureWarning for names containing '-'.
      • After few releases remove the warning and make the code replacing '-' with '_'.
    16. serhiy-storchaka commented on Oct 9, 2024

      @serhiy-storchaka
      Member

      @sfllaw, is it your patches? Are you interesting in creating a PR based on 15125-1.patch?

    17. sfllaw commented on Oct 9, 2024

      @sfllaw
      Contributor

      @serhiy-storchaka Sorry, but I no longer have good context for this patch. Please feel free to use it as a basis for your own PR, since you appear to have thought about this recently!

    18. added a commit that references this issue on Oct 9, 2024
    19. serhiy-storchaka commented on Oct 9, 2024

      @serhiy-storchaka
      Member

      Created #125215 based on 15125-1.patch. Thank you for your contribution @sfllaw.

    20. added a commit that references this issue on Oct 12, 2024
    21. added
      3.14bugs and security fixes
      and removed
      3.12only security fixes
      on Oct 12, 2024
    22. moved this from Bugs to Doc issues in Argparse issueson Oct 12, 2024
    23. mh-dm commented on Nov 21, 2024

      @mh-dm

      As a new user of argparse I've hit this issue and the experience was just bad and the workaround and documentation is far from optimal. Please reopen this issue!

      1. Reading through this and Argparse add_argument treats - in flags differently to - in positional arguments. #95100 there is agreement that that it would be nice if the behavior were normalized between positional and optional args.
      2. The documentation and behavior is still confusing. Do you really want every single developer that adds a two or more words positional argument to have to learn the details and differences of specifying 'foo-bar', dest='foo_bar' and 'metavar='foo-bar'?
      3. The current behavior resists any future attempts at fixing it, even in a python 4 / with a long deprecation notice.

      Let's go through the new developer experience that already has some args:

          parser.add_argument('--input-file')
          parser.add_argument('file')
          args = parser.parse_args()
          # Using args.input_file, args.file is fine

      Let's be more explicit about what 'file' is with parser.add_argument('output-file'). Using args.output_file we get:

      AttributeError: 'Namespace' object has no attribute 'output_file'
      

      Quick investigation with print(args):
      Namespace(input_file=None, **{'output-file': 'foo'})

      Weird, but I there's this dest option for specifying where I want my output to go and which a quick read of the documentation confirms: "The name of the attribute to be added to the object returned by parse_args()" and "The name of this attribute is determined by the dest keyword argument of add_argument(). For positional argument actions, dest is normally supplied as the first argument to add_argument()". So:

          parser.add_argument('output-file', dest='output_file')

      which results in

      ValueError: dest supplied twice for positional argument
      

      Excuse me, what? That's not what "normally supplied" means.

      Or, after PR #125215 the error is:
      dest supplied twice for positional argument, did you mean metavar?

      NO, I don't mean metavar. parser.add_argument('output-file', metavar='output_file') is literally the opposite of what I wanted.

      Then you read more and realize that you have to do this:

          parser.add_argument('output_file', metavar='output-file')
          # OR
          parser.add_argument(metavar='output-file', dest='output_file')

      Both versions are not intuitive and would be harder to automatically migrate in a future major release that fully fixes the issue.

      PROPOSAL

      Please fix parser.add_argument('output-file', dest='output_file') to work as expected. This fix is backwards compatible because existing code that uses it is already 'broken' (currently raises the dest supplied twice error). It's also compatible with a future fix in say a python 4 that would process parser.add_argument('output-file') just like parser.add_argument('--input-file').
      It also provides an easy automated upgrade path from python 3 to 4: any existing add_argument('foo-bar') can be machine translated to add_argument('foo-bar', dest='foo-bar').

    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.14bugs and security fixesdocsDocumentation in the Doc dirstdlibStandard 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