gh-138525: raise error when BooleanOptionalAction used with prefix_chars without '-' by mattwang44 · Pull Request #138526 · python/cpython · GitHub
Skip to content

gh-138525: raise error when BooleanOptionalAction used with prefix_chars without '-'#138526

Closed
mattwang44 wants to merge 8 commits into
python:mainfrom
mattwang44:gh-138525
Closed

gh-138525: raise error when BooleanOptionalAction used with prefix_chars without '-'#138526
mattwang44 wants to merge 8 commits into
python:mainfrom
mattwang44:gh-138525

Conversation

@mattwang44

@mattwang44 mattwang44 commented Sep 5, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

(this contribution is made during PyCon TW sprint)

fixes #138525. Here's the output of my local build with this patch.

>>> from argparse import BooleanOptionalAction, ArgumentParser
>>> p = ArgumentParser(prefix_chars='+')
>>> p.add_argument("++foo", action=BooleanOptionalAction)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    p.add_argument("++foo", action=BooleanOptionalAction)
    ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/whw/Desktop/cpython/Lib/argparse.py", line 1554, in add_argument
    action._check_compatibility(self)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "/Users/whw/Desktop/cpython/Lib/argparse.py", line 969, in _check_compatibility
    raise ValueError(f"BooleanOptionalAction requires '-' in prefix_chars.")
ValueError: BooleanOptionalAction requires '-' in prefix_chars.

I believe the description of BooleanOptionalAction on the argparse doc has already implied that it should be used with prefix_chars containing '-', so I didn't make any change to the docs.

@bedevere-app bedevere-app Bot mentioned this pull request Sep 5, 2025
Closed
@corona10

corona10 commented Sep 5, 2025

Copy link
Copy Markdown
Member

@serhiy-storchaka

serhiy-storchaka commented Sep 5, 2025

Copy link
Copy Markdown
Member

Isn't it too late? It raises an error when the option is used, not when it is defined.

@mattwang44

mattwang44 commented Sep 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Isn't it too late? It raises an error when the option is used, not when it is defined.

I find it difficult to achieve this without changing the interface. I've added a generic _check_compatibility() validation hook in the Action base class that gets called after action creation in add_argument(), allowing BooleanOptionalAction to validate itself against the parser's prefix_chars without leaking action-specific knowledge into _ActionsContainer. PTAL.

sobolevn
sobolevn reviewed Sep 8, 2025
View reviewed changes
Comment thread Lib/argparse.py Outdated
Comment thread Misc/NEWS.d/next/Library/2025-09-05-15-42-16.gh-issue-138525.ngZ0qT.rst Outdated
Comment thread Lib/test/test_argparse.py Outdated
@savannahostrowski savannahostrowski mentioned this pull request Nov 22, 2025
Merged
@serhiy-storchaka

serhiy-storchaka commented Nov 22, 2025

Copy link
Copy Markdown
Member

Thank you for your PR, @mattwang44, but after adding support of alternate prefix chars in #138692 this check is no longer needed.

@savannahostrowski

savannahostrowski commented Nov 22, 2025

Copy link
Copy Markdown
Member

@mattwang44 mattwang44 deleted the gh-138525 branch November 23, 2025 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sobolevn sobolevn sobolevn left review comments
@savannahostrowski savannahostrowski Awaiting requested review from savannahostrowski savannahostrowski is a code owner

Assignees

@savannahostrowski savannahostrowski

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

argparse.BooleanOptionalAction silently failed when used with parser whose prefix_chars has no '-'

5 participants

@mattwang44 @corona10 @serhiy-storchaka @savannahostrowski @sobolevn