-
-
Notifications
You must be signed in to change notification settings - Fork 170
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
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mjpieters
changed the title
Correct type annotation on JsonCoder.decode()
Clean up type annotations
May 9, 2023
mjpieters
force-pushed
the
correct_json_decode_return
branch
from
May 9, 2023 14:50
d678a45
to
2de89ab
Compare
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
force-pushed
the
correct_json_decode_return
branch
from
May 9, 2023 16:55
3396ec0
to
b5263f0
Compare
Thanks! Please resolve conflicts. |
mjpieters
force-pushed
the
correct_json_decode_return
branch
6 times, most recently
from
May 10, 2023 18:02
adbc104
to
969b019
Compare
mkdir700
reviewed
May 11, 2023
mjpieters
force-pushed
the
correct_json_decode_return
branch
from
May 11, 2023 11:27
969b019
to
3b97429
Compare
- 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
force-pushed
the
correct_json_decode_return
branch
from
May 11, 2023 11:57
3b97429
to
0763cd7
Compare
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Typing cleanup
The codebase now passes
mypy --strict
andpyright
type checks, and I've includedmypy
andpyright
in the CI checks to keep it that way.Compatibility with older Python versions
Optional
andUnion
instead of... | None
anda | b
typing_extensions.Protocol
instead oftyping.Protocol
typing.Dict
,typing.List
, etc. instead of the concrete types.Fix incorrect type hints
.get()
annotations; not all were marked asOptional[str]
Coder.decode_as_type()
type parameter must be a type to be compatible withModelField(..., type_=...)
.Optional[]
use, remove where it is not needed.Response
(returning a 304 Not Modified response).JsonCoder.decode()
to matchCoder.decode()
.Code fixes guided by the type checker
Backend.set()
methods.response
could beNone
.memcached.get()
use ofaiomemcache.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.