classification
Title: Undocumented (?) behaviour change in argparse from 3.2.3 to 3.3.0
Type: Stage:
Components: Library (Lib) Versions: Python 3.3, Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, asvetlov, bethard, epicfaace, maker, paul.j3, r.david.murray, telmich
Priority: normal Keywords:

Created on 2012-10-24 09:22 by telmich, last changed 2019-08-25 00:50 by paul.j3.

Files
File name Uploaded Description Edit
subparse.py telmich, 2012-10-24 13:44
subparse2.py telmich, 2012-10-24 14:12
subparse3.py telmich, 2012-10-24 14:17
Messages (17)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python triager) 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) * (Python triager) 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
History
Date User Action Args
2019-08-25 00:50:08paul.j3setmessages: + msg350415
2019-08-24 23:46:54epicfaacesetnosy: + epicfaace
messages: + msg350412
2013-05-01 20:16:26makersetnosy: + maker
2013-05-01 17:09:47r.david.murraylinkissue17889 superseder
2013-04-19 05:35:21paul.j3setnosy: + paul.j3
messages: + msg187343
2013-04-18 14:36:59r.david.murraysetmessages: + msg187255
versions: + Python 3.4
2012-12-10 14:39:44Arfreversetnosy: + Arfrever
2012-12-09 21:45:11telmichsetmessages: + msg177237
2012-11-08 14:26:52telmichsetmessages: + msg175167
2012-10-28 12:29:39asvetlovsetnosy: + asvetlov
2012-10-24 15:08:14telmichsetmessages: + msg173692
2012-10-24 15:06:53telmichsetmessages: + msg173691
2012-10-24 14:40:49r.david.murraysetmessages: + msg173690
2012-10-24 14:30:25telmichsetmessages: + msg173688
2012-10-24 14:17:18telmichsetfiles: + subparse3.py

messages: + msg173686
2012-10-24 14:12:14telmichsetfiles: + subparse2.py

messages: + msg173685
2012-10-24 13:44:20telmichsetfiles: + subparse.py

messages: + msg173684
2012-10-24 13:20:39r.david.murraysetmessages: + msg173683
2012-10-24 13:11:18telmichsetmessages: + msg173681
2012-10-24 12:17:54r.david.murraysetnosy: + r.david.murray, bethard
messages: + msg173672
2012-10-24 09:22:33telmichcreate