This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: argparse does not preserve namespace with subparser defaults
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.11, Python 3.10, Python 3.9, Python 3.8, Python 3.7, Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: ALSchwalm, Frost Ming, RhinosF1, dgw, lukasz.langa, lwalejko, miss-islington, pablogsal, paul.j3, rhettinger
Priority: normal Keywords: patch

Created on 2021-09-17 14:58 by ALSchwalm, last changed 2022-04-11 14:59 by admin.

Pull Requests
URL Status Linked Edit
PR 28420 merged ALSchwalm, 2021-09-17 15:18
PR 28442 merged miss-islington, 2021-09-18 04:20
PR 28443 merged miss-islington, 2021-09-18 04:20
PR 29525 merged rhettinger, 2021-11-11 18:53
PR 29530 merged miss-islington, 2021-11-12 03:53
PR 29531 merged miss-islington, 2021-11-12 03:53
PR 29574 open Frost Ming, 2021-11-16 08:29
PR 30219 open Frost Ming, 2021-12-21 08:33
Messages (23)
msg402060 - (view) Author: Adam Schwalm (ALSchwalm) * Date: 2021-09-17 14:58
The following snippet demonstrates the problem. If a subparser flag has a default set, argparse will override the existing value in the provided 'namespace' if the flag does not appear (e.g., if the default is used):

    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 should return 'Namespace(flag=20)' because 'flag' already exists in the namespace, but instead it returns 'Namespace(flag=10)'. This intended behavior is described and demonstrated in the second example here: https://docs.python.org/3/library/argparse.html#default

Lib's behavior is correct for the non-subparser cause.
msg402120 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-09-18 04:20
New changeset a6e8db5e8e6780411db749d404715dbe021647c7 by Adam Schwalm in branch 'main':
bpo-45235: Fix argparse overrides namespace with subparser defaults (GH-28420)
https://github.com/python/cpython/commit/a6e8db5e8e6780411db749d404715dbe021647c7
msg402121 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-09-18 06:47
New changeset 6e4101add568910409294554c8e863226a66bb64 by Miss Islington (bot) in branch '3.10':
bpo-45235: Fix argparse overrides namespace with subparser defaults (GH-28420) (GH-28442)
https://github.com/python/cpython/commit/6e4101add568910409294554c8e863226a66bb64
msg402122 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-09-18 06:47
New changeset a18d52269ab6071a605d6c72f6af585a4c533ca4 by Miss Islington (bot) in branch '3.9':
bpo-45235: Fix argparse overrides namespace with subparser defaults (GH-28420) (GH-28443)
https://github.com/python/cpython/commit/a18d52269ab6071a605d6c72f6af585a4c533ca4
msg404990 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2021-10-25 18:16
I haven't studied or tested this change, but it looks like a partial retraction of

https://bugs.python.org/issue9351 
argparse set_defaults on subcommands should override top level set_defaults

Originally the main namespace was passed to the subparser.  Steven Bethard changed it so that the subparser got a fresh namespace, and all values were copied to the main namespace.

I and others raised the question of how it affected user provided values 

https://bugs.python.org/issue27859
argparse - subparsers does not retain namespace

Looks like this patch tries to solve some problems by moving the self._defaults step to the end of parser_know_args.  I view that change with some trepidation.  Handling defaults is tricky enough, with setting them at the start, but then only passing them through 'type' at the end if they still match the original strings.

Mostly I've been telling StackOverflow questioners that it best not to use the same argument 'dest' in both the main and subparsers.  Flags can be the same, but the 'dest' should be different to avoid conflicts over which default has priority.

Again, I haven't tested this change, but I have a gut feeling it could have backward compatibility issues.
msg404991 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2021-10-25 18:47
I just downloaded this `argparse.py`.

This change makes it impossible to use a subparser argument if it is defined in the user provided namespace, or by the main parser.  It blocks not only subparser default, but also user input.

It has reverted the 9351 patch which dates to 2014.
msg405007 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2021-10-25 23:27
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","--flag=15"], argparse.Namespace(flag=20)))

still returns flag=20

User input should override values set by the provided Namespace.
msg405129 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2021-10-27 21:41
I should study previous posts in more detail, but here are some thoughts on correctly handling user namespace.

At the start of `parse_known_args`, there's a

        if namespace is None:
            namespace = Namespace()

