-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fixing the return type hint for the transaction method in the standalone client. #3660
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
Fixing the return type hint for the transaction method in the standalone client. #3660
Conversation
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.
Pull Request Overview
This PR updates the return type hint for the transaction
method in the standalone Redis client to reflect that it may return the results of Pipeline.execute()
, None
, or a custom callback’s return value.
- Changed the return annotation of
transaction
fromNone
toOptional[Union[List[Any], Any]]
- Adjusted the method signature to allow for custom callback return types
Comments suppressed due to low confidence (3)
redis/client.py:454
- The docstring doesn’t describe the new return values. Please add a
Returns:
section explaining that this method may return the list fromexecute
, the custom callback’s return value, orNone
.
"""
redis/client.py:450
- Add tests covering scenarios where
transaction
returns the custom callback result, the list fromexecute
, andNone
. This ensures the new return types are exercised.
def transaction(
redis/client.py:453
- Ensure that
Optional
,Union
,List
, andAny
are imported from thetyping
module (orcollections.abc
in newer versions) to prevent unresolved references.
) -> Optional[Union[List[Any], Any]]:
redis/client.py
Outdated
self, func: Callable[["Pipeline"], None], *watches, **kwargs | ||
) -> None: | ||
) -> Optional[Union[List[Any], Any]]: |
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.
The return type hint Optional[Union[List[Any], Any]]
can be simplified. Since Any
already encompasses List[Any]
, consider using -> Any
or -> Optional[Any]
. For stronger typing, introduce a TypeVar
for the callback return: e.g.
R = TypeVar('R')
def transaction(self, func: Callable[["Pipeline"], R], *watches, **kwargs) -> Optional[R]:
...
Copilot uses AI. Check for mistakes.
…d_teturn_type_annotation
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
Fixing the return type hint for the transaction method in the standalone client.
The method returns either the returned response from execute method call which is List[Any] or None, or the response from the provided custom function, which can be anything.
Fixes issue #3631