-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
feat: add typevar expansion #3242
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3242 +/- ##
========================================
Coverage 98.25% 98.25%
========================================
Files 322 322
Lines 14666 14670 +4
Branches 2333 2334 +1
========================================
+ Hits 14410 14414 +4
Misses 116 116
Partials 140 140 ☔ View full report in Codecov by Sentry. |
litestar/utils/signature.py
Outdated
"""Expand TypeVar for any parameters in type_hint | ||
|
||
Args: | ||
type_hint: mapping of parameter to type obtained from calling `get_type_hints` or `get_fn_type_hints` | ||
namespace: mapping of TypeVar to concrete type | ||
|
||
Returns: | ||
type_hint with any TypeVar parameter expanded | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"""Expand TypeVar for any parameters in type_hint | |
Args: | |
type_hint: mapping of parameter to type obtained from calling `get_type_hints` or `get_fn_type_hints` | |
namespace: mapping of TypeVar to concrete type | |
Returns: | |
type_hint with any TypeVar parameter expanded | |
""" | |
"""Expand :class:`~typing.TypeVar` for any parameters in ``type_hint`` | |
Args: | |
type_hint: mapping of parameter to type obtained from calling ``get_type_hints`` or ``get_fn_type_hints`` | |
namespace: mapping of :class:`~typing.TypeVar` to concrete type | |
Returns: | |
``type_hint`` with any :class:`~typing.TypeVar` parameter expanded | |
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @haryle! This is a cool change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @haryle - LGTM!
One nit, can we please not add arbitrary style changes - there are a few cases of added line wraps for imports that would otherwise be lines untouched by the PR. Perhaps you have some global formatter settings that conflict with ours?
Cheers
litestar/utils/typing.py
Outdated
from typing_extensions import ( | ||
Annotated, | ||
NotRequired, | ||
Required, | ||
get_args, | ||
get_origin, | ||
get_type_hints, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please limit the changes to those necessary for the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, all the import formating changes were done by my linter. I will reformat them.
TypedDict, | ||
get_args, | ||
get_type_hints, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to go after the comments by @peterschutt has been resolved :)
Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/3242 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@all-contributors add @haryle code tests |
I've put up a pull request to add @haryle! 🎉 |
Description
Add a method for typevar expansion on registration
This allows the use of generic route handler and generic controller without relying on forward references.
Closes #3206
A self-contained example - which would fail in the previous version:
All failed tests were not caused by this new update. Please let me know what you think.