We need to hang on to a copy of this namespace, e.g. call it

        import copy
        orig_namespace = copy.copy(namespace)

In _SubParsersAction.__call__, pass this copy to the subparser (instead of None):

    subnamespace, arg_strings = parser.parse_known_args(arg_strings, orig_namespace)
        for key, value in vars(subnamespace).items():
            setattr(namespace, key, value)

Prior to 9351, the main namespace was passed to the subparser

    namespace, arg_strings = parser.parse_known_args(arg_strings, namespace)

The trick is to get orig_namespace from the main parse_known_args to SubParsersAction.__call__ method.

in a 9351 post I explore the idea of allowing the user to specify a 'sub_namespace' for the subparser.

https://bugs.python.org/msg230056

In any case, this is a complicated issue that needs careful thought and more extensive testing.  I didn't entirely like the original 9351 change, but since that's been part of argparse for many years, we need to very careful about clobbering it.
msg406052 - (view) Author: dgw (dgw) Date: 2021-11-09 20:50
Can confirm that this patch DOES cause backward compatibility issues, as paul.j3's gut feeling said it could. One of my projects, testing against 3.6-3.9, now fails its test suite on Python 3.9.8, which includes this change. Arguments passed to a subparser are indeed ignored in lieu of default values. We are tracking the problem in our own issue, https://github.com/sopel-irc/sopel/issues/2210

I, for one, am not amused that 7-year-old behavior was "clobbered" (as previously described in this thread) in a patch release.
msg406079 - (view) Author: Łukasz Wałejko (lwalejko) Date: 2021-11-10 09:50
I can also confirm that there is an regression in Python 3.9.8 regarding argparse

For example, using watchdog 2.1.6 package

Python 3.9.7 (correct behaviour)
In [1]: from watchdog import watchmedo
In [2]: watchmedo.cli.parse_args(["auto-restart", "echo", "123"])
Out[2]: Namespace(command='echo', command_args=['123'], directories=None, patterns='*', ignore_patterns='', ignore_directories=False, recursive=False, timeout=1.0, signal='SIGINT', debug_force_polling=False, kill_after=10.0, func=<function auto_restart at 0x7f296d1d9dc0>)

Python 3.9.8 (incorrect behaviour)
In [1]: from watchdog import watchmedo
In [2]: watchmedo.cli.parse_args(["auto-restart", "echo", "123"])
Out[2]: Namespace(command='auto-restart', command_args=['123'], directories=None, patterns='*', ignore_patterns='', ignore_directories=False, recursive=False, timeout=1.0, signal='SIGINT', debug_force_polling=False, kill_after=10.0, func=<function auto_restart at 0x7fc39480cee0>)
msg406121 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-11-10 18:22
This has caused a regression, also reported here:
https://github.com/Azure/azure-cli/issues/20269 Global Arguments stop working in Python 3.9.8

Can we please get some attention from @rhettinger and @ambv?
msg406123 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-11-10 18:50
Unless anyone objects, I'll revert this across all affected branches.
msg406128 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-11-10 19:32
Go for it, Raymond.
msg406171 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-11-11 19:14
How long until the next bugfix releases for 3.9 and 3.10?  To avoid further snowballing, it would be great to have this reversion pushed out soonish.
msg406172 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2021-11-11 19:17
The next bugfixe release of 3.10 is the 6th of December
msg406175 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-11-11 19:59
Only 3.9 needs an expedited rerelease.
msg406186 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-11-12 03:53
New changeset 807f839bbfd5805fb76eb3436c9252a0441296eb by Raymond Hettinger in branch 'main':
bpo-45235:  Revert an argparse bugfix that caused a regression (GH-29525)
https://github.com/python/cpython/commit/807f839bbfd5805fb76eb3436c9252a0441296eb
msg406231 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-11-12 18:44
New changeset 587ff7f50bcbfd8346c6d5db459a1628a350c04d by Miss Islington (bot) in branch '3.9':
bpo-45235:  Revert an argparse bugfix that caused a regression (GH-29525) (GH-29531)
https://github.com/python/cpython/commit/587ff7f50bcbfd8346c6d5db459a1628a350c04d
msg406232 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-11-12 18:44
New changeset e4c5a5eabadd1dcd0b522ffbd70157cd95506ad1 by Miss Islington (bot) in branch '3.10':
bpo-45235:  Revert an argparse bugfix that caused a regression (GH-29525) (GH-29530)
https://github.com/python/cpython/commit/e4c5a5eabadd1dcd0b522ffbd70157cd95506ad1
msg406233 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-11-12 18:46
I've restored the prior state of affairs.

