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

gh-89398: Fix argparse namespace overridden by subparsers default #30219

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

frostming
Copy link

@frostming frostming commented Dec 21, 2021

This PR is to fix gh-89398 with another approach than #29574.

The default values are stored in a dunder attribute in Namespace to ensure they don't override the given values.

def _get_kwargs(self):
kwargs = self.__defaults__ | self.__dict__
kwargs.pop('__defaults__', None)
return list(kwargs.items())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you convert it into a list if the caller (__eq__) turns it into a dict again?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The __repr__ implementation of the parent class _AttributeHolder also calls this. But this change can be considered.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, I should have checked that. Better to just leave it as is then.

Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test module shows no issues in 1674 tests.

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Apr 11, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

@frostming frostming force-pushed the fix/45235-argparse-overrided-by-defaults-2 branch from 2281bfe to 4c5cc3c Compare April 12, 2022 13:00
@frostming frostming changed the title bpo-45235: Fix argparse namespace overridden by subparsers default gh-89398: Fix argparse namespace overridden by subparsers default Apr 12, 2022
@frostming frostming force-pushed the fix/45235-argparse-overrided-by-defaults-2 branch from 4c5cc3c to 95cb850 Compare December 1, 2022 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

argparse does not preserve namespace with subparser defaults
6 participants