argparse: default args in mutually exclusive groups

@rhettinger

BPO	18943
Nosy	@rhettinger, @pitrou, @wm75, @mlouielu
Files	test_argparse.diff: Patch for test_argparse.py (for trunk, but also applies on 2.7 head) patch_1.diff

^{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 2013-09-06.09:00:02.098>
labels = ['3.7', 'type-bug', 'library']
title = 'argparse: default args in mutually exclusive groups'
updated_at = <Date 2020-12-05.07:19:06.915>
user = 'https://github.com/arigo'

bugs.python.org fields:

activity = <Date 2020-12-05.07:19:06.915>
actor = 'rhettinger'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2013-09-06.09:00:02.098>
creator = 'arigo'
dependencies = []
files = ['31626', '31704']
hgrepos = []
issue_num = 18943
keywords = ['patch']
message_count = 24.0
messages = ['197058', '197121', '197128', '197141', '197161', '197225', '197275', '197290', '197295', '197336', '197350', '197414', '197427', '211229', '212693', '292305', '292313', '293931', '299355', '307710', '307758', '307760', '307762', '307852']
nosy_count = 7.0
nosy_names = ['rhettinger', 'pitrou', 'bethard', 'paul.j3', 'wolma', 'louielu', 'talkless']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue18943'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.7']

Linked PRs

In argparse, default arguments have a strange behavior that shows up in mutually exclusive groups: specifying explicitly on the command-line an argument, but giving it its default value, is sometimes equivalent to not specifying the argument at all, and sometimes not.

See the attached test diff: it contains two apparently equivalent pieces of code, but one passes and one fails. The difference is that, in CPython, int("42") is 42 but int("4200") is not 4200 (in the sense of the operator "is").

The line that uses "is" in this way is this line in argparse.py (line 1783 in 2.7 head):

            if argument_values is not action.default:

The patch isn't a good unittest case because it produces an Error, not a Failure. It does, though, raise a valid question about how a Mutually_exclusive_group tests for the use of its arguments.

As you note, argparse does use the is test: argument_values is not action.default. argument_values is the result of passing an argument_string through its 'type' function.

This reworks your test case a bit:

    group = parser.add_mutually_exclusive_group()
    group.add_argument('--foo', default='test')
    group.add_argument('--bar', type=int, default=256)
    group.add_argument('--baz', type=int, default=257)

'--foo test --baz 257' will give the argument --foo: not allowed with argument --baz error message, but '--foo test --baz 256' does not.

So which is right? Should it complain because 2 exclusive arguments are being used? Or should it be excused from complaining because the values match their defaults?

The other issue is whether the values really match the defaults or not. With an is test, the ids must match. The ids for small integers match all the time, while ones >256 differ.

Strings might have the same id or not, depending on how they are created. If I create x='test', and y='--foo test'.split()[1]. x==y is True, but x is y is False. So '--foo test' argument_value does not match the 'foo.default'.

So large integers (>256) behave like strings when used as defaults in this situation. It's the small integers that have unique ids, and hence don't trigger mutually_exclusive_group errors when they should.

This mutually_exclusive_group 'is' test might not be ideal (free from all ambiguities), but I'm not sure it needs to be changed. Maybe there needs to be a warning in the docs about mutually_exclusive_groups and defaults other than None.

A further complication on this. With the arguments I defined in the previous post

    p.parse_args('--foo test --baz 257'.split())

gives the mutually exclusive error message. sys.argv does the same.

    p.parse_args(['--foo', 'test', '--baz', '257'])

