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: default_format parameter on Arg #184

Closed
pawamoy opened this issue Nov 23, 2024 · 6 comments · Fixed by #188
Closed

feature: default_format parameter on Arg #184

pawamoy opened this issue Nov 23, 2024 · 6 comments · Fixed by #188

Comments

@pawamoy
Copy link
Contributor

pawamoy commented Nov 23, 2024

I find myself wanting to customize how each default value is displayed in the CLI.

Examples

The following option should display the path to the default config file as default value, not the config object representation:

    config: An[
        Config,
        cappa.Arg(
            short="-c",
            long=True,
            parse=_load_config,
            group=_GROUP_GLOBAL,
            propagate=True,
        ),
        Doc("Path to the configuration file."),
    ] = field(default_factory=Config.from_default_location)

The following option should display "standard error" (without the quotes) as default value:

    log_path: An[
        str | None,
        cappa.Arg(short="-P", long=True, group=_GROUP_GLOBAL, propagate=True),
        Doc("Write log messages to this file path."),
    ] = None

The following option should display `INFO` as default value (with the backticks), not just INFO (without backticks):

    log_level: An[
        Literal["TRACE", "DEBUG", "INFO", "SUCCESS", "WARNING", "ERROR", "CRITICAL"],
        cappa.Arg(short="-L", long=True, parse=str.upper, group=_GROUP_GLOBAL, propagate=True),
        Doc("Log level to use when logging messages."),
    ] = "INFO"

It's not possible to configure all these at once with for example:

    help_formatter = cappa.HelpFormatter(default_format="Default: `{default}`.")

...since as we saw some defaults should be wrapped in backticks, and other shouldn't. Other default values might provide their own __str__ method, which again outputs backticks or not (see #180 (comment)).

Suggestion

Accept a default_format parameter on Arg? This way we could do this:

    config: An[
        Config,
        # Hardcoded f-string.
        cappa.Arg(..., default_format=f"`{defaults.DEFAULT_CONF_PATH}`"),
        Doc("Path to the configuration file."),
    ] = field(default_factory=Config.from_default_location)

    log_path: An[
        str | None,
        # Hardcoded string.
        cappa.Arg(..., default_format="standard error"),
        Doc("Write log messages to this file path."),
    ] = None

    log_level: An[
        Literal["TRACE", "DEBUG", "INFO", "SUCCESS", "WARNING", "ERROR", "CRITICAL"],

        # Note that it's not an f-string, and Cappa will replace the `{value}` placeholder
        # with the actual, stringified default value. The placeholder could be named `default`, too.
        cappa.Arg(..., default_format="`{value}`"),

        Doc("Log level to use when logging messages."),
    ] = "INFO"

...

help_formatter = cappa.HelpFormatter(default_format="Default: {default}.")

Ideally the Arg class would provide a method to render the default value according to default_format, to be used in scripts, for example when rendering Markdown/HTML docs for the CLI. This way we could do this:

default = arg.render_default()
line += f" Default: {default}."  

...instead of reimplementing the logic ourselves.

WDYT ☺️?

@DanCardin
Copy link
Owner

I expected to not want to get into this, reading the title. mostly because could always just supply it as part of the help text itself. But then problem becomes, you need a per-arg way to turn that behavior off anyways, so it might as well just flexibly allow formatting, and default_format="" would equally be a way of doing that.

So i think i'm down for this

@pawamoy
Copy link
Contributor Author

pawamoy commented Nov 25, 2024

Honestly I'd be fine writing the default text in each argument's help text, if the help text has all the necessary placeholders that get formatted with .format(**kwargs) 🙂 What you feel is best!

@DanCardin
Copy link
Owner

I mean it doesnt currently. the help is taken verbatim, and i'm not sure what format kwargs you'd expect to be there or how you'd expect them to be provided given your examples.

You could globally disable the default_format and then manually Doc("Path to the configuration file. Default: {defaults.DEFAULT_CONF_PATH}"), Doc("Write log messages to this file path. Default: standard error"), and Doc("Log level to use when logging messages. Default: INFO"), but obviously if you're relying on the normal default anywhere now, that's annoying.

What's nice about default_format= is that it lets you either wholly override it (by setting a static string), or do one-off customization of the format, given the default (maybe less relevant because you could also then just materialize the string there rather than templating the default...), and (per your last note) provides a way to programmatically arrive at the same string we generate.

@pawamoy
Copy link
Contributor Author

pawamoy commented Nov 25, 2024

I mean it doesnt currently. the help is taken verbatim, and i'm not sure what format kwargs you'd expect to be there or how you'd expect them to be provided given your examples.

I would likely expect a {default} placeholder, which gets replaced by the default value. But format-strings are not f-strings, so for complex default values (such as lazy Default), we wouldn't be able to do something like {', '.join(str(d) for d in default.sequence}. Instead we would have to rely on a good enough __str__ in Default, and subclasses of it with custom __str__ for each subtype (Env, ValueFrom, and further subclasses). Maybe not the most ergonomic API when declaring the CLI help.

You could globally disable the default_format and then manually Doc("Path to the configuration file. Default: {defaults.DEFAULT_CONF_PATH}"), Doc("Write log messages to this file path. Default: standard error"), and Doc("Log level to use when logging messages. Default: INFO"), but obviously if you're relying on the normal default anywhere now, that's annoying.

So, in this case, I would actually avoid writing the default in Doc, because Doc's primary use is for API docs, not CLI help, and API docs will render actual API default values (through their own means), which can differ from CLI default values. It just happens that Cappa supports Doc ( 🙏 ❤️ ), which is rather convenient when both the API docs and the CLI help can use the same message. If the API docs and CLI help start differing, I'll write the CLI help with cappa.Arg(help="..."), even at the price of some duplication. Another argument against using Doc to document defaults, is that you'd be tempted to use dynamic strings (f-strings), or templated strings (formatted-strings), which static analysis tools would have trouble keeping up with. In the end, that means you'd have to actually maintain the string representation of the CLI default value, loosing sync with the actual CLI default value (two places to maintain in sync manually).

What's nice about default_format= is that it lets you either wholly override it (by setting a static string), or do one-off customization of the format, given the default

Yes!

(maybe less relevant because you could also then just materialize the string there rather than templating the default...)

True. That works when the CLI default value is assigned to a name in the current scope (config option above). In other cases, for example when the default value is a literal (log_level option above), it's always possible to declare a variable somewhere and use that instead of the literal (defaults.DEFAULT_LOG_LEVEL = "INFO"). It's still true that it can fall out of sync (using another var as default value, forgetting to update default_format). The templated default_format is convenient to keep things in sync 🙂

@DanCardin
Copy link
Owner

I suddenly realized I had already implemented show_default: bool a bit ago to control the display of default values in service of another feature.

It occurred to me that, similar to default having syntactical shortcuts to a more complex shape, this field would be a natural spot for controlling default value display.

This was added recently enough (0.24.0) for an internal-facing feature that I'd probably be willing to rename this to default_format if you had a strong negative reaction to overloading this field/name.

But as I was implementing it "show_default" imo does linguistically cover both whether to show and/or how to show. and the dual show: bool/format: str fields/input both seem to translate to the same field well in code. show_default=False or show_default="{default}"

(Whereas default_format=False doesn't as well).

@pawamoy
Copy link
Contributor Author

pawamoy commented Nov 27, 2024

Yeah reusing show_default sounds good!

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

Successfully merging a pull request may close this issue.

2 participants