Leaving this issue open because it still isn't clear what should be guaranteed or whether further improvements need to be made.
msg406571 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-11-19 06:01
Paul, should this be closed or do you think there is still a namespace issue to be resolved?
msg406814 - (view) Author: Frost Ming (Frost Ming) * Date: 2021-11-23 03:10
Hi, I noticed this bug because of the regression of Python 3.9.8. And I proposed a better approach in PR 29574.

Maybe the folks here can have a look. Thanks

- Frost
msg408991 - (view) Author: Frost Ming (Frost Ming) * Date: 2021-12-21 08:47
Per the review comments of @jiasli, I worked out a second PR to fix this issue.

This fix has less side-effect and better backward compatibility. I will leave the two PRs open. Any feedback is welcome.

- Frost
History
Date User Action Args
2022-04-11 14:59:50adminsetgithub: 89398
2021-12-22 19:20:37gvanrossumsetnosy: - gvanrossum
2021-12-21 08:47:50Frost Mingsetmessages: + msg408991
2021-12-21 08:33:11Frost Mingsetpull_requests: + pull_request28441
2021-11-23 03:10:25Frost Mingsetmessages: + msg406814
2021-11-19 06:01:52rhettingersetassignee: rhettinger ->
messages: + msg406571
2021-11-16 08:29:55Frost Mingsetnosy: + Frost Ming
pull_requests: + pull_request27818
2021-11-12 18:46:36rhettingersetpriority: high -> normal

messages: + msg406233
2021-11-12 18:44:59rhettingersetmessages: + msg406232
2021-11-12 18:44:33rhettingersetmessages: + msg406231
2021-11-12 03:53:50miss-islingtonsetpull_requests: + pull_request27781
2021-11-12 03:53:45miss-islingtonsetpull_requests: + pull_request27780
2021-11-12 03:53:33rhettingersetmessages: + msg406186
2021-11-11 19:59:26rhettingersetmessages: + msg406175
2021-11-11 19:17:38pablogsalsetmessages: + msg406172
2021-11-11 19:14:26rhettingersetpriority: normal -> high
nosy: + pablogsal
messages: + msg406171

2021-11-11 18:53:53rhettingersetstage: needs patch -> patch review
pull_requests: + pull_request27775
2021-11-10 19:32:49lukasz.langasetnosy: + lukasz.langa
messages: + msg406128

resolution: fixed ->
stage: test needed -> needs patch
2021-11-10 18:50:09rhettingersetmessages: + msg406123
2021-11-10 18:22:48gvanrossumsetnosy: + gvanrossum
messages: + msg406121
2021-11-10 09:50:27lwalejkosetnosy: + lwalejko
messages: + msg406079
2021-11-09 21:19:06RhinosF1setnosy: + RhinosF1
2021-11-09 20:50:17dgwsetnosy: + dgw
messages: + msg406052
2021-10-27 21:41:36paul.j3setstatus: pending -> open

messages: + msg405129
2021-10-26 16:08:46paul.j3setstatus: closed -> pending
stage: resolved -> test needed
2021-10-25 23:27:59paul.j3setmessages: + msg405007
2021-10-25 18:47:04paul.j3setmessages: + msg404991
2021-10-25 18:16:15paul.j3setmessages: + msg404990
2021-10-25 16:46:20paul.j3setnosy: + paul.j3
2021-09-18 06:48:49rhettingersetstatus: open -> closed
assignee: rhettinger
resolution: fixed
stage: patch review -> resolved
2021-09-18 06:47:45rhettingersetmessages: + msg402122
2021-09-18 06:47:20rhettingersetmessages: + msg402121
2021-09-18 04:20:44miss-islingtonsetpull_requests: + pull_request26846
2021-09-18 04:20:40miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request26845
2021-09-18 04:20:38rhettingersetnosy: + rhettinger
messages: + msg402120
2021-09-17 15:18:00ALSchwalmsetkeywords: + patch
stage: patch review
pull_requests: + pull_request26832
2021-09-17 14:58:54ALSchwalmcreate