does not give an error, because here the 'test' argument string is the same as the default 'test'. So the m_x_g test thinks `--foo' is the default, and does not count as an input.

Usually in testing an argparse setup I use the list and split arguments interchangeably, but this shows they are not equivalent.

Getting consistently one behavior or the other would be much better imho; I think it's wrong-ish to have the behavior depend uncontrollably on implementation details. But I agree that it's slightly messy to declare which of the two possible fixes is the "right" one. I'm slightly in favor of the more permissive solution ("--bar 42" equivalent to no arguments at all if 42 is the default) only because the other solution might break someone's existing code. If I had no such backward-compatibility issue in mind, I'd vote for the other solution (you can't specify "--bar" with any value, because you already specified "--foo").

The patch isn't a good unittest case because it produces an Error, not
a Failure.

Please let's not be pedantic about what a "good unittest" is.

Changing the test from

    if argument_values is not action.default:

to

if argument_values is not action.default and \
    (action.default is None or argument_values != action.default):

makes the behavior more consistent. Strings and large ints behave like small ints, matching the default and not counting as "present"

Simply using argument_values != action.default was not sufficient, since it raised errors in existing test cases (such as ones involving Nones).

A possibly unintended consequence to this seen_non_default_actions testing is that default values do not qualify as 'present' when testing for a required mutually exclusive group.

    p=argparse.ArgumentParser()
    g=p.add_mutually_exclusive_group(required=True)
    g.add_argument('--foo',default='test')
    g.add_argument('--bar',type=int,default=42)
    p.parse_args('--bar 42'.split())

raises an error: one of the arguments --foo --bar is required

In the original code

    p.parse_args('--foo test'.split())

does not raise an error because 'test' does not qualify as default. But with the change I proposed, it does raise the error.

This issue may require adding a failures_when_required category to the test_argparse.py MEMixin class. Currently nothing in test_argparse.py tests for this issue.

Note that this contrasts with the handling of ordinarily required arguments.

    p.add_argument('--baz',type=int,default=42,required=True)

'--baz 42' does not raise an error. It is 'present' regardless of whether its value matches the default or not.

This argues against tightening the seen_non_default_actions test. Because the current testing only catches a few defaults (None and small ints) it is likely that no user has come across the required group issue. There might actually be fewer compatibility issues if we simply drop the default test (or limit it to the case where the default=None).

I should add that defaults with required arguments (or groups?) doesn't make much sense. Still there's nothing in the code that prevents it.

Fwiw I agree with you :-) I'm just relaying a bug report that originates on PyPy (https://bugs.pypy.org/issue1595).

This argument_values comes from _get_values(). Most of the time is derived from the argument_strings. But in a few cases it is set to action.default, specifically when the action is an optional postional with an empty argument_strings.

test_argparse.TestMutuallyExclusiveOptionalAndPositional is such a case. badger is an optional positional in a mutually exclusive group. As such it can be 'present' without really being there (tricky). Positionals are always processed - otherwise it raises an error.

If this is the case, what we need is a more reliable way of knowing whether _get_values() is doing this, one that isn't fooled by this small int caching.

We could rewrite the is not test as:

if not argument_strings and action.nargs in ['*','?'] and  argument_values is action.default:
    pass # _get_values() has set: argument_values=action.default
else:
    seen_non_default_actions.add(action)
    ...

is a little better, but still feels like a kludge. Having _get_values return a flag that says "I am actually returning action.default" would be clearer, but, I think, too big of a change.

At the very least the is not action.default needs to be changed. Else where in argparse is is only used with None or constant like SUPPRESS. So using it with a user defined parameter is definitely not a good idea.

Possible variations on how is behaves across implementations (pypy, ironpython) only complicates the issue. I'm also familiar with a Javascript translation of argparse (that uses its !== in this context).

This patch uses a narrow criteria - if _get_values() sets the value to action.default, then argument counts as 'not present'. I am setting a using_default flag in _get_values, and return it for use by take_action.

In effect, the only change from previous behavior is that small ints (<257) now behave like large ints, strings and other objects.

It removes the nonstandard 'is not action.default' test, and should behave consistently across all platforms (including pypy).

The patch looks good to me. It may break existing code, though, as reported on https://bugs.pypy.org/issue1595. I would say that it should only go to trunk. We can always fix PyPy (at Python 2.7) in a custom manner, in a "bug-to-bug" compatibility mode.

This patch corrects the handling of seen_non_default_action in another case - a positional with '?' and type=int (or other conversion).

if

    parser.add_argument('badger', type=int, nargs='?', default=2) # or '2'

and the original test 'seen_non_default_actions' is:

if argument_values is not action.default

'argument_values' will be an 'int' regardless of the default. But it will pass the 'is' test with the (small) int default but not the string default.

With the patch proposed here, both defaults behave the same - 'badger' will not appear in 'seen_non_default_actions' if it did not occur in the argument_strings (i.e. match an empty string).

I may add case like this to test_argparse.py for this patch.

pythongh-63143: Fix parsing mutually exclusive arguments in argparse

A simpler fix is included as the part of #124306. But since there is already specific issue and that PR do not have specific tests, I created a specific PR #124307.

Instead of relying on small integer cache, it uses bool for testing.

gh-63143: Fix parsing mutually exclusive arguments in argparse (GH-12…

pythongh-63143: Fix parsing mutually exclusive arguments in argparse (p…

[3.12] gh-63143: Fix parsing mutually exclusive arguments in argparse (…

[3.13] gh-63143: Fix parsing mutually exclusive arguments in argparse (…

Use a different version of python

Set maximum Python version to 3.12.6.

Set maximum Python version to 3.12.6 (#314)

Navigation Menu

Repository navigation

argparse: default args in mutually exclusive groups #63143

Linked PRs

19 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

argparse: default args in mutually exclusive groups #63143

Description

Linked PRs

Activity

arigo commented on Sep 6, 2013

paulj3 commented on Sep 6, 2013

paulj3 commented on Sep 7, 2013

arigo commented on Sep 7, 2013

pitrou commented on Sep 7, 2013

paulj3 commented on Sep 8, 2013

paulj3 commented on Sep 8, 2013

paulj3 commented on Sep 8, 2013

arigo commented on Sep 8, 2013

paulj3 commented on Sep 8, 2013

paulj3 commented on Sep 9, 2013

paulj3 commented on Sep 9, 2013

arigo commented on Sep 10, 2013

paulj3 commented on Feb 14, 2014

19 remaining items

serhiy-storchaka commented on Sep 21, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions