bpo-45235: Fix argparse overrides namespace with subparser defaults #28420

ALSchwalm · 2021-09-17T15:17:59Z

argparse will now preserve existing values in a provided namespace
even if a subparser has a default for a value in the namespace. For
example:

import argparse
parser = argparse.ArgumentParser()
sub = parser.add_subparsers()
example_subparser = sub.add_parser("example")
example_subparser.add_argument("--flag", default=10)
print(parser.parse_args(["example"], argparse.Namespace(flag=20)))

This snippet now returns 'Namespace(flag=20)' instead of
'Namespace(flag=10)'.

https://bugs.python.org/issue45235

argparse will now preserve existing values in a provided namespace even if a subparser has a default for a value in the namespace. For example: import argparse parser = argparse.ArgumentParser() sub = parser.add_subparsers() example_subparser = sub.add_parser("example") example_subparser.add_argument("--flag", default=10) print(parser.parse_args(["example"], argparse.Namespace(flag=20))) This snippet now returns 'Namespace(flag=20)' instead of 'Namespace(flag=10)'.

the-knights-who-say-ni · 2021-09-17T15:18:02Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@ALSchwalm

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

rhettinger · 2021-09-17T15:58:46Z

This looks good.

Please add a Misc/NEWS entry with blurb.

ALSchwalm · 2021-09-17T16:57:03Z

Done, I have also signed the CLA, it's just not showing up yet.

miss-islington · 2021-09-18T04:20:34Z

Thanks @ALSchwalm for the PR, and @rhettinger for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

bedevere-bot · 2021-09-18T04:20:43Z

GH-28442 is a backport of this pull request to the 3.10 branch.

bedevere-bot · 2021-09-18T04:20:47Z

GH-28443 is a backport of this pull request to the 3.9 branch.

miss-islington · 2021-09-18T04:24:41Z

Thanks @ALSchwalm for the PR, and @rhettinger for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington · 2021-09-18T04:24:41Z

Thanks @ALSchwalm for the PR, and @rhettinger for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

…ythonGH-28420) (cherry picked from commit a6e8db5) Co-authored-by: Adam Schwalm <[email protected]>

…H-28420) (GH-28442)

…H-28420) (GH-28443)

…ythonGH-28420)

tacaswell · 2021-09-27T21:43:00Z

Lib/argparse.py

@@ -1843,11 +1844,6 @@ def parse_known_args(self, args=None, namespace=None):
                    if action.default is not SUPPRESS:
                        setattr(namespace, action.dest, action.default)

-        # add any parser defaults that aren't present


Does this block need to be moved below? This change is breaking traitlets (and IPython) who rely on there being a default value set before the Action logic fires in self._parse_known_args.

I had to move setting the defaults to after the actions because otherwise subparser defaults will be hidden by the parent's defaults (e.g., test_set_defaults_on_parent_and_subparser will fail). It's not obvious to me how that could be avoided. Perhaps the defaults could be exposed in some other way for libraries that depend on that?

I see.

Maybe all that is needed is documentation that relying on the defaults to be there before the Actions run is broken. I am pretty sure that the old behavior was not intentional and that you used to have access to mutable default values in the Actions was a weird implementation detail.

This change on when default value is set also breaks azure-cli: Azure/azure-cli#20269 (comment)

In python/cpython#28420 the order of setting the default values on the Namespace object and processing the Actions. This means that in the `__call__` method of `_FlagAction` the `_flags` attribute has not yet been put on the namespace. This change makes `_FlagAction.__call__` forgiving if the `_flags` attribute does not exist (by creating it!).

In python/cpython#28420 the order of setting the default values on the Namespace object and processing the Actions was reversed. This means that in the `__call__` method of `_FlagAction` the `_flags` attribute has not yet been put on the namespace. This change makes `_FlagAction.__call__` forgiving if the `_flags` attribute does not exist (by creating it!).

jiasli · 2021-11-10T07:12:14Z

Unfortunately, this causes a breaking change and backward compatibility issue as detailed in

…faults (pythonGH-28420) (pythonGH-28443)" This reverts commit a18d522.

* Revert "bpo-45235: Fix argparse overrides namespace with subparser defaults (GH-28420) (GH-28443)" This reverts commit a18d522.

…GH-29525) * Revert "bpo-45235: Fix argparse overrides namespace with subparser defaults (pythonGH-28420) (pythonGH-28443)" This reverts commit a18d522. (cherry picked from commit 807f839) Co-authored-by: Raymond Hettinger <[email protected]>

…GH-29525) * Revert "bpo-45235: Fix argparse overrides namespace with subparser defaults (pythonGH-28420) (pythonGH-28443)" This reverts commit a18d522.

the-knights-who-say-ni added the CLA not signed label Sep 17, 2021

bedevere-bot added the awaiting review label Sep 17, 2021

rhettinger added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes type-bug An unexpected behavior, bug, or error labels Sep 17, 2021

📜🤖 Added by blurb_it.

4182953

the-knights-who-say-ni added CLA signed and removed CLA not signed labels Sep 17, 2021

rhettinger approved these changes Sep 18, 2021

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Sep 18, 2021

