-
-
Notifications
You must be signed in to change notification settings - Fork 30.3k
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
gh-92248: Deprecate type
, choices
, metavar
parameters of argparse.BooleanOptionalAction
#103678
Conversation
…`argparse.BooleanOptionalAction`
Misc/NEWS.d/next/Library/2023-04-22-12-30-10.gh-issue-92248.NcVTKR.rst
Outdated
Show resolved
Hide resolved
…VTKR.rst Co-authored-by: Kirill <[email protected]>
This would gradually nudge the code toward the way it should have been designed from the outset. Eventually, it might save someone a few minutes of time. That said, this is a pretty minor issue (ignoring unused arguments). Changing the behavior will be inconvenient for people who happened to have fed in an unused argument in code that is currently functioning correctly — we've breaking code that was sloppy but correct — I'm not sure they will appreciate being forced to make a minor edit that doesn't change behavior or fix any problem that they have. |
@rhettinger thank you for your input. However, I don't quite agree with some of your points.
Two things here:
>>> from argparse import *
>>> parser = ArgumentParser()
>>> parser.add_argument('--foo', action=BooleanOptionalAction, type=int) # we expect `int`
>>> parser.parse_args(['--foo']) # but get `bool`
Namespace(foo=True) Here's the code does not "function correctly" in a sense that The second example: >>> from argparse import *
>>> parser = ArgumentParser()
>>> parser.add_argument('--foo', action=BooleanOptionalAction, choices=['on', 'off'])
>>> parser.parse_args(['--foo'])
Namespace(foo=True) And again: user specifies two values that must be respected. But, they are ignored. We also do not document this behaviour: So, any user might expect it to work similarly to other action types. Which is not the case right now. All in all, I see this as an improvement:
|
type
, chocies
, metavar
parameters of argparse.BooleanOptionalAction
type
, choices
, metavar
parameters of argparse.BooleanOptionalAction
Co-authored-by: Hugo van Kemenade <[email protected]>
Let's merge this before the beta if there's no remaining objections. |
* main: (30 commits) pythongh-103987: fix several crashes in mmap module (python#103990) docs: fix wrong indentation causing rendering error in dis page (python#104661) pythongh-94906: Support multiple steps in math.nextafter (python#103881) pythongh-104472: Skip `test_subprocess.ProcessTestCase.test_empty_env` if ASAN is enabled (python#104667) pythongh-103839: Allow building Tkinter against Tcl 8.7 without external libtommath (pythonGH-103842) pythongh-85984: New additions and improvements to the tty library. (python#101832) pythongh-104659: Consolidate python examples in enum documentation (python#104665) pythongh-92248: Deprecate `type`, `choices`, `metavar` parameters of `argparse.BooleanOptionalAction` (python#103678) pythongh-104645: fix error handling in marshal tests (python#104646) pythongh-104600: Make type.__type_params__ writable (python#104634) pythongh-104602: Add additional test for listcomp with lambda (python#104639) pythongh-104640: Disallow walrus in comprehension within type scopes (python#104641) pythongh-103921: Rename "type" header in argparse docs (python#104654) Improve readability of `typing._ProtocolMeta.__instancecheck__` (python#104649) pythongh-96522: Fix deadlock in pty.spawn (python#96639) pythonGH-102818: Do not call `PyTraceBack_Here` in sys.settrace trampoline. (pythonGH-104579) pythonGH-103545: Add macOS specific constants for ``os.setpriority`` to ``os`` (python#104606) pythongh-104623: Update macOS installer to SQLite 3.42.0 (pythonGH-104624) pythongh-104619: never leak comprehension locals to outer locals() (python#104637) pythongh-104602: ensure all cellvars are known up front (python#104603) ...
argparse.BooleanOptionalAction
was added in 3.9#85039 removed just a single wrong parameter.
But, it was on time when 3.9 was recently released, and no deprecation warning was issued.
The parameter was just removed.
But, since we already have 3.9, 3.10, and 3.11, (and all alphas of 3.12), I am sure that we need a deprecation period now.
People might use this (the use-case is probably wrong), but I don't like breaking things for no clear benefit.
I'm feeling pretty safe about this deprecation (I even considered just removing these parameters straight away), because code search shows that no one is using these arguments: https://cs.github.com/?q=action%3DBooleanOptionalAction
Plus, they don't make any sense and are mostly not changing any behaviour of
BooleanOptionalAction
.CC @barneygale since you've asked for this 😆
CC @rhettinger and @hauntsaninja since you've reviewed #85039