Skip to content

Refactor(Typing): Add and update type annotations for core Redis commands #3281

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+461 −494
Open

Refactor(Typing): Add and update type annotations for core Redis commands #3281

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
31a4cf4
refactor: Add and update type annotations for core Redis commands
ElayGelbart Jun 15, 2024
0e1002c
Merge branch 'master' into typing/corecommands
ElayGelbart Jun 21, 2024
c0c88a0
Merge branch 'master' into typing/corecommands
gerzse Jul 3, 2024
07744ab
Merge branch 'master' into typing/corecommands
ElayGelbart Jul 5, 2024
abc67d1
Merge remote-tracking branch 'upstream/master' into typing/corecommands
ElayGelbart Aug 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
938 changes: 448 additions & 490 deletions redis/commands/core.py
View file

Large diffs are not rendered by default.

17 changes: 13 additions & 4 deletions redis/typing.py
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# from __future__ import annotations

from datetime import datetime, timedelta
from typing import (
TYPE_CHECKING,
Any,
Awaitable,
Iterable,
List,
Literal,
Mapping,
Protocol,
Type,
@@ -32,7 +32,14 @@
PatternT = _StringLikeT # Patterns matched against keys, fields etc
FieldT = EncodableT # Fields within hash tables, streams and geo commands
KeysT = Union[KeyT, Iterable[KeyT]]
ResponseT = Union[Awaitable[Any], Any]
OldResponseT = Union[Awaitable[Any], Any] # Deprecated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used anymore, I would say just get rid of it.

AnyResponseT = TypeVar("AnyResponseT", bound=Any)
ResponseT = Union[AnyResponseT, Awaitable[AnyResponseT]]
OKT = Literal[True]
ArrayResponseT = List
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two aspects here.

First, List is itself generic, wouldn't it be cleaner to keep that into account? I guess in most places it would end up being ArrayResponseT[Any], but still it would feel more correct.

Second, why define aliases to single types? You can just use List and it would be easier on the reader, I think.

IntegerResponseT = int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same point here, just use int, why alias it? At first sight this is a bit of speculative generality.

NullResponseT = type(None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems not used at all.

BulkStringResponseT = str
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, why not use str directly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for clarity, it isn't just any str it is a BulkStringResponseT even better would be a uage of newtype https://docs.python.org/3/library/typing.html#newtype

ChannelT = _StringLikeT
GroupT = _StringLikeT # Consumer group
ConsumerT = _StringLikeT # Consumer name
@@ -54,8 +61,10 @@
class CommandsProtocol(Protocol):
connection_pool: Union["AsyncConnectionPool", "ConnectionPool"]

def execute_command(self, *args, **options) -> ResponseT: ...
def execute_command(self, *args, **options) -> ResponseT[Any]: ...


class ClusterCommandsProtocol(CommandsProtocol):
encoder: "Encoder"

def execute_command(self, *args, **options) -> ResponseT[Any]: ...