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

Types: don't leave generic types without a parameter #2401

Merged
merged 5 commits into from
Jan 20, 2023

Conversation

mjpieters
Copy link
Contributor

@mjpieters mjpieters commented Nov 8, 2022

Enable disallow_any_generics and provide type information for missing parameters for type hints.

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.

@mjpieters mjpieters marked this pull request as ready for review November 9, 2022 17:12
@davidism
Copy link
Member

I'm really interested in the patterns you used for decorators by using ParamSpec. It's clearly a more sophisticated pattern than the one mypy documents. Is there anywhere I can read more about it?

@davidism
Copy link
Member

Also, I usually consider typing changes bug fixes, so you can retarget this to 8.1.x (although I may just make the next release 8.2 anyway).

@mjpieters
Copy link
Contributor Author

mjpieters commented Nov 10, 2022

I'm really interested in the patterns you used for decorators by using ParamSpec. It's clearly a more sophisticated pattern than the one mypy documents. Is there anywhere I can read more about it?

Sure: ParamSpec and Concatenate are defined in PEP 612 – Parameter Specification Variables.

@mjpieters
Copy link
Contributor Author

Also, I usually consider typing changes bug fixes, so you can retarget this to 8.1.x (although I may just make the next release 8.2 anyway).

I wasn't aware I targeted anything :-D I found the Version 8.2.0 -- Unreleased section and put my comment there. Should I inject a Version 8.1.4 -- Unreleased section, should I replace 8.2.0 with 8.1.4, or what?

@davidism
Copy link
Member

You can rebase to the 8.1.x branch then change the PR target. Yeah, just add a section for another unreleased version, or I can handle the changelog.

@mjpieters mjpieters force-pushed the types_disallow_any_generics branch 2 times, most recently from 244aac2 to c4bbc9f Compare November 10, 2022 15:45
@mjpieters mjpieters changed the base branch from main to 8.1.x November 10, 2022 15:46
@mjpieters
Copy link
Contributor Author

You can rebase to the 8.1.x branch then change the PR target. Yeah, just add a section for another unreleased version, or I can handle the changelog.

Check, done.

It's better to change the PR target first and then push a rebased set of commits, as the github checks won't be re-run on a PR target change. I had to force a new commit SHA so I could force-push again and trigger the pipeline. ;-)

@coloursofnoise
Copy link

Having the ParamSpec type hint on decorators like pass_context is a very welcome change, since it helps to enforce the correct number of arguments in the command callback function.

I'm currently doing this as part of my unit tests, but I was also looking into creating a PR similar to this one before I saw it already exists. I will definitely be testing this PR in development going forward.

mjpieters and others added 5 commits January 19, 2023 16:33
Enable `disallow_any_generics` and provide type information for missing parameters for type hints.
Clean out a series of ignores, either by specifying types or by reworking code slightly the ignore is no longer needed.
A combination of overloads, TypeVar, ParamSpec and Concatenate make it possible to tell the type checker more about what kinds of callables are expected and what is being returned.
@davidism davidism added this to the 8.1.4 milestone Jan 20, 2023
@davidism davidism merged commit c0092d2 into pallets:8.1.x Jan 20, 2023
@mjpieters mjpieters deleted the types_disallow_any_generics branch January 20, 2023 18:45
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 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.

Type hints: public API uses generic hints without parameters
3 participants