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

Fix group overload #2565

Merged
merged 1 commit into from Jul 18, 2023
Merged

Fix group overload #2565

merged 1 commit into from Jul 18, 2023

Conversation

layday
Copy link
Contributor

@layday layday commented Jul 13, 2023

Additional fixes for #2558 building on @Tinche's PR :)


Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@Tinche
Copy link
Contributor

Tinche commented Jul 13, 2023

Out of curiosity, what does strict mode do for pyright?

@layday
Copy link
Contributor Author

layday commented Jul 13, 2023

It turns on these additional checks. In my experience Pyright won't error for type vars it's not able to solve with default settings. The group overload error would've been caught by the assert, but there might be auxiliary errors we are not explicitly testing against that strict mode might be able to detect.

@Tinche
Copy link
Contributor

Tinche commented Jul 13, 2023

Cool, thanks. What's the default type checking mode? It's not off I hope

Edit: it's basic, found it in the docs finally

@ofek
Copy link
Contributor

ofek commented Jul 13, 2023

While you are fixing things, does this look like a bug? #2558 (comment)

Your change fixes the issue actually, thanks! #2558 (comment)

@Tinche
Copy link
Contributor

Tinche commented Jul 13, 2023

LGTM FWIW. Might want to add a changelog entry.

@alex
Copy link

alex commented Jul 15, 2023

Will this resolve the typing of:

@click.group()
def cli():
    pass


@cli.command()
@click.argument("version")
def release(version: str) -> None:
    # ...

or is this a distinct issue?

@layday
Copy link
Contributor Author

layday commented Jul 15, 2023

That doesn't produce an error for me. Have you tried with click 8.1.5? Fixed by this PR.

@davidism davidism changed the title Fix group overload; run Pyright in strict mode Fix group overload Jul 18, 2023
@davidism davidism merged commit 1a15373 into pallets:8.1.x Jul 18, 2023
12 checks passed
@davidism
Copy link
Member

Click 8.1.6 released.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants