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

Feature request: better support for argumentless CLI boolean flags #361

Closed
sersorrel opened this issue Aug 5, 2024 · 4 comments · Fixed by #365
Closed

Feature request: better support for argumentless CLI boolean flags #361

sersorrel opened this issue Aug 5, 2024 · 4 comments · Fixed by #365
Assignees

Comments

@sersorrel
Copy link

Currently, you can declare a model like this:

class Settings(BaseSettings):
    dry_run: bool = False
    model_config = SettingsConfigDict(
        cli_parse_args=True,
        alias_generator=AliasGenerator(
            validation_alias=lambda s: AliasChoices(s, to_snake(s).replace("_", "-"))
        ),
    )

and end up with an application you can run in dry-run mode by running myapp --dry-run=true. But this is not exactly what is usually expected of CLI applications; usually, boolean flags are argumentless (--dry-run, --force, etc.) and their presence implies the associated value is true. Sometimes they have an inverse like --no-dry-run which implies the opposite.

I don't think pydantic-settings currently supports arguments like this. It would be very useful if it did :)

@hramezani
Copy link
Member

Thanks @sersorrel for this feature request.

@kschwab Do you think we can support this in pydantic-settings?

@kschwab
Copy link
Contributor

kschwab commented Aug 7, 2024

@sersorrel thanks for the request.

@hramezani, yes, we can support this. My thoughts are to add a cli_implicit_flags config that will convert optional bool fields into flags. We can then add CliExplicitFlag and CliImplicitFlag annotations to provide control at a more granular level. e.g.:

class Cfg(BaseSettings, cli_parse_args=True, cli_implicit_flags=True):
    req_flag: bool
    """--req_flag {True,False}"""

    implicit_flag: bool = False
    """--implicit_flag, --no-implicit_flag"""

    # Flags are implicit by default, so must override explicit flags with annotation
    explicit_flag: CliExplicitFlag[bool] = False
    """--explicit_flag {True,False}"""

class Cfg(BaseSettings, cli_parse_args=True, cli_implicit_flags=False):
    req_flag: bool
    """--req_flag {True,False}"""

    # Flags are explicit by default, so must override implicit flags with annotation
    implicit_flag: CliImplicitFlag[bool] = False
    """--implicit_flag, --no-implicit_flag"""

    explicit_flag: bool = False
    """--explicit_flag {True,False}"""

If the above seems reasonable, I'll move forward. The only question I have is what the default should be for cli_implicit_flags? I lean towards having it disabled by default (the current behavior) to keep it aligned with env vars, but am interested in your thoughts.

@hramezani
Copy link
Member

hramezani commented Aug 7, 2024

Thanks @kschwab for the proposal.

If the above seems reasonable, I'll move forward. The only question I have is what the default should be for cli_implicit_flags? I lean towards having it disabled by default (the current behavior) to keep it aligned with env vars, but am interested in your thoughts.

Yes, LGTM. the default should be cli_implicit_flags=False because this won't introduce a breaking change.

I assigned the issue to you @kschwab. but feel free to unassign yourself if you don't want to continue working on this issue.

@kschwab
Copy link
Contributor

kschwab commented Aug 16, 2024

Final implementation is slightly modified from above proposal. CLI implicit flags will also convert required fields for python >= 3.9:

class Cfg(BaseSettings, cli_parse_args=True, cli_implicit_flags=True):
    req_implicit_flag: bool
    """--req_implicit_flag, --no-req_implicit_flag (required)"""

    # Flags are implicit by default, so must override explicit flags with annotation
    req_explicit_flag: CliExplicitFlag[bool]
    """--req_explicit_flag bool (required)"""

    opt_implicit_flag: bool = False
    """--opt_implicit_flag, --no-opt_implicit_flag (default: False)"""

    # Flags are implicit by default, so must override explicit flags with annotation
    opt_explicit_flag: CliExplicitFlag[bool] = False
    """--opt_explicit_flag bool (default: False)"""

class Cfg(BaseSettings, cli_parse_args=True, cli_implicit_flags=False):
    # Flags are explicit by default, so must override implicit flags with annotation
    req_implicit_flag: CliImplicitFlag[bool]
    """--req_implicit_flag, --no-req_implicit_flag (required)"""

    req_explicit_flag: bool
    """--req_explicit_flag bool (required)"""

    # Flags are explicit by default, so must override implicit flags with annotation
    opt_implicit_flag: CliImplicitFlag[bool] = False
    """--opt_implicit_flag, --no-opt_implicit_flag (default: False)"""

    opt_explicit_flag: CliExplicitFlag[bool] = False
    """--opt_explicit_flag bool (default: False)"""

For python < 3.9, the --no-flag option will not be supported due to underlying argparse support, and implicit flags can only be applied to optional bool fields. Therefore, python 3.8 will look like original proposal:

class Cfg(BaseSettings, cli_parse_args=True, cli_implicit_flags=True):
    req_flag: bool
    """--req_flag bool (required)"""

    implicit_flag: bool = False
    """--implicit_flag (default: False)"""

    # Flags are implicit by default, so must override explicit flags with annotation
    explicit_flag: CliExplicitFlag[bool] = False
    """--explicit_flag bool (default: False)"""

class Cfg(BaseSettings, cli_parse_args=True, cli_implicit_flags=False):
    req_flag: bool
    """--req_flag bool (required)"""

    # Flags are explicit by default, so must override implicit flags with annotation
    implicit_flag: CliImplicitFlag[bool] = False
    """--implicit_flag (default: False)"""

    explicit_flag: bool = False
    """--explicit_flag bool (default: False)"""

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

Successfully merging a pull request may close this issue.

4 participants