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

Subparsers overwrite kwargs of original parser #310

Open
bnorick opened this issue Apr 16, 2024 · 1 comment
Open

Subparsers overwrite kwargs of original parser #310

bnorick opened this issue Apr 16, 2024 · 1 comment
Labels
good first issue Good for newcomers

Comments

@bnorick
Copy link

bnorick commented Apr 16, 2024

Describe the bug
When argparse creates a new simple_parsing.ArgumentParser in the call to parser.add_subparsers it doesn't pass the original kwargs (obviously), so the global class attributes on FieldWrapper like add_dash_variants get overwritten with the defaults.

Your refactor may address this, but in case you weren't aware I thought I'd raise the issue. Those globals seem not ideal.

Here's a workaround:

import inspect
import pathlib

FIRST_KWARGS = None


class ArgumentParser(simple_parsing.ArgumentParser):
    def __init__(self, *args, **kwargs):
        global FIRST_KWARGS
        if FIRST_KWARGS is None:
            FIRST_KWARGS = kwargs

        # only enforce FIRST_KWARGS when called by argparse
        caller = inspect.stack()[1]
        caller_path = pathlib.Path(caller.filename)
        if caller_path.match("*lib/**/argparse.py"):
            super().__init__(*args, **FIRST_KWARGS)
        else:
            super().__init__(*args, **kwargs)

To Reproduce
Use subparsers with non-default kwargs on the top level parser, e.g., add_dash_variants.

Expected behavior
Subparsers obey the kwargs used by the parent.

Actual behavior
Subparsers use default kwargs.

Desktop (please complete the following information):

  • Version latest pip installable
  • Python version: 3,11.8
@lebrice
Copy link
Owner

lebrice commented Apr 25, 2024

Hey there @bnorick, thanks for posting!

Those globals seem not ideal.

Totally agree. I definitely need to remove those global vars. One idea could be to preserve the kwargs on the ArgumentParser so that parser.add_subparsers transfers them. I'll have to try that out.

@lebrice lebrice added the good first issue Good for newcomers label Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants