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

Boolean typer.Option missing type and default value in --help #493

Open
7 tasks done
Skyler-Altol opened this issue Nov 9, 2022 · 3 comments
Open
7 tasks done

Boolean typer.Option missing type and default value in --help #493

Skyler-Altol opened this issue Nov 9, 2022 · 3 comments
Labels
investigate question Question or problem

Comments

@Skyler-Altol
Copy link

Skyler-Altol commented Nov 9, 2022

First Check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the Typer documentation, with the integrated search.
  • I already searched in Google "How to X in Typer" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to Typer but to Click.

Commit to Help

  • I commit to help with one of those options 👆

Example Code

import typer

app = typer.Typer(add_completion=False)


@app.command()
def main(some_param: bool = typer.Option(False, "--some-param")):
    pass


@app.command()
def main2(some_param2: str = typer.Option("Testing")):
    pass


if __name__ == "__main__":
    app()

Description

  1. Create a small typer script using the code mentioned above.
  2. Execute script.py main --help
  3. Execute script.py main2 --help
  4. Observe that the string optional parameter has a type of TEXT noted, and a listed default value in the main2 command.
  5. Observe that the boolean optional parameter has no type listed in the main command, and no default value listed, even though it has a default value of False set.

This is actually a contradiction of what's currently in the Typer documentation located here: https://typer.tiangolo.com/tutorial/parameter-types/bool/

The docs shows the following image:
image

It can clearly be seen that at the time of that documentation being created, it was intended for an optional parameter to be specified with a "flag" style, but also retain its default value. Testing has shown that this may be related to the upgrade from Click 7.1.2 -> Click 8.x, but I was unable to pin down whether or not this issue relates to Click itself, or if it's due to Typer's new compatibility code that uses Rich for --help menus rather than Click's help formatter.

Typer 0.4.1 and Click 7.1.2 did not show this issue when tested, but Typer 0.6.1 and Click 8.1.3 did, as does Typer 0.7 and Click 8.1.3.

Ideally the intended result would be to have the type be shown as well. So it would be --some-param BOOL [Default: False] or something like that, to keep things consistent with all other types.

Operating System

Windows

Operating System Details

No response

Typer Version

0.7

Python Version

3.8.10

Additional Context

No response

@Skyler-Altol Skyler-Altol added the question Question or problem label Nov 9, 2022
@Skyler-Altol
Copy link
Author

Further context, command line output from the reproduction steps:

(venv) PS C:\Users\Skyler\PycharmProjects\typer_broken> python .\main.py main --help
Usage: main.py main [OPTIONS]              

Options:                                   
  --some-param
  --help        Show this message and exit.
(venv) PS C:\Users\Skyler\PycharmProjects\typer_broken> python .\main.py main2 --help
Usage: main.py main2 [OPTIONS]

Options:
  --some-param2 TEXT  [default: Testing]
  --help              Show this message and exit.

@Skyler-Altol
Copy link
Author

Skyler-Altol commented Nov 13, 2022

I have identified the code in question that is causing the default value block to be missing. But this is a bit of a multi-faceted issue. In Click 8.x, there was a design decision made that flags should not display a default value, The code in question within Click is located here:

https://github.com/pallets/click/blob/main/src/click/core.py#L2771

However from a UX perspective it's not ideal that there is no default value set on a flag. How is a user meant to know if toggling that flag sets the underlying variable to True or False? The name of the flag should help with this, but to make it as simple as possible it's much more preferable to show the actual default value.

But... the code that actually controls this within Typer is located in core.py, here: https://github.com/tiangolo/typer/blob/master/typer/core.py#L515

This is almost a direct carbon copy of the code above in Click's core module. But click's module has this extra code underneath:

        if show_default_is_str or (show_default and (default_value is not None)):
            if show_default_is_str:
                default_string = f"({self.show_default})"
            elif isinstance(default_value, (list, tuple)):
                default_string = ", ".join(str(d) for d in default_value)
            elif inspect.isfunction(default_value):
                default_string = _("(dynamic)")
            elif self.is_bool_flag and self.secondary_opts:
                # For boolean flags that have distinct True/False opts,
                # use the opt without prefix instead of the value.
                default_string = split_opt(
                    (self.opts if self.default else self.secondary_opts)[0]
                )[1]
            elif self.is_bool_flag and not self.secondary_opts and not default_value:
                default_string = ""
            else:
                default_string = str(default_value)

            if default_string:
                extra.append(_("default: {default}").format(default=default_string))

Whereas Typer has this:

        # Typer override:
        # Extracted to _extract_default() to allow re-using it in rich_utils
        default_value = self._extract_default_help_str(ctx=ctx)
        # Typer override end

        show_default_is_str = isinstance(self.show_default, str)

        if show_default_is_str or (
            default_value is not None and (self.show_default or ctx.show_default)
        ):
            # Typer override:
            # Extracted to _get_default_string() to allow re-using it in rich_utils
            default_string = self._get_default_string(
                ctx=ctx,
                show_default_is_str=show_default_is_str,
                default_value=default_value,
            )
            # Typer override end
            if default_string:
                extra.append(_("default: {default}").format(default=default_string))

Typer is doing some shenanigans up in line 148:

def _extract_default_help_str(
    obj: Union["TyperArgument", "TyperOption"], *, ctx: click.Context
) -> Optional[Union[Any, Callable[[], Any]]]:
    # Extracted from click.core.Option.get_help_record() to be reused by
    # rich_utils avoiding RegEx hacks
    # Temporarily enable resilient parsing to avoid type casting
    # failing for the default. Might be possible to extend this to
    # help formatting in general.
    resilient = ctx.resilient_parsing
    ctx.resilient_parsing = True

    try:
        if _get_click_major() > 7:
            default_value = obj.get_default(ctx, call=False)
        else:
            if inspect.isfunction(obj.default):
                default_value = "(dynamic)"
            else:
                default_value = obj.default
    finally:
        ctx.resilient_parsing = resilient
    return default_value

In theory we could do some special handling to override the Click design decision that's causing this. Or we can ask the Click devs if they'd be willing to allow some sort of configuration option in the click.core.Option class to specify that we always want a default value shown for variables defined as a flag (regardless of whether it defaults to True or False).

I'm happy to raise an issue over in the Click repository and cross reference this one @tiangolo, I'll wait on your advice before doing so. If you'd like to handle this situation directly within Typer, then I won't raise that ticket with the Click guys.

@Skyler-Altol
Copy link
Author

Skyler-Altol commented Nov 13, 2022

Further info that this change was an intended design decision on Click's part:

image

from: https://click.palletsprojects.com/en/8.1.x/options/

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

No branches or pull requests

2 participants