rhettinger merged commit a6e8db5 into python:main Sep 18, 2021

bedevere-bot removed the awaiting merge label Sep 18, 2021

bedevere-bot removed the needs backport to 3.10 only security fixes label Sep 18, 2021

bedevere-bot removed the needs backport to 3.9 only security fixes label Sep 18, 2021

rhettinger added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Sep 18, 2021

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 18, 2021

bpo-45235: Fix argparse overrides namespace with subparser defaults (p…

d92cb1f

…ythonGH-28420) (cherry picked from commit a6e8db5) Co-authored-by: Adam Schwalm <[email protected]>

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 18, 2021

bpo-45235: Fix argparse overrides namespace with subparser defaults (p…

de96ddd

…ythonGH-28420) (cherry picked from commit a6e8db5) Co-authored-by: Adam Schwalm <[email protected]>

rhettinger pushed a commit that referenced this pull request Sep 18, 2021

bpo-45235: Fix argparse overrides namespace with subparser defaults (G…

6e4101a

…H-28420) (GH-28442)

rhettinger pushed a commit that referenced this pull request Sep 18, 2021

bpo-45235: Fix argparse overrides namespace with subparser defaults (G…

a18d522

…H-28420) (GH-28443)

niyas-sait pushed a commit to niyas-sait/cpython that referenced this pull request Sep 21, 2021

bpo-45235: Fix argparse overrides namespace with subparser defaults (p…

19ba77f

…ythonGH-28420)

tacaswell reviewed Sep 27, 2021

View reviewed changes

tacaswell mentioned this pull request Sep 27, 2021

FIX: account for changes in py310 ipython/traitlets#678

Merged

jiasli mentioned this pull request Nov 10, 2021

Global Arguments stop working in Python 3.9.8 Azure/azure-cli#20269

Closed

jiasli mentioned this pull request Nov 11, 2021

{CI} Use Python 3.10 as the default and disable Python 3.9 tests Azure/azure-cli#20287

Merged

rhettinger added a commit to rhettinger/cpython that referenced this pull request Nov 11, 2021

Revert "bpo-45235: Fix argparse overrides namespace with subparser de…

c2278aa

…faults (pythonGH-28420) (pythonGH-28443)" This reverts commit a18d522.

rhettinger added a commit that referenced this pull request Nov 12, 2021

bpo-45235: Revert an argparse bugfix that caused a regression (GH-29525)

807f839

* Revert "bpo-45235: Fix argparse overrides namespace with subparser defaults (GH-28420) (GH-28443)" This reverts commit a18d522.

This was referenced Nov 12, 2021

[3.10] bpo-45235: Revert an argparse bugfix that caused a regression (GH-29525) #29530

Merged

[3.9] bpo-45235: Revert an argparse bugfix that caused a regression (GH-29525) #29531

Merged

tuzi3040 mentioned this pull request Nov 13, 2021

azure-cli: migrate to [email protected] Homebrew/homebrew-core#86758

Closed

jiasli mentioned this pull request Dec 13, 2021

bpo-45235: Fix argparse namespace overridden by subparsers default #29574

Open

ALSchwalm mannequin mentioned this pull request Apr 10, 2022

argparse does not preserve namespace with subparser defaults #89398

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-45235: Fix argparse overrides namespace with subparser defaults #28420

bpo-45235: Fix argparse overrides namespace with subparser defaults #28420

ALSchwalm commented Sep 17, 2021 •

edited by bedevere-bot

Loading

the-knights-who-say-ni commented Sep 17, 2021

rhettinger commented Sep 17, 2021

ALSchwalm commented Sep 17, 2021

miss-islington commented Sep 18, 2021

bedevere-bot commented Sep 18, 2021

bedevere-bot commented Sep 18, 2021

miss-islington commented Sep 18, 2021

miss-islington commented Sep 18, 2021

tacaswell Sep 27, 2021

ALSchwalm Sep 27, 2021

tacaswell Sep 28, 2021

jiasli Nov 15, 2021

jiasli commented Nov 10, 2021

bpo-45235: Fix argparse overrides namespace with subparser defaults #28420

bpo-45235: Fix argparse overrides namespace with subparser defaults #28420

Conversation

ALSchwalm commented Sep 17, 2021 • edited by bedevere-bot Loading

the-knights-who-say-ni commented Sep 17, 2021

CLA Missing

rhettinger commented Sep 17, 2021

ALSchwalm commented Sep 17, 2021

miss-islington commented Sep 18, 2021

bedevere-bot commented Sep 18, 2021

bedevere-bot commented Sep 18, 2021

miss-islington commented Sep 18, 2021

miss-islington commented Sep 18, 2021

tacaswell Sep 27, 2021

Choose a reason for hiding this comment

ALSchwalm Sep 27, 2021

Choose a reason for hiding this comment

tacaswell Sep 28, 2021

Choose a reason for hiding this comment

jiasli Nov 15, 2021

Choose a reason for hiding this comment

jiasli commented Nov 10, 2021

ALSchwalm commented Sep 17, 2021 •

edited by bedevere-bot

Loading