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.

Sign up for GitHub

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

Jump to bottom

feat: make add_dataclass_options public, separate field extraction into public helper function #59

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

feat: make add_dataclass_options public, separate field extraction into public helper function #59

wants to merge 5 commits into from

Conversation

johanneskoester
Copy link

@johanneskoester johanneskoester commented Sep 11, 2023 •
edited

This PR adds two things:

  1. add_dataclass_options becomes a public function, which fixes make _add_dataclass_options public #58
  2. I have separated the logic that infers args and kwargs for add_argument into a separate public helper function. This allows to further customize the added argument if needed. We use this in https://github.com/snakemake/snakemake-interface-executor-plugins to add additional arguments that are needed for e.g. utilizing the configargparse package in combination with argparse_dataclass.

I apologize for the mixed up diff. In fact, field_to_argument_args is just an exact copy of the inner loop that was formerly implemented in add_dataclass_options.
Also note that this PR does not introduce a breaking change, it just adds to the public API the two functions.

mivade
mivade reviewed Sep 11, 2023
View reviewed changes
argparse_dataclass.py Outdated Show resolved Hide resolved
@mivade
Copy link
Owner

mivade commented Sep 18, 2023

Looks like there is some typing issue that's failing on Python 3.8 but I haven't spotted what exactly it is yet.

@martinResearch martinResearch mentioned this pull request Sep 20, 2023
Draft
@martinResearch
Copy link

Could you please add a test like I did in my draft PR? #60
thanks!

@vsoch
Copy link

vsoch commented Feb 21, 2024

Any progress here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mivade mivade mivade left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

make _add_dataclass_options public
4 participants
@johanneskoester @mivade @martinResearch @vsoch