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

Minimize boilerplate when building Commands/Options #4292

Draft
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

nwatson22
Copy link
Contributor

No description provided.

@nwatson22 nwatson22 self-assigned this Apr 27, 2024
@rv-jenkins rv-jenkins changed the base branch from master to develop April 27, 2024 00:09
@nwatson22 nwatson22 linked an issue Apr 27, 2024 that may be closed by this pull request
@nwatson22
Copy link
Contributor Author

nwatson22 commented Apr 27, 2024

Current issue I'm having:
The Option class is ideally where we'll input every piece of data about an individual option, e.g. --debug.
It would be nice to impose some type restrictions on each Option, so it's declared as a specific type and its default parameter can only be of that type. However, when I use generics to tell the Option what type it is, there doesn't seem to be any good way in python to read that type at runtime, and when I try to pass the type in as a normal parameter of type type, then it doesn't accept union types, e.g. Int | None.

Edit: I solved this for now by using a separate class for each type of option we might want to have, some of which have special features, e.g. StringOption, BoolOption, EnumOption, ReadFileOption

@nwatson22 nwatson22 requested a review from tothtamas28 May 1, 2024 21:29
optional: bool = False,
help_str: str | None = None,
metavar: str | None = None,
default: Any = 'NoDefault',
Copy link
Contributor

@tothtamas28 tothtamas28 May 2, 2024

Choose a reason for hiding this comment

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

Consider introducing a dedicated value (a null object) for this, e.g.

NO_DEFAULT: Final = object()

)


class BoolOption(Option):
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be best to avoid the need for this class hierarchy. It is possible I'm missing something, but I think you can just add to Option all the attributes necessary to call add_argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was looking to hide the argparse implementation details, like FileType, but I guess this is not really necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Model commands and options with composition
2 participants