Resolve types correctly (PEP 563, postponed annotations) #5461
Conversation
|
📝 Docs preview for commit 07fd3b6 at: https://633def1f4e0be700889c7cfe--fastapi.netlify.app |
mastercoms
force-pushed
the
patch-1
branch
2 times, most recently
from
5f2e2ac to
c2378bf
Compare
fixes fastapi#1654 fixes fastapi#4557 fixes fastapi#4750 fixes fastapi#5065
|
Currently failing due to python/mypy#5079 |
|
📝 Docs preview for commit e4bc092 at: https://633df31cb6573709f4f8792d--fastapi.netlify.app |
mastercoms
changed the title
|
📝 Docs preview for commit 0dc7591 at: https://633e820e75af827a0c144fd4--fastapi.netlify.app |
|
📝 Docs preview for commit fe9d827 at: https://633e88b0ec49c476bb822d6d--fastapi.netlify.app |
|
📝 Docs preview for commit c6c279d at: https://633ebec666be07349e88ef16--fastapi.netlify.app |
|
📝 Docs preview for commit d0b0c11 at: https://633ec2c8b16274004cf10f4d--fastapi.netlify.app |
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #5461 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 540 540
Lines 13969 13947 -22
=========================================
- Hits 13969 13947 -22
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
📝 Docs preview for commit af686a5 at: https://633ec456e6bb360061861352--fastapi.netlify.app |
|
Now building and passing tests, waiting on #5456 to fix some checks. |
|
📝 Docs preview for commit 7350706 at: https://634b34d37268307b95f8fc7e--fastapi.netlify.app |
|
📝 Docs preview for commit fd01fa8 at: https://6384d47e14b0af109c74e5cf--fastapi.netlify.app |
|
📝 Docs preview for commit 3a51083 at: https://639ce6c12c118d073e47facd--fastapi.netlify.app |
|
📝 Docs preview for commit 8fddf4f at: https://63e27637ee4cdf05497ab55a--fastapi.netlify.app |
|
📝 Docs preview for commit 4fd7b04 at: https://63e281e2c68e580b386f349f--fastapi.netlify.app |
|
I'll be looking to update this for 0.89.1 soon. |
| @@ -247,36 +247,36 @@ def is_scalar_sequence_field(field: ModelField) -> bool: | |||
|
|
|||
| def get_typed_signature(call: Callable[..., Any]) -> inspect.Signature: | |||
| signature = inspect.signature(call) | |||
| globalns = getattr(call, "__globals__", {}) | |||
| is_true_function = ( | |||
There was a problem hiding this comment.
It might look a bit nicer here if you pass a tuple of arguments to isinstance.
if isinstance(call, (FunctionType, MethodType, BuiltinFunctionType))
|
The project seems to have taken a different path on this, so closing since I cannot provide updates for new usage of the old annotations. |
This commit fixes various problems stemming from incorrect evaluation of types in function signatures.
First, we use Python's own
typing.get_type_hintsmethod to ensure we can resolve type hints with the correct scope. Python handles classes, wrapped functions which are not considered in the current implementation. I also prefer this approach as it will most likely be the best at handling any future changes to Python.However, unfortunately,
get_type_hintsresolves type hints for the class__init__function when passed a callable class, so I had to add some checks to see if the__call__function should be passed in rather than the Callable itself. If the callable is an actual function of some form, we can pass itself toget_type_hints. If the callable is a type annotation, we need to call the class type's__init__function, and thus we pass that intoget_type_hints. Otherwise, we pass in the__call__function.Finally, the return type with the old approach was not resolved, so it used the default
return_annotation=typing.Any.get_type_hintsis able to retrieve return type information as well, so I was able to pass in the return annotation and thus the return type is no longer erased. In the case that no return type annotation is found, we pass in the default ofAnyagain to represent the unknown status of the type.