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

Updated get_group and get_group_from_info to return a Click Group object #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Updated get_group and get_group_from_info to return a Click Group object #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions typer/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ def __call__(self) -> Any:
return get_command(self)()


def get_group(typer_instance: Typer) -> click.Command:
def get_group(typer_instance: Typer) -> click.Group:
group = get_group_from_info(TyperInfo(typer_instance))
return group

Expand All @@ -228,11 +228,11 @@ def get_command(typer_instance: Typer) -> click.Command:
or len(typer_instance.registered_commands) > 1
):
# Create a Group
click_command = get_group(typer_instance)
Copy link
Member Author

Choose a reason for hiding this comment

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

I've renamed from click_command to click_group mainly because otherwise it would return Incompatible types in assignment (expression has type "Command", variable has type "Group"). Seems to be a scope issue?

Copy link
Member

Choose a reason for hiding this comment

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

It's a type error! My understanding is that Group is more specific than Command, and this function returns a Command. So you'd probably want to cast the result of get_group.

Copy link
Member Author

@csr csr Jun 11, 2020 •
edited

Choose a reason for hiding this comment

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

@juped Thank you.

Do you suggest making so that main.get_group returns a click.Group (I believe this is a requirement for mypy to run successfully)? By looking at the example code in the issue page, the user was calling get_group from their script, so I'm assuming for mypy to run successfully we need main.get_group to return a click.Group?

If yes, then I'm trying to figure out how to cast the result of main.get_group_from_info (which is what main.get_group is calling) from click.Command to click.Group. Is this how you do it in Python:

def get_group(typer_instance: Typer) -> click.Group:
    group = click.Group(get_group_from_info(TyperInfo(typer_instance)))
    return group

Thank you!

Copy link
Member

Choose a reason for hiding this comment

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

This is at the type hinting level, where I believe it's cast().

Copy link
Member Author

@csr csr Jun 11, 2020 •
edited

Choose a reason for hiding this comment

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

I tried using return cast(click.Group, group) in main.get_group (this requires an import of cast from the typing module), like this:

def get_group(typer_instance: Typer) -> click.Command:
    group = get_group_from_info(TyperInfo(typer_instance))
    return cast(click.Group, group)

However when I run mypy on the sample script in the issue I get

main.py:16: error: "Command" has no attribute "command"
Found 1 error in 1 file (checked 1 source file)

which isn't that helpful as line 16 is just a series of imports. Mmm...

click_group = get_group(typer_instance)
if typer_instance._add_completion:
click_command.params.append(click_install_param)
click_command.params.append(click_show_param)
return click_command
click_group.params.append(click_install_param)
click_group.params.append(click_show_param)
return click_group
elif len(typer_instance.registered_commands) == 1:
# Create a single Command
click_command = get_command_from_info(typer_instance.registered_commands[0])
Expand Down Expand Up @@ -339,7 +339,7 @@ def solve_typer_info_defaults(typer_info: TyperInfo) -> TyperInfo:
return TyperInfo(**values)


def get_group_from_info(group_info: TyperInfo) -> click.Command:
def get_group_from_info(group_info: TyperInfo) -> click.Group:
assert (
group_info.typer_instance
), "A Typer instance is needed to generate a Click Group"
Expand All @@ -356,7 +356,7 @@ def get_group_from_info(group_info: TyperInfo) -> click.Command:
convertors,
context_param_name,
) = get_params_convertors_ctx_param_name_from_function(solved_info.callback)
cls = solved_info.cls or click.Group
Copy link
Member Author

Choose a reason for hiding this comment

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

So from the Click docs cls is the the argument class to instantiate (which, from my understanding, that could be a Command or a Group). I believe this change will basically create groups of single commands instead of creating a group or a command depending on what you need. If you need just a command, then it may not make sense to create a group (extra information in memory), but I'm not too sure.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure of this one (don't know enough about the Click internals).

cls = click.Group
group = cls( # type: ignore
name=solved_info.name or "",
commands=commands,
Expand Down