Skip to content
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.

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

BooleanOptionalAction does not correctly resolve arguments that are defined as starting with "--no-" #117941

Closed
noah-gil opened this issue Apr 16, 2024 · 6 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@noah-gil
Copy link

noah-gil commented Apr 16, 2024

Bug report

Bug description:

When using argparse to define a BooleanOptionAction that starts with "--no-", it is not possible to set the flag as True. This is counterintuitive, as one would expect that passing the argument should set the flag to True, regardless of the name of the argument.

The bug appears to originate from this line of code:

setattr(namespace, self.dest, not option_string.startswith('--no-'))

Because BooleanOptionAction always generates another argument that starts with "--no-", the code here attempts to check for the case where the user passed in that generated argument. It doesn't consider the possibility that the original argument could also start with "--no-".

This code here demonstrates the bug:

import argparse
parser = argparse.ArgumentParser()
parser.add_argument('--no-foo', action=argparse.BooleanOptionalAction)
assert(parser.parse_args(['--no-no-foo']).no_foo == False) # Passes
assert(parser.parse_args(['--no-foo']).no_foo == True) # Fails

While I tested this on 3.10, the argparse snippet above is from the main branch, which suggests the bug is in the latest version.

CPython versions tested on:

3.10

Operating systems tested on:

Linux

Linked PRs

@encukou
Copy link
Member

encukou commented Apr 22, 2024

I think the proper thing to do here is to deprecate adding boolean options that start with --no-. They certainly do not work as intended.

@serhiy-storchaka
Copy link
Member

I concur with @encukou. We can even reject them without a deprecation period if they are absolutely broken.

@encukou
Copy link
Member

encukou commented Apr 24, 2024

I don't think they're absolutely broken. Someone might be relying on the current behaviour (accidentally, most likely).

@serhiy-storchaka
Copy link
Member

@encukou, are you interesting in creating a patch for deprecating a BooleanOptionAction that starts with "--no-"?

@encukou
Copy link
Member

encukou commented Sep 23, 2024

It's on my TODO list, but it doesn't have much priority. Feel free to do it.

@serhiy-storchaka
Copy link
Member

I do not see much value in deprecation, so I can only create a PR for disallowing such names.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Oct 23, 2024
…e.BooleanOptionalAction

They never worked correctly.
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Oct 23, 2024
…e.BooleanOptionalAction

They never worked correctly.
serhiy-storchaka added a commit that referenced this issue Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
Status: Doc issues
Development

No branches or pull requests

3 participants