Skip to content

bpo-38956: remove default option from help string in argparse #17447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
michiboo wants to merge 3 commits into from
Closed

bpo-38956: remove default option from help string in argparse #17447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bbb941b
remove add default option to help strings in argparse
michiboo Dec 3, 2019
ee03060
add unit test
michiboo Dec 10, 2019
2a1d783
Add default for BooleanOptionalAction in help string
michiboo May 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Lib/argparse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,9 +873,6 @@ def __init__(self,
option_string = '--no-' + option_string[2:]
_option_strings.append(option_string)

if help is not None and default is not None:
Copy link
Contributor

@rhettinger rhettinger May 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the effect of this edit is too broad. Instead, focus on making sure the default only prints once when the formatter_class is set to ArgumentDefaultsHelpFormatter. When formatter_class isn't set, the default still needs to be shown.

Given this script:

from argparse import *

parser = ArgumentParser()
parser.add_argument("--foo", action=BooleanOptionalAction, help="Whether to foo it", default=False)
parser.add_argument("--quux", help="Set the quux", default=42)
print(parser.parse_args())

We still want this output:

$ py tmp.py --help
usage: tmp.py [-h] [--foo | --no-foo] [--quux QUUX]

optional arguments:
  -h, --help       show this help message and exit
  --foo, --no-foo  Whether to foo it (default: False)
  --quux QUUX      Set the quux

Copy link
Contributor Author

@michiboo michiboo May 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made the requested changes; please review again

Hi I have updated the code in add action function since it is not possible to get formatter class in action object, ArgumentDefaultsHelpFormatter already add default value and that was the origin problem which result in duplicate default value in the help string. Now with the default formatter class it can also show the default value.

Copy link
Contributor

@abadger abadger Jul 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @rhettinger could you speak on why the default should be displayed for BooleanOptionalArgument out-of-the-box whereas all other shipped actions require setting formatter_class=ArgumentDefaultsHelpFormatter? I do find that showing the default value is useful but it seems to break the expected separation of responsibility between action and formatter.

I ask about this since I have a different bug which would be fixed if BooleanOptionalArgument were to no longer try to add defaults on its own. If BooleanOptionalArgument should continue to do that formatting, though, I could extract the code from ArgumentDefaultsHelpFormatter and have both ArgumentDefaultsHelpFormatter and BooleanOptionalArgument use it which would likely fix my issue, this issue, and a similar class of issues that we just haven't encountered yet.

help += f" (default: {default})"

super().__init__(
option_strings=_option_strings,
dest=dest,
Expand Down Expand Up @@ -1796,6 +1793,9 @@ def add_subparsers(self, **kwargs):
return action

def _add_action(self, action):
if type(action) == BooleanOptionalAction:
if self.formatter_class == HelpFormatter and action.default is not None:
action.help += f" (default: {action.default})"
if action.option_strings:
self._optionals._add_action(action)
else:
Expand Down
21 changes: 14 additions & 7 deletions Lib/test/test_argparse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4189,6 +4189,9 @@ class TestHelpArgumentDefaults(HelpTestCase):
argument_signatures = [
Sig('--foo', help='foo help - oh and by the way, %(default)s'),
Sig('--bar', action='store_true', help='bar help'),
Sig('--taz', action=argparse.BooleanOptionalAction,
help='Whether to taz it', default=True),
Sig('--quux', help="Set the quux", default=42),
Sig('spam', help='spam help'),
Sig('badger', nargs='?', default='wooden', help='badger help'),
]
Expand All @@ -4197,25 +4200,29 @@ class TestHelpArgumentDefaults(HelpTestCase):
[Sig('--baz', type=int, default=42, help='baz help')]),
]
usage = '''\
usage: PROG [-h] [--foo FOO] [--bar] [--baz BAZ] spam [badger]
usage: PROG [-h] [--foo FOO] [--bar] [--taz | --no-taz] [--quux QUUX]
[--baz BAZ]
spam [badger]
'''
help = usage + '''\

description

positional arguments:
spam spam help
badger badger help (default: wooden)
spam spam help
badger badger help (default: wooden)

optional arguments:
-h, --help show this help message and exit
--foo FOO foo help - oh and by the way, None
--bar bar help (default: False)
-h, --help show this help message and exit
--foo FOO foo help - oh and by the way, None
--bar bar help (default: False)
--taz, --no-taz Whether to taz it (default: True)
--quux QUUX Set the quux (default: 42)

title:
description

--baz BAZ baz help (default: 42)
--baz BAZ baz help (default: 42)
'''
version = ''

Expand Down