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

Complete type hints #122

Merged
merged 9 commits into from
Apr 28, 2023
Merged

Complete type hints #122

merged 9 commits into from
Apr 28, 2023

Conversation

mjpieters
Copy link
Collaborator

This series of commits brings FastAPI-Cache up to type completeness, as verified by
pyright
:

$ pyright --verifytypes fastapi_cache --ignoreexternal
Module name: "fastapi_cache"
Package directory: "/Users/mj/Development/Projects/OSS/fastapi-cache/fastapi_cache"
Module directory: "/Users/mj/Development/Projects/OSS/fastapi-cache/fastapi_cache"
Path of py.typed file: "/Users/mj/Development/Projects/OSS/fastapi-cache/fastapi_cache/py.typed"

Public modules: 10
   fastapi_cache
   fastapi_cache.backends
   fastapi_cache.backends.dynamodb
   fastapi_cache.backends.inmemory
   fastapi_cache.backends.memcached
   fastapi_cache.backends.redis
   fastapi_cache.coder
   fastapi_cache.decorator
   fastapi_cache.key_builder
   fastapi_cache.types

Symbols used in public interface:

Symbols exported by "fastapi_cache": 76
  With known type: 76
  With ambiguous type: 0
  With unknown type: 0
    (Ignoring unknown types imported from other packages)
  Functions without docstring: 40
  Functions without default param: 1
  Classes without docstring: 11

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

Type completeness score: 100%

Completed in 0.478sec
  • Correct type hint: namespace is not optional
  • Mark up the class variables as such
  • Remove a type: ignore comment
  • JSONResponse.body is UTF-8 bytes and must be decoded
  • Define keybuilder protocol
  • Simplify key_builder calling
  • Use complete type hints with all generic parameters filled
  • Provide annotation for the session attribute
  • The backend needs an async redis client with a pipeline method

The namespace argument is positional and will never be `None` so should
not be marked as Optional. It is always a string, and the default is
to pass in an empty string.
There is never an instance of this class, so these are not instance attributes.
GIve the type checker more information about the converters instead.
This lets others create key builders that are type checked fully.
The keybuilder is either returning a string, or a coroutine or other
awaitable. If the latter, await on the return value to get the string.
This makes the core fastapi_cache pass all strict type checker tests.
The Abstract* classes lack the pipeline method so are not sufficient.
Copy link
Contributor

@mkdir700 mkdir700 left a comment

Choose a reason for hiding this comment

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

nice.

@long2ice long2ice merged commit b26059b into long2ice:main Apr 28, 2023
@long2ice
Copy link
Owner

Thanks!

@mjpieters mjpieters deleted the type-hinting branch April 28, 2023 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants