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 types, introduce type tests #2562

Merged
merged 11 commits into from Jul 13, 2023
Merged

Conversation

Tinche
Copy link
Contributor

@Tinche Tinche commented Jul 9, 2023

Fix signatures, introduce simple tests.

I followed the Flask model for typing tests. Initially I was going to replicate the attrs model (https://github.com/python-attrs/attrs/blob/main/tests/test_mypy.yml) but maybe this is good enough to start?

Also introduced Pyright tests, again copied from attrs.

Comment on lines -162 to +157
cls: t.Type[CmdType] = ...,
*,
cls: t.Type[CmdType],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the same change should be made to group's third overload.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, but I think it's going to be up to the community to fill out the tests and fix the fallout. This PR is mainly to set up the infrastructure for it.

@davidism
Copy link
Member

davidism commented Jul 9, 2023

We were also using pyright --verifytypes click, not sure what the exact difference is but might want to run both.

Should this be used instead of #2559?

@Tinche
Copy link
Contributor Author

Tinche commented Jul 9, 2023

Should this be used instead of #2559?

I've added help_option to the typing tests, so this is now a strict superset. So pretty much yeah.

@Tinche
Copy link
Contributor Author

Tinche commented Jul 9, 2023

We were also using pyright --verifytypes click, not sure what the exact difference is but might want to run both.

I've added it. Looks like it prints out a bunch of stuff and succeeds.

pyright: commands[0]> pyright --verifytypes click
Module name: "click"
Package directory: "/home/tin/pg/click/.tox/pyright/lib/python3.11/site-packages/click"
Module directory: "/home/tin/pg/click/.tox/pyright/lib/python3.11/site-packages/click"
Path of py.typed file: "/home/tin/pg/click/.tox/pyright/lib/python3.11/site-packages/click/py.typed"

Public modules: 12
   click
   click.core
   click.decorators
   click.exceptions
   click.formatting
   click.globals
   click.parser
   click.shell_completion
   click.termui
   click.testing
   click.types
   click.utils

Symbols used in public interface:

Symbols exported by "click": 646
  With known type: 646
  With ambiguous type: 0
  With unknown type: 0
  Functions without docstring: 98
  Functions without default param: 22
  Classes without docstring: 13

Other symbols referenced but not exported by "click": 247
  With known type: 247
  With ambiguous type: 0
  With unknown type: 0

Type completeness score: 100%

Completed in 1.389sec

@ofek
Copy link
Contributor

ofek commented Jul 12, 2023

Is this ready to be merged?

@davidism
Copy link
Member

Yes, I just need to find some time, busy with regular job.

@davidism davidism changed the base branch from main to 8.1.x July 13, 2023 12:58
@davidism
Copy link
Member

I've switched from installing and using nodenv directly to installing the pyright package from PyPI. This allows us to pin the pyright version the same as mypy. I prefer to pin these tools because new versions regularly cause our tests to fail, which is confusing to contributors making PRs. (That said, the failures are usually nice things like being able to remove ignore comments.) Everything seems to work still, but let me know if there was a specific reason for using nodenv directly.

@davidism davidism merged commit 449d38f into pallets:8.1.x Jul 13, 2023
12 checks passed
@Tinche
Copy link
Contributor Author

Tinche commented Jul 13, 2023

Cool, thanks for the tip. I've actually borrowed this code from attrs, I'm not a pyright expert or anything. Maybe @hynek has an opinion?

@Tinche Tinche deleted the tin/typing-tests branch July 13, 2023 15:08
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 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.

Typing changes in 8.1.4 cause failure of mypy check
4 participants