classification
Title: argparse: positional arguments containing - in name not handled well
Type: behavior Stage: needs patch
Components: Documentation Versions: Python 3.2, Python 3.3, Python 3.4, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: bethard, docs@python, eric.araujo, flox, khughitt, mapleoin, martin.panter, nstiurca, paul.j3, r.david.murray, sfllaw, tshepang
Priority: normal Keywords: easy, patch

Created on 2012-06-21 15:34 by nstiurca, last changed 2014-08-23 13:03 by khughitt.

Files
File name Uploaded Description Edit
argparse-argument-names.diff mapleoin, 2012-07-07 10:24 review
15125-1.patch sfllaw, 2012-11-03 18:01
15125-2.patch sfllaw, 2012-11-03 19:25 review
Messages (20)
msg163340 - (view) Author: Nicu Stiurca (nstiurca) Date: 2012-06-21 15:34
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.
msg163383 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-06-22 01:06
It does.
msg163456 - (view) Author: Nicu Stiurca (nstiurca) Date: 2012-06-22 18:45
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
msg163457 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-06-22 18:49
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.
msg163461 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-06-22 19:00
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?
msg164789 - (view) Author: Ionuț Arțăriși (mapleoin) Date: 2012-07-07 08:10
I'm working on this right now as part of EuroPython's CPython sprint.
msg164792 - (view) Author: Florent Xicluna (flox) * (Python committer) Date: 2012-07-07 09:21
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.
msg164808 - (view) Author: Ionuț Arțăriși (mapleoin) Date: 2012-07-07 10:24
I agree with Florent that this is maybe just a documentation issue, since the argument is accessible via getattr().
msg164968 - (view) Author: Nicu Stiurca (nstiurca) Date: 2012-07-08 06:50
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. :/
msg165060 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-07-09 05:00
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).
msg165067 - (view) Author: Nicu Stiurca (nstiurca) Date: 2012-07-09 05:55
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.)
msg166181 - (view) Author: Steven Bethard (bethard) * (Python committer) Date: 2012-07-22 23:27
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).
msg174655 - (view) Author: Simon Law (sfllaw) * Date: 2012-11-03 17:58
>> 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')

I don't think that making ``dest`` more magical is a good idea. In the
included patch, you'll find a change that makes the ValueError tell
people about ``metavar``, which is the right way to go about things
anyway.

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

In the included patch.

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

If we make optional and positional arguments consistent, and provide
backwards-compatibility for positional arguments, then these two are
not necesssary.
msg174656 - (view) Author: Simon Law (sfllaw) * Date: 2012-11-03 18:01
Sorry, there was a small typo in the previous patch. Here's the newer version.
msg174660 - (view) Author: Simon Law (sfllaw) * Date: 2012-11-03 18:26
Note that 15125-1.patch applies to Python 2.7 cleanly as it is a bugfix.
msg174677 - (view) Author: Simon Law (sfllaw) * Date: 2012-11-03 19:25
15125-2.patch applies to the default branch.

It makes dest behave the same for positional and optional arguments in terms of name mangling.

Also, there is a backward-compatibility path in Namespace to support old-style getattr() access. However, it's not documented as we really don't want people to use it.
msg174725 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-11-04 00:39
Will apply the doc patches soon™ and wait for Steven’s feedback about the 3.4 behavior change.
msg225527 - (view) Author: Keith Hughitt (khughitt) Date: 2014-08-19 10:27
Any progress on this issue? Still persists in Python 3.4.1.
msg225628 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-08-22 00:54
It still needs a documentation patch.  

And if the documentation was clearer, would sfllaw's patch still be needed?
msg225744 - (view) Author: Keith Hughitt (khughitt) Date: 2014-08-23 13:03
Although it would be nice if the behavior were normalized between positional and optional args, it seems like doc patch would be the most straight-forward at this point. The down-side is that I suspect many people will assume the behavior for optional args, have it fail, and then consults the docs to see what happened.

I would suggest making the note pretty obvious or early on in the docs for argparse.
History
Date User Action Args
2014-08-23 13:03:01khughittsetmessages: + msg225744
2014-08-22 00:54:16paul.j3setmessages: + msg225628
2014-08-19 10:27:02khughittsetnosy: + khughitt
messages: + msg225527
2013-08-24 06:10:38martin.pantersetnosy: + martin.panter
2013-05-28 00:14:10r.david.murraylinkissue18074 superseder
2013-05-17 14:31:08r.david.murraysetnosy: + paul.j3
2013-05-17 14:27:19r.david.murraylinkissue17965 superseder
2012-11-04 00:39:29eric.araujosetmessages: + msg174725
2012-11-03 19:25:20sfllawsetfiles: + 15125-2.patch

messages: + msg174677
2012-11-03 18:26:47sfllawsetmessages: + msg174660
2012-11-03 18:15:19eric.araujosetfiles: - 15125-1.patch
2012-11-03 18:01:11sfllawsetfiles: + 15125-1.patch

messages: + msg174656
2012-11-03 17:58:08sfllawsetfiles: + 15125-1.patch
nosy: + sfllaw
messages: + msg174655

2012-07-22 23:27:30bethardsetversions: + Python 3.4
nosy: + docs@python

messages: + msg166181

assignee: docs@python
components: + Documentation
2012-07-09 05:55:08nstiurcasetmessages: + msg165067
2012-07-09 05:00:01eric.araujosetnosy: + eric.araujo
messages: + msg165060
2012-07-08 06:50:40nstiurcasetmessages: + msg164968
2012-07-07 10:24:47mapleoinsetfiles: + argparse-argument-names.diff
keywords: + patch
messages: + msg164808
2012-07-07 09:21:22floxsetnosy: + flox
messages: + msg164792
2012-07-07 08:10:22mapleoinsetnosy: + mapleoin
messages: + msg164789
2012-06-22 19:00:11r.david.murraysetmessages: + msg163461
2012-06-22 18:49:11r.david.murraysetmessages: + msg163457
versions: - Python 2.6
2012-06-22 18:45:57nstiurcasetmessages: + msg163456
versions: + Python 2.6
2012-06-22 17:48:20tshepangsetnosy: + tshepang
2012-06-22 01:06:07r.david.murraysetversions: + Python 2.7, Python 3.2, Python 3.3, - Python 2.6
nosy: + r.david.murray, bethard

messages: + msg163383

keywords: + easy
stage: needs patch
2012-06-21 15:34:07nstiurcacreate