msg210950 - (view) |
Author: Thomas Guettler (guettli) * |
Date: 2014-02-11 15:28 |
I think the docs of argparse still contain confusing magic:
parser.parse_args('7'.split())
You know what it does and I know it. But a lot of people new to Python, don't understand what this should be.
Please use:
parser.parse_args(['7'])
Close this ticket, if you don't care for people new to Python.
|
msg210951 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-02-11 15:35 |
I agree that '7'.split() looks strange, an explicit list would be more obvious and simpler: ['7'].
'X --foo Y'.split() can be replaced with ['X', '--foo', 'Y'].
argparse examples:
http://docs.python.org/dev/library/argparse.html#type
|
msg210952 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-02-11 15:36 |
@Thomas Guettler: Would you be interested to propose a patch on the source of the documentation directly? You can find the file here:
http://hg.python.org/cpython/file/default/Doc/library/argparse.rst
|
msg210953 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2014-02-11 15:37 |
I agree that does look weird. However, it's nicely consistent with the the normal case which has multiple arguments like "-foo 3 -l".split().
I think this is an excellent thing for newcomers to try out with the interactive shell. :)
|
msg210954 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2014-02-11 15:38 |
IMO, keeping the string intact is slightly better because it's easier to read than a commandline with a bunch of quotes and commas in the middle.
|
msg210961 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-02-11 16:22 |
> IMO, keeping the string intact is slightly better because it's easier to read than a commandline with a bunch of quotes and commas in the middle.
Many other argparse examples which use list literals. I don't see quotes in >>7<< string :-)
|
msg225043 - (view) |
Author: paul j3 (paul.j3) * |
Date: 2014-08-07 22:20 |
For documentation, ['this','is','a','test'] might be a bit clearer than 'this is a test'.split(). But generating a list of strings by split is, I think, a pretty basic idiom.
But for testing purposes the split() version is a bit more robust because it is closer to what might appear in sys.argv.
In [196]: ['this','is','a','test'][-1] is 'test'
Out[196]: True
In [198]: 'this is a test'.split()[-1] is 'test'
Out[198]: False
That is, the 'id' of a string generated by a split is not the same as that of an explicit string. sys.argv is created by a split in the shell and/or the interpreter, so it too will fail this 'is' test.
|
msg236110 - (view) |
Author: Julian Berman (Julian) * |
Date: 2015-02-16 21:00 |
+1 to lists all over, this is just confusing.
|
msg259098 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-01-28 06:51 |
Here is a patch changing all the '. . .'.split() calls to lists of strings. I think the illustrations are actually less complicated to understand when you see the list directly, and they more consistent with the remaining illustrations that already use lists, but I see Benjamin disagrees.
|
msg259100 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2016-01-28 07:03 |
I don't feel that strongly about it.
|
msg259116 - (view) |
Author: SilentGhost (SilentGhost) * |
Date: 2016-01-28 09:54 |
I think it's worth saying that this wasn't a random choice intended to confuse beginners. Strings used represent input that is identical to the command-line input: if one wants to test it, one can just copy that string directly into the propmpt, it might be harder to reconstruct it from a list of strings.
|
msg259117 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2016-01-28 10:07 |
Using str.split() for splitting full command line on arguments is anti-idiom. More correct way is to use shlex. But this is overkill for argparse examples (and in most cases we already have a list sys.argv). Explicit lists look clear enough.
The patch LGTM.
|
msg259152 - (view) |
Author: paul j3 (paul.j3) * |
Date: 2016-01-28 18:38 |
I can understand changing
'7'.split()
but this change is IMO ugly:
- >>> print(parser.parse_args('--foo B cmd --arg1 XX ZZ'.split()))
+ >>> print(parser.parse_args(['--foo', 'B', 'cmd', '--arg1', 'XX', 'ZZ']))
I've answered a lot of argparse questions on Stackoverflow, and don't recall anyone being confused by the use of 'split' in the documentation. The documentation as a whole is overwealming to many users. But not this detail.
Many SO answers use the split idiom. Being aware of this issue I have edited some of my answers to use lists, even if I used split when creating them in Ipython. But only in the short cases.
Other times I copy-n-paste a complete (short) script, along with one or more sample runs (bash line plus printout). That kind of display is closer to what most new users expect and see. But it does not fit with the doctest format used in the argparse documentation.
Come to think of it, the doctest example format might be a greater hindrance to understanding than the split idiom. New users tend to construct complete scripts, and then complain that it doesn't do what they want. Often I have to ask them to print sys.argv to see what the shell is giving the parser. And to print args to see what parser is giving back. New users aren't in habit of using interactive test inputs as illustrated in the docs.
The patch proposed here may be nice in terms of consistency, but I don't think it improves readability.
|
msg259162 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-01-28 21:37 |
I don’t want to make any controversial changes. Half the purpose of the patch was to let people see what it would look like. Another option would be to only do the single or empty list changes, and leave most of the longer lists as they are with split().
|
msg260265 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-02-14 07:33 |
Here is a more conservative patch. What do you think Paul?
* Keep all the changes to single-item and empty lists
* Revert many of the other changes, except where there are only a few arguments and I feel it would make the section or piece of code too inconsistent
|
msg264259 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-04-26 11:48 |
New changeset 44f888e47ab0 by Martin Panter in branch '2.7':
Issue #20598: Replace trivial split() calls with lists in argparse docs
https://hg.python.org/cpython/rev/44f888e47ab0
New changeset 49561532504c by Martin Panter in branch '3.5':
Issue #20598: Replace trivial split() calls with lists in argparse docs
https://hg.python.org/cpython/rev/49561532504c
New changeset 14cb17682831 by Martin Panter in branch 'default':
Issue #20598: Merge argparse docs from 3.5
https://hg.python.org/cpython/rev/14cb17682831
|
msg264273 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2016-04-26 12:44 |
I committed my simpler patch. I hope that is okay :)
|
msg264282 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2016-04-26 13:40 |
Thanks. The doc looks better like that.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:58 | admin | set | github: 64797 |
2016-04-26 14:03:45 | berker.peksag | set | stage: patch review -> resolved |
2016-04-26 13:40:10 | vstinner | set | messages:
+ msg264282 |
2016-04-26 12:44:11 | martin.panter | set | status: open -> closed resolution: fixed messages:
+ msg264273
|
2016-04-26 11:48:55 | python-dev | set | nosy:
+ python-dev messages:
+ msg264259
|
2016-02-14 07:33:11 | martin.panter | set | files:
+ single-arg-list.patch
messages:
+ msg260265 stage: commit review -> patch review |
2016-01-28 21:37:40 | martin.panter | set | messages:
+ msg259162 |
2016-01-28 18:38:50 | paul.j3 | set | messages:
+ msg259152 |
2016-01-28 10:07:00 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg259117
assignee: docs@python -> martin.panter stage: patch review -> commit review |
2016-01-28 09:54:35 | SilentGhost | set | nosy:
+ SilentGhost messages:
+ msg259116
|
2016-01-28 07:03:28 | benjamin.peterson | set | messages:
+ msg259100 |
2016-01-28 06:51:27 | martin.panter | set | files:
+ arg-list.patch
versions:
+ Python 3.5, Python 3.6 keywords:
+ patch nosy:
+ martin.panter
messages:
+ msg259098 stage: patch review |
2015-02-16 21:00:55 | Julian | set | nosy:
+ Julian messages:
+ msg236110
|
2014-08-07 22:20:19 | paul.j3 | set | nosy:
+ paul.j3 messages:
+ msg225043
|
2014-02-14 18:54:52 | eric.araujo | set | nosy:
+ eric.araujo
|
2014-02-11 16:22:16 | vstinner | set | messages:
+ msg210961 |
2014-02-11 15:38:25 | benjamin.peterson | set | messages:
+ msg210954 |
2014-02-11 15:37:08 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages:
+ msg210953
|
2014-02-11 15:36:34 | vstinner | set | messages:
+ msg210952 |
2014-02-11 15:35:36 | vstinner | set | nosy:
+ vstinner messages:
+ msg210951
|
2014-02-11 15:28:25 | guettli | create | |