Skip to content

Fix types, introduce type tests#2562

Merged
davidism merged 11 commits intopallets:8.1.xfrom
Tinche:tin/typing-tests
Jul 13, 2023
Merged

Fix types, introduce type tests#2562
davidism merged 11 commits intopallets:8.1.xfrom
Tinche:tin/typing-tests

Conversation

@Tinche
Copy link
Copy Markdown
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.

@Tinche Tinche mentioned this pull request Jul 9, 2023
Closed
layday
layday reviewed Jul 9, 2023
View reviewed changes
Comment thread src/click/decorators.py
Comment on lines -162 to +157
cls: t.Type[CmdType] = ...,
*,
cls: t.Type[CmdType],
Copy link
Copy Markdown
Contributor

@layday layday Jul 9, 2023

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
Copy Markdown
Contributor Author

@Tinche Tinche Jul 9, 2023

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
Copy Markdown
Member

davidism commented Jul 9, 2023 •
edited
Loading

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
Copy Markdown
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
Copy Markdown
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

@davidism davidism mentioned this pull request Jul 9, 2023
Closed
6 tasks
This was referenced Jul 12, 2023
Merged
Merged
@ofek
Copy link
Copy Markdown
Contributor

ofek commented Jul 12, 2023

Is this ready to be merged?

@davidism
Copy link
Copy Markdown
Member

davidism commented Jul 12, 2023

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

This was referenced Jul 12, 2023
Closed
Merged
@davidism davidism linked an issue Jul 13, 2023 that may be closed by this pull request
Typing changes in 8.1.4 cause failure of mypy check #2558
Closed
@davidism davidism added this to the 8.1.5 milestone Jul 13, 2023
@davidism davidism changed the base branch from main to 8.1.x July 13, 2023 12:58
@davidism
Copy link
Copy Markdown
Member

davidism commented Jul 13, 2023

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
@Tinche
Copy link
Copy Markdown
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.

Reviewers

1 more reviewer

@layday layday layday left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

8.1.5

Development

Successfully merging this pull request may close these issues.

Typing changes in 8.1.4 cause failure of mypy check

4 participants

@Tinche @davidism @ofek @layday