gh-62090: Simplify argparse usage formatting #105039
Conversation
Rationale ========= argparse performs a complex formatting of the usage for argument grouping and for line wrapping to fit the terminal width. This formatting has been a constant source of bugs for at least 10 years (see linked issues below) where defensive assertion errors are triggered or brackets and paranthesis are not properly handeled. Problem ======= The current implementation of argparse usage formatting relies on regular expressions to group arguments usage only to separate them again later with another set of regular expressions. This is a complex and error prone approach that caused all the issues linked below. Special casing certain argument formats has not solved the problem. The following are some of the most common issues: - empty `metavar` - mutually exclusive groups with `SUPPRESS`ed arguments - metavars with whitespace - metavars with brackets or paranthesis Solution ======== The following two comments summarize the solution: - python#82091 (comment) - python#77048 (comment) Mainly, the solution is to rewrite the usage formatting to avoid the group-then-separate approach. Instead, the usage parts are kept separate and only joined together at the end. This allows for a much simpler implementation that is easier to understand and maintain. It avoids the regular expressions approach and fixes the corresponding issues. This closes the following issues: - Closes python#62090 - Closes python#62549 - Closes python#77048 - Closes python#82091 - Closes python#89743 - Closes python#96310 - Closes python#98666 These PRs become obsolete: - Closes python#15372 - Closes python#96311
|
All commit authors signed the Contributor License Agreement. |
|
@rhettinger @hpaulj I’d like to get your opinion on this change if you don’t mind.
|
|
I agree that the existing usage formatter is too brittle. 9 years ago I tried to write an alternative as part of a project to generalize the concept of mutually-exclusive-groups, allowing other relations (and/or/any etc), and nesting. https://bugs.python.org/issue11588. I can't offhand find my best usage patch, but agree with the idea of building usage groups, and combining them at the end. But currently I'm not as involved with github and argparse. I'm commenting on some issues, but have not contributed (or tested) a patch (in the old style) or github push in a long time. |
|
A simplification sounds good to me. I'll start reading up on argparse so I can review this in detail. |
|
This does not handle nested exclusive groups correctly. Nested exclusive groups are deprecated, but still supported for now. For example: Without this change, the output is not ideal, but technically correct: But with this change, the argument added at the end ( One fix would be to pad the new sequence with |
|
We need to be very careful when nesting groups. What's the intent, and
what's the actual parsing action? Carefully consider all the discussion
dealing with the deprecation. It isn't just because the usage formatting is
broken.
…On Mon, Apr 15, 2024, 7:31 AM Petr Viktorin ***@***.***> wrote:
This does not handle nested exclusive groups correctly. Nested exclusive
groups are deprecated, but still supported for now. For example:
import argparse
parser = argparse.ArgumentParser()
g=parser.add_mutually_exclusive_group()
g.add_argument('--spam')
gg=g.add_mutually_exclusive_group()
gg.add_argument('--hax')
gg.add_argument('--hex')
g.add_argument('--eggs')
parser.add_argument('--num')
print(parser.format_usage())
Without this change, the output is not ideal, but technically correct:
/tmp/repro.py:5: DeprecationWarning: Nesting mutually exclusive groups is deprecated.
gg=g.add_mutually_exclusive_group()
usage: repro.py [-h] [--spam SPAM | [--hax HAX | --hex HEX |] --eggs EGGS] [--num NUM]
But with this change, the argument added at the end (--num) joins the
group:
/tmp/repro.py:5: DeprecationWarning: Nesting mutually exclusive groups is deprecated.
gg=g.add_mutually_exclusive_group()
usage: repro.py [-h] [--spam SPAM | [--hax HAX | --hex HEX] | --eggs EGGS | [--num NUM]]
One fix would be to pad the new sequence with None to keep the original
length when replacing parts[start:end] at the end. That way the meaning
of indices wouldn't change
—
Reply to this email directly, view it on GitHub
<#105039 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAITB6ES7PAKCXDOQTHIGH3Y5PQDPAVCNFSM6AAAAAAYRZ6D3CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJXGAYDINRSHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I'm aware that nested groups are broken and deprecated. But, their behaviour should not be made worse. This PR, as it is, breaks unrelated usage formatting in the presence of nested groups: the |
|
Thank you both for the valuable feedback, I've addressed the case of nested groups and tested several variations with/without suppressed actions locally and they all produce the correct usage text now. |
|
Thank you for the fix! |
Rationale ========= argparse performs a complex formatting of the usage for argument grouping and for line wrapping to fit the terminal width. This formatting has been a constant source of bugs for at least 10 years (see linked issues below) where defensive assertion errors are triggered or brackets and paranthesis are not properly handeled. Problem ======= The current implementation of argparse usage formatting relies on regular expressions to group arguments usage only to separate them again later with another set of regular expressions. This is a complex and error prone approach that caused all the issues linked below. Special casing certain argument formats has not solved the problem. The following are some of the most common issues: - empty `metavar` - mutually exclusive groups with `SUPPRESS`ed arguments - metavars with whitespace - metavars with brackets or paranthesis Solution ======== The following two comments summarize the solution: - python#82091 (comment) - python#77048 (comment) Mainly, the solution is to rewrite the usage formatting to avoid the group-then-separate approach. Instead, the usage parts are kept separate and only joined together at the end. This allows for a much simpler implementation that is easier to understand and maintain. It avoids the regular expressions approach and fixes the corresponding issues. This closes the following GitHub issues: - python#62090 - python#62549 - python#77048 - python#82091 - python#89743 - python#96310 - python#98666 These PRs become obsolete: - python#15372 - python#96311
Rationale
argparse performs a complex formatting of the usage for argument grouping and for line wrapping to fit the terminal width. This formatting has been a constant source of bugs for at least 10 years (see linked issues below) where defensive assertion errors are triggered or brackets and paranthesis are not properly handeled.
Problem
The current implementation of argparse usage formatting relies on regular expressions to group arguments usage only to separate them again later with another set of regular expressions. This is a complex and error prone approach that caused all the issues linked below. Special casing certain argument formats has not solved the problem. The following are some of the most common issues:
metavarSUPPRESSed argumentsSolution
The following two comments summarize the solution:
Mainly, the solution is to rewrite the usage formatting to avoid the group-then-separate approach. Instead, the usage parts are kept separate and only joined together at the end. This allows for a much simpler implementation that is easier to understand and maintain. It avoids the regular expressions approach and fixes the corresponding issues.
This closes the following issues:
These PRs become obsolete: