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

Clean up type annotations #133

Merged
merged 8 commits into from
May 12, 2023

Conversation

mjpieters
Copy link
Collaborator

@mjpieters mjpieters commented May 9, 2023

Typing cleanup

The codebase now passes mypy --strict and pyright type checks, and I've included mypy and pyright in the CI checks to keep it that way.

  • Compatibility with older Python versions

    • use Optional and Union instead of ... | None and a | b
    • use typing_extensions.Protocol instead of typing.Protocol
    • use typing.Dict, typing.List, etc. instead of the concrete types.
  • Fix incorrect type hints

    • Fix backend .get() annotations; not all were marked as Optional[str]
    • The Coder.decode_as_type() type parameter must be a type to be compatible with ModelField(..., type_=...).
    • Clean up Optional[] use, remove where it is not needed.
    • Clean up variable use in decorator, keeping the raw cached value separate from the return value from the wrapped endpoint.
    • Add targeted ignores and casts where the type checker can't quite see what is going on
    • Annotate the decorator wrapper as returning either the original type or a Response (returning a 304 Not Modified response).
    • Correct type annotation on JsonCoder.decode() to match Coder.decode().
  • Code fixes guided by the type checker

    • Don't return anything from Backend.set() methods.
    • Clean up small edge-case in the decorator where response could be None.
    • Correct memcached.get() use of aiomemcache.Client.get(); only pass in the (bytes) key once, and decode the value from bytes to string.
  • Use additional typing packages to provide type hints for aiobotocore and redis.

@mjpieters mjpieters changed the title Correct type annotation on JsonCoder.decode() Clean up type annotations May 9, 2023
@mjpieters mjpieters force-pushed the correct_json_decode_return branch from d678a45 to 2de89ab Compare May 9, 2023 14:50
@mjpieters
Copy link
Collaborator Author

mjpieters commented May 9, 2023

Sorry, this PR has ballooned a bit, once I realised there were more issues than just the one little error.

I've included a commit here that expands tests to cover all supported Python versions so that this kind of issue can be caught sooner in future. You may want to update your branch protection rules to require the new test run names.

@mjpieters mjpieters force-pushed the correct_json_decode_return branch from 3396ec0 to b5263f0 Compare May 9, 2023 16:55
@long2ice
Copy link
Owner

Thanks! Please resolve conflicts.

@mjpieters mjpieters force-pushed the correct_json_decode_return branch 6 times, most recently from adbc104 to 969b019 Compare May 10, 2023 18:02
pyproject.toml Outdated Show resolved Hide resolved
@mjpieters mjpieters force-pushed the correct_json_decode_return branch from 969b019 to 3b97429 Compare May 11, 2023 11:27
mjpieters added 8 commits May 11, 2023 12:31
- Compatibility with older Python versions
  - use `Optional` and `Union` instead of `... | None` and `a | b`
  - use `typing_extensions.Protocol` instead of `typing.Protocol`
  - use `typing.Dict`, `typing.List`, etc. instead of the concrete types.

- Fix backend `.get()` annotations; not all were marked as `Optional[str]`
- Don't return anything from `Backend.set()` methods.
- The `Coder.decode_as_type()` type parameter must be a type to be
  compatible with `ModelField(..., type_=...)`.
- Clean up `Optional[]` use, remove where it is not needed.
- Clean up variable use in decorator, keeping the raw cached value
  separate from the return value from the wrapped endpoint.
- Annotate the wrapper as returning either the original type _or_ a
  Response (returning a 304 Not Modified response).
- Clean up small edge-case where `response` could be `None`.
- Correct type annotation on `JsonCoder.decode()` to match `Coder.decode()`.
This lets you catch compatibility issues regardless of the current python version used for development.
@mjpieters mjpieters force-pushed the correct_json_decode_return branch from 3b97429 to 0763cd7 Compare May 11, 2023 11:57
@long2ice long2ice merged commit 9638d70 into long2ice:main May 12, 2023
@long2ice
Copy link
Owner

Thanks!

@mjpieters mjpieters deleted the correct_json_decode_return branch May 12, 2023 09:02
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