classification
Title: argparse: mutually exclusive groups full of help-suppressed args can cause AssertionErrors
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Yogesh.Chaudhari, bethard, gholms, martin.panter, paul.j3, r.david.murray, terry.reedy
Priority: normal Keywords: patch

Created on 2013-05-02 00:43 by gholms, last changed 2016-01-29 03:31 by martin.panter.

Files
File name Uploaded Description Edit
argparse-assertfail.py gholms, 2013-05-02 00:43 Test case
7691d1d4b955.diff Yogesh.Chaudhari, 2013-05-02 13:53 gholms repo review
Issue17890-27.patch Yogesh.Chaudhari, 2013-05-02 16:22 Resolution for Python 2.7 issue17890 review
Repositories containing patches
https://bitbucket.org/gholms/cpython
Messages (16)
msg188250 - (view) Author: Garrett Holmstrom (gholms) * Date: 2013-05-02 00:43
When it goes to format a usage message, argparse seems to (correctly) fail to satisfy one of its assertions when all of the following are true:

1. A mutually exclusive group contains only args that are suppressed
2. An unsuppressed arg follows that group
3. The usage is long enough to need to line-wrap

The cause seems to be that the set of regular expressions that argparse uses to clean up mutually exclusive groups' separators doesn't handle the space that follows what would otherwise be an empty pair of square braces, sort of like this:

1. [-h] [ ] [--spam] ...
2. [-h] [] [--spam] ...
3. [-h]  [--spam] ...

A test case is attached.  I was able to reproduce this with python-2.7.3-13.fc18.x86_64 on Fedora as well as with commit 83588:e6b962fa44bb in 3.4 mainline.  I have a small patch for the latter that I'll submit shortly.

Sorry if I missed anything.  This is my first bug report against python proper.
msg188253 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013-05-02 07:04
Looks like the 

text = text.strip()

at the end of the set of regex (in _format_actions_usage) needs to be replaced with something that removes all excess spaces, e.g.

text = _re.sub( '\s+', ' ', text ).strip()
msg188278 - (view) Author: Yogesh Chaudhari (Yogesh.Chaudhari) * Date: 2013-05-02 16:22
Made similar required changes for version 2.7
msg188291 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013-05-03 05:23
In the test case: class TestMutuallyExclusiveManySuppressed
even with a short 'eggs' argument, there is a difference

Old usage would be:

usage: PROG [-h]  [--eggs EGGS]

new

usage: PROG [-h] [--eggs EGGS]

i.e. 2 v 1 space.  But extra spaces are not as dramatic a failure as an assertion error.

It would also be good to check what happens when there are 2 suppressed groups.  If the text before all trimming is:

[ -h ] [] () [ --eggs EGGS ]

does it reduce to?

[-h] [--eggs EGGS]

The old code would have left 3 spaces.

I can't think of a situation in which a user would want a (generated) usage line with multiple spaces.  If some sort of special formatting is needed, there is always the option of specifying an explicit usage line.

parser = ArugmentParser(usage='one \ttwo  \nthree   four')
msg188295 - (view) Author: Yogesh Chaudhari (Yogesh.Chaudhari) * Date: 2013-05-03 08:45
A user generated line with multiple spaces may be essential if we are getting the usage data (particularly from) a csv file (that may have random spaces in certain fields) or some other form of stored or auto-generated data.
msg188306 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013-05-03 16:56
I see three solutions - 

1) gholms' patch which removes '() ' and [] '

2) Yogesh's patch which removes all duplicated spaces.

3) remove the 2 asserts. 

The first 2 do the same thing most of the time, but may differ if the user somehow inserts spaces into names.  The third leaves the extra blanks, but renders them innocuous.  I doubt if the asserts were written to catch this problem.  They probably were included to verify the usage line had been split up as expected prior to reassembling on multiple lines.

