Undocumented (?) behaviour change in argparse from 3.2.3 to 3.3.0

[telmich]

BPO	16308
Nosy	@bitdancer, @asvetlov, @mmaker, @epicfaace
Files	subparse.py subparse2.py subparse3.py

^{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-10-24.09:22:33.064>
labels = ['library']
title = 'Undocumented (?) behaviour change in argparse from 3.2.3 to 3.3.0'
updated_at = <Date 2019-08-25.00:50:08.945>
user = 'https://bugs.python.org/telmich'

bugs.python.org fields:

activity = <Date 2019-08-25.00:50:08.945>
actor = 'paul.j3'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2012-10-24.09:22:33.064>
creator = 'telmich'
dependencies = []
files = ['27694', '27695', '27696']
hgrepos = []
issue_num = 16308
keywords = []
message_count = 17.0
messages = ['173659', '173672', '173681', '173683', '173684', '173685', '173686', '173688', '173690', '173691', '173692', '175167', '177237', '187255', '187343', '350412', '350415']
nosy_count = 8.0
nosy_names = ['bethard', 'Arfrever', 'r.david.murray', 'asvetlov', 'maker', 'paul.j3', 'telmich', 'epicfaace']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue16308'
versions = ['Python 3.3', 'Python 3.4']

[telmich]

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

[bitdancer]

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.

[telmich]

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: bpo-10424: argument names are now included in the missing argument message

[bitdancer]

Thanks. I've put this on my todo list to look at this weekend during the bug day.

[telmich]

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'

[telmich]

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.

[telmich]

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)

[telmich]

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.

[bitdancer]

How does all of this relate to bpo-9253?

[telmich]

Interesting! The bpo-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 bpo-9523, though the other way round: in bpo-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 bpo-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 bpo-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.

[telmich]

s/9523/9253/g in my previous post

[telmich]

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

[telmich]

Anyone alive at bugs.python.org?

[bitdancer]

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 bpo-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 bpo-9253 as is.

The issue with (1) is that the discussion in bpo-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.