bpo-8538: Add support for boolean actions to argparse by remilapeyre · Pull Request #11478 · python/cpython

Hi!

I was about to write a patch to add the same feature. Good thing that I decided to look if someone was already working on it, thank you :-)

I'd like to contribute with the following comments:

1. Register action rather than exposing the class

I don't think BooleanOptionalAction should be exposed to the module user. Rather, I think it should be "private" (i.e., _BooleanOptionalAction) and it should be registered like the other ones (look for calls like self.register('action', ...) in the source file). My suggestion is:

self.register('action', 'optional_boolean', _BooleanOptionalAction)

2. Be aware of short options

Your current implementation does not ignore short options when adding the prefix '--no-'. They should be ignored IMO.

3. Ordering of parameters names

I think the --no-<option_name> variants should be next to --<option_name> in the list. Consider the following case where the user decided to have 2 names for the same option:

parser.add_argument('--foo', '--bar', action='optional_boolean', ...)

Then, in the help message, --foo and --bar would appear before --no-foo and --no-bar.

4. Documentation

In the case of registering the action 'optional_boolean', it would be necessary to update the documentation on the list of supported values for the action keyword of add_argument.

5. My custom action class

About (2) and (3), I have a custom implementation in one project of mine that does that:

class BoolAction(argparse.Action):
    def __init__(self, option_strings, dest, **kw):
        self.original_option_strings = option_strings

        kw['nargs'] = 0

        option_strings = []
        for s in self.original_option_strings:
            option_strings.append(s)

            if s.startswith('--'):
                s = '--no-' + s[2:]
                option_strings.append(s)

        super(BoolAction, self).__init__(option_strings, dest, **kw)

    def __call__(self, parser, namespace, values, option_string):
        value = option_string in self.original_option_strings
        setattr(namespace, self.dest, value)