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

add typing annotations #1856

Merged
merged 4 commits into from Apr 23, 2021
Merged

add typing annotations #1856

merged 4 commits into from Apr 23, 2021

Conversation

davidism
Copy link
Member

@davidism davidism commented Apr 23, 2021

Adds full type annotations, with a strict config. py.typed file is added, so these annotations will be preferred over typeshed. Notes on some issues discovered / future work:

  • Command.name, Parameter.name, etc. can all be None, but this makes the typing more complex than it has to be.
    • For commands, name can be None as long as it's provided when using Group.add_command. The test runner will set name to "root" if the entry point doesn't have a name. I would deprecate not passing a name (it's only used this way in a few tests, it's already impossible when using the decorators).
    • For parameters, name can be None only if expose_value is disabled. Behind the scenes, the values are still recorded by the parser, but they're all overwrite the None key, which makes the dict type more annoying since you either have to ignore that the key is Optional[str] or put extra casts/asserts all over the place since the code never actually accesses that key. I would change this to not record the value at all if it's not exposed.
  • Done: WindowsChunkedWriter was only used for a Windows 7 / Python 2 compat, and is no longer needed.
  • Done: get_strerror can be removed, OSError always has strerror and it's only called on OSError.
  • Various IO wrappers don't sublcass io base classes, and so they need to be cast() all over.
  • Generics could be used. In Werkzeug, I couldn't make the data structure classes generic because instantiation is slower until 3.9, and they get created at a high enough volume each request to make a meaningful difference. In Click it's less clear what the performance implications would be, some of these classes would get created at startup when defining the CLI, but as a one time, lower volume cost.
    • IO classes could be generic so that they work with either str or bytes explicitly.
    • ParamType could be generic so that convert returns the expected type. This would also cut down on a lot of ignores on the number mixins.
    • ProgressBar could be generic so that it yields the expected type.
  • The parser allows ctx to be optional, with a comment "for advanced use cases", but none of those use cases are suggested or tested, and the module is intended to be internal.
  • Core classes and decorators should take all arguments instead of hiding most of them in **kwargs. This would make IDE completion a lot better.
    • We should figure out how to transition to keyword-only arguments in most places. Too bad positional-only isn't until 3.8, that would help with invoke and forward.

@davidism davidism added this to the 8.0.0 milestone Apr 23, 2021
@davidism davidism merged commit ffeddba into master Apr 23, 2021
@davidism davidism deleted the typing branch April 23, 2021 16:25
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 8, 2021
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

1 participant