msg173659 - (view) |
Author: (telmich) |
Date: 2012-10-24 09:22 |
Using argparse with subparsers, the default behaviour if no subparser was selected, was to print help:
cdist% git describe
2.0.14-59-g5315c41
cdist% ./bin/cdist
usage: cdist [-h] [-d] [-v] [-V] {banner,config} ...
cdist: error: too few arguments
cdist% python3 -V
Python 3.2.3
With python 3.3.0, this changed and thus the so far uncalled code block is executed:
cdist% ./bin/cdist
Traceback (most recent call last):
File "./bin/cdist", line 237, in <module>
commandline()
File "./bin/cdist", line 104, in commandline
args.func(args)
AttributeError: 'Namespace' object has no attribute 'func'
cdist% git describe
2.0.14-59-g5315c41
cdist% python3 -V
Python 3.3.0
Question:
What is the correct way now to abort, if no subparser or
no option was given (i.e. restore the previous behaviour)?
I tried to fix this problem by using
parser['main'].set_defaults(func=commandline_main)
to setup a default handler for main, but this overwrites the func= parameter for all subparsers as well and thus renders this possibility useless (commits b7a8a84 and 840dbc5 in cdist).
The source code for cdist using the subparsers is available at http://git.schottelius.org/?p=cdist
|
msg173672 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2012-10-24 12:17 |
There was bug fixing/enhancement activity around subparsers, one of them probably had an unexpected side effect for which there are currently no tests. If you can figure out which change set introduced the change, that might speed up resolution of the issue.
|
msg173681 - (view) |
Author: (telmich) |
Date: 2012-10-24 13:11 |
Having a closer look at the changes using hg diff -r v3.2:v3.3.0 Lib/argparse.py, it seems the following removal could be related to the different behaviour:
- # if we didn't use all the Positional objects, there were too few
- # arg strings supplied.
- if positionals:
- self.error(_('too few arguments'))
-
[15:09] brief:cpython% hg grep "if positionals:" Lib/argparse.py
Lib/argparse.py:79016: if positionals:
And this seems to be the relevant change:
changeset: 70741:cab204a79e09
user: R David Murray <rdmurray@bitdance.com>
date: Thu Jun 09 12:34:07 2011 -0400
summary: #10424: argument names are now included in the missing argument message
|
msg173683 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2012-10-24 13:20 |
Thanks. I've put this on my todo list to look at this weekend during the bug day.
|
msg173684 - (view) |
Author: (telmich) |
Date: 2012-10-24 13:44 |
Thanks a lot - let me know when I can help more.
After re-reading the documentation, I think the behaviour for subparsers is not specified: It is not specified, whether a subparser is required to be present or not.
Attached stripped down version of the cdist code to a bare minimum (subparse.py) which shows the problem more clear:
% python3 -V
Python 3.2.3
% python3 ~/subparse.py
usage: subparse.py [-h] [-V] {banner} ...
subparse.py: error: too few arguments
versus
[15:44] brief:~% python3 -V
Python 3.3.0
[15:46] brief:~% python3 subparse.py
Traceback (most recent call last):
File "subparse.py", line 31, in <module>
commandline()
File "subparse.py", line 25, in commandline
args.func(args)
AttributeError: 'Namespace' object has no attribute 'func'
|
msg173685 - (view) |
Author: (telmich) |
Date: 2012-10-24 14:12 |
I've copy & pasted the example from the documentation and added the following lines to it (attached full file)
args = parser.parse_args('')
args.func(args)
Following the style / way show in the documentation, I'd expect this block to work (3.2.2 behaviour). I do however understand from a logical point that this does not work:
* no subparsers are required
* no argument is required
=> args.func is never setup
One could check for this situation using getattr:
try:
a = getattr(args, "func")
except AttributeError:
parser['main'].print_help()
sys.exit(0)
Though this look quite manual compared to the usual way of using argparse.
|
msg173686 - (view) |
Author: (telmich) |
Date: 2012-10-24 14:17 |
Assuming 3.3.0 behaviour is correct (as in no subparser specified, args.func not setup), I'd expect to have setup a function on the main parser. Trying this, it prevents the subparsers function from being completly:
[16:03] brief:~% python3 subparse3.py
main: Namespace(func=<function main at 0x7fdf844b90e0>, x=2, y=1.0)
main: Namespace(func=<function main at 0x7fdf844b90e0>, z='XYZYX')
main: Namespace(func=<function main at 0x7fdf844b90e0>)
Added lines to the original documentation are:
def main(args):
print('main: %s' % args)
...
parser.set_defaults(func=main)
...
args = parser.parse_args('')
args.func(args)
|
msg173688 - (view) |
Author: (telmich) |
Date: 2012-10-24 14:30 |
Proposal / resume from the previous tests:
- add a parameter to add_subparsers(): required
If required=True, one of the subcommand names need to be present.
If required=False (default), allow parse_args() to return successfully, if no subcommand is given.
- Change implementation in such a way that using parser.set_defaults() on the main parser does not affect the sub parsers: Sub parsers are not related to the main parser like parent parsers and thus should not be affected by changes in the main parser.
The second change would allow catching the case of no subcommands given and can be used to catch the empty arguments case.
|
msg173690 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2012-10-24 14:40 |
How does all of this relate to issue 9253?
|
msg173691 - (view) |
Author: (telmich) |
Date: 2012-10-24 15:06 |
Interesting! The issue 9523 has now somehow become obsolete,
as in 3.3.0 subcommands are de-facto optional, which is the problem I am facing.
Regarding my last comment, here are the relations:
- add a parameter to add_subparsers(): required
This is a similar proposal to the original issue 9523, though the other way round: in #9523 the required case was proposed to be the default. As 3.3.0 now defaults to optional, my proposal was to keep this behaviour and allow passing required=True.
- Change implementation in such a way that using parser.set_defaults() on the main parser does not affect the sub parsers:
As set_defaults() already exists, I think this is the cleaner method to implement this feature instead of using the default='' variant specified in #9523.
Besides that I think the current behaviour of set_defaults changing the functions of the sub parsers is definitely a bug on its own.
So summarised / compared:
- Python 3.3.0 seems to treat subcommands as optional, compared to the original issue 9523
- set_defaults(func=x) on the main parsers affects sub parsers, which renders using set_defaults() useless
- If requiring the behaviour of set_defaults() to pass the func= arguments to other parsers, the parents=[] argument already exists
- To fix this issue, only correcting the set_defaults() call would be necessary
- It makes sense though, to add the 'required' parameter, as the need has also been spotted by others already.
|
msg173692 - (view) |
Author: (telmich) |
Date: 2012-10-24 15:08 |
s/9523/9253/g in my previous post
|
msg175167 - (view) |
Author: (telmich) |
Date: 2012-11-08 14:26 |
Any news on this one?
If the proposed changes are ok for you, I'd implement two patches:
- fix set_defaults() to not propagate func= to child parsers => makes it usable
- add required= argument to add_subparsers, defaulting to False
|
msg177237 - (view) |
Author: (telmich) |
Date: 2012-12-09 21:45 |
Anyone alive at bugs.python.org?
|
msg187255 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-04-18 14:36 |
Yes, just with limited amounts of attention to spare :(
This is a rather difficult situation, since it appears we inadvertently changed a behavior. Fortunately it was in a feature release. Unfortunately a new parameter to make a subparser required again can't be done in a bug fix release.
There seem to be two ways to handle this:
(1) accept the new feature. Then the bug fix should be to make the traceback not happen, and we should update the docs to mention the new behavior as added in 3.3. Then we change issue 9253 to be about adding the required to subparsers in 3.4 with a default of False.
(2) treat this as a bug. In that case the bug fix would be to restore the old required=True behavior, and making them optional again is an enhancement for 3.4 covered by issue 9253 as is.
The issue with (1) is that the discussion in #9253 concluded that the previous default behavior was "safer" for users.
In either case, we need a unit test that tests the behavior.
I think we need input either from Steven or from other Python core devs on which way to go, because I can't make up my mind :) A post to python-dev may be in order, since we haven't heard anything from Steven for a while now.
|
msg187343 - (view) |
Author: paul j3 (paul.j3) * |
Date: 2013-04-19 05:35 |
I submitted a patch to http://bugs.python.org/issue9253
The reason why subparser became optional in the latest release is that a old test for missing positionals was phased out, and new one added. Subparsers have always had a default 'required=False', but the old test did not actually pay attention to that attribute.
It is easy to change the default attribute, but that ends up exposing a bug in the new test (trying to ','.join[...,None,...]).
My patch corrects that, and exposes the 'required' attribute. It also has doc changes and test additions.
I could rewrite it so it does not change the API or the doc.
|
msg350412 - (view) |
Author: Ashwin Ramaswami (epicfaace) * |
Date: 2019-08-24 23:46 |
What's the status of this? Was paul.j3's patch ever reviewed?
|
msg350415 - (view) |
Author: paul j3 (paul.j3) * |
Date: 2019-08-25 00:50 |
https://bugs.python.org/issue26510
https://bugs.python.org/issue33109
There was some flip/flop over whether required should be true by default or not - the current behavior is False, (the 3.3.0)
The lasting change since this issue in 2012 is that `add_subparsers` now takes the 'required' parameter. And this is documented.
To avoid the problem raising in this issue, the subparsers should be defined with:
p1 = parser.add_subparsers(required=True, dest='cmd')
The 'dest' parameter is documented, but that fact that it is needed with required=True is not. Without it you can get an error when missing-argument is being formatted. Without it the error formatter doesn't know what to call this argument. I've mentioned this in one or more issues.
See my SO answer for discussion on this:
https://stackoverflow.com/questions/23349349/argparse-with-required-subparser/23354355#23354355
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:37 | admin | set | github: 60512 |
2019-08-25 00:50:08 | paul.j3 | set | messages:
+ msg350415 |
2019-08-24 23:46:54 | epicfaace | set | nosy:
+ epicfaace messages:
+ msg350412
|
2013-05-01 20:16:26 | maker | set | nosy:
+ maker
|
2013-05-01 17:09:47 | r.david.murray | link | issue17889 superseder |
2013-04-19 05:35:21 | paul.j3 | set | nosy:
+ paul.j3 messages:
+ msg187343
|
2013-04-18 14:36:59 | r.david.murray | set | messages:
+ msg187255 versions:
+ Python 3.4 |
2012-12-10 14:39:44 | Arfrever | set | nosy:
+ Arfrever
|
2012-12-09 21:45:11 | telmich | set | messages:
+ msg177237 |
2012-11-08 14:26:52 | telmich | set | messages:
+ msg175167 |
2012-10-28 12:29:39 | asvetlov | set | nosy:
+ asvetlov
|
2012-10-24 15:08:14 | telmich | set | messages:
+ msg173692 |
2012-10-24 15:06:53 | telmich | set | messages:
+ msg173691 |
2012-10-24 14:40:49 | r.david.murray | set | messages:
+ msg173690 |
2012-10-24 14:30:25 | telmich | set | messages:
+ msg173688 |
2012-10-24 14:17:18 | telmich | set | files:
+ subparse3.py
messages:
+ msg173686 |
2012-10-24 14:12:14 | telmich | set | files:
+ subparse2.py
messages:
+ msg173685 |
2012-10-24 13:44:20 | telmich | set | files:
+ subparse.py
messages:
+ msg173684 |
2012-10-24 13:20:39 | r.david.murray | set | messages:
+ msg173683 |
2012-10-24 13:11:18 | telmich | set | messages:
+ msg173681 |
2012-10-24 12:17:54 | r.david.murray | set | nosy:
+ r.david.murray, bethard messages:
+ msg173672
|
2012-10-24 09:22:33 | telmich | create | |