As best I can tell test_argparse.py does not test for these spaces.  Curiously though a port of argparse to javascript does have a test case with the extra space.
msg188337 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-05-03 23:59
Garrett and Yogesh, please submit contributor license agreements
http://www.python.org/psf/contrib/contrib-form/
if you have not yet. When one is properly recorded, a * appears after your name.
msg188379 - (view) Author: Yogesh Chaudhari (Yogesh.Chaudhari) * Date: 2013-05-04 17:44
I am not sure if I am missing something. I had filled out the form at http://www.python.org/psf/contrib/contrib-form/ on the day I submitted the patch and even got back an email from Ewa Jodlowska. However, I don't see any "*" after my name. I submitted the same form to contributors@python.org and shot a mail to python-dev mailing list but there is no change. Any suggestions?
msg188413 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-05-04 22:00
Wait a week and see what happens.
msg188984 - (view) Author: Yogesh Chaudhari (Yogesh.Chaudhari) * Date: 2013-05-12 06:56
@Terry: Thanks for the info. I seem to have the elusive "*" after my username now. I am not sure how this works, but can you review/test/apply the patch now?
msg189054 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-05-12 19:22
Argparse is out of my area of competence/experience. I have added a couple of people who have worked on argparse in the past and should be better able to review or suggest another reviewer.
msg189064 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013-05-12 21:25
I'm following a dozen argparse issues with patches.  I haven't seen much posting by argparse experts (like bethard, david.murry) since last December.
msg189065 - (view) Author: Yogesh Chaudhari (Yogesh.Chaudhari) * Date: 2013-05-12 21:41
@terry and @paul:
I have 2 argparse related patches, sitting idle for quiet sometime now. Can you suggest any more names who can take this forward? (I could not find anyone else related to argparse in http://docs.python.org/devguide/experts.html)
msg189355 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-05-16 10:56
I've been observing the activity on the argparse issues and am appreciating the work, but I don't have time right now to review the patches.  I should have more time next month, and expect to get to them then, if no one else gets to them before I do.
msg193191 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2013-07-16 20:53
I just submitted a patch to http://bugs.python.org/issue11874 that substantially rewrites _format_actions_usage().  It generates the group and action parts separately, and does not do the kind of cleanup that produces this issue.
msg259189 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-01-29 03:31
See duplicate Issue 22363 for the traceback, and a workaround. There is also a patch there similar to Garrett’s original fix, but using the RE r'%s *%s ?'.

I restored the diff from Garrett’s repository, in case it is still useful.

Yogesh: I am not familiar with how the code works, and I struggle to be excited at using regular expressions to build a usage message :). But can you explain why you changed Garrett’s original fix? Does it affect intentional double spaces (or tabs, non-ASCII spaces, etc) in customizable parts of the usage message?
History
Date User Action Args
2016-01-29 03:31:48martin.pantersetnosy: + martin.panter
title: argparse: mutually exclusive groups full of suppressed args can cause AssertionErrors -> argparse: mutually exclusive groups full of help-suppressed args can cause AssertionErrors
messages: + msg259189

stage: patch review
2016-01-29 03:30:00martin.panterlinkissue22363 superseder
2016-01-29 03:04:18martin.pantersetfiles: + 7691d1d4b955.diff
2013-07-16 20:53:23paul.j3setmessages: + msg193191
2013-05-16 10:56:43r.david.murraysetmessages: + msg189355
2013-05-12 21:41:23Yogesh.Chaudharisetmessages: + msg189065
2013-05-12 21:25:37paul.j3setmessages: + msg189064
2013-05-12 19:22:24terry.reedysetnosy: + bethard, r.david.murray
messages: + msg189054
2013-05-12 06:56:34Yogesh.Chaudharisetmessages: + msg188984
2013-05-04 22:00:48terry.reedysetmessages: + msg188413
2013-05-04 17:44:44Yogesh.Chaudharisetmessages: + msg188379
2013-05-03 23:59:38terry.reedysetnosy: + terry.reedy

messages: + msg188337
versions: + Python 3.3
2013-05-03 16:56:02paul.j3setmessages: + msg188306
2013-05-03 08:45:25Yogesh.Chaudharisetmessages: + msg188295
2013-05-03 05:23:25paul.j3setmessages: + msg188291
2013-05-02 16:26:17Yogesh.Chaudharisetfiles: - 7691d1d4b955.diff
2013-05-02 16:22:44Yogesh.Chaudharisetfiles: + Issue17890-27.patch
nosy: + Yogesh.Chaudhari
messages: + msg188278

2013-05-02 13:53:44Yogesh.Chaudharisetfiles: + 7691d1d4b955.diff
keywords: + patch
2013-05-02 07:04:20paul.j3setnosy: + paul.j3
messages: + msg188253
2013-05-02 00:49:37gholmssethgrepos: + hgrepo187
2013-05-02 00:43:02gholmscreate