ronantakizawa/codereview-bench huggingface dataset:
Repository: redis/redis-py
File: doctests/dt_json.py (Python)

Code before:
# REMOVE_END

# STEP_START update_filters1
res16 = r.json().set("bikes:inventory", "$.inventory.*[?(@.price<2000)].price", 1500)
res17 = r.json().get("bikes:inventory", "$..price")
print(res17)    # >>> [1500, 2072, 3264, 1500, 3941]
# STEP_END

# REMOVE_START
assert res17 == [1500, 2072, 3264, 1500, 3941]
# REMOVE_END

# STEP_START update_filters2
res14 = r.json().arrappend(
    "bikes:inventory", "$.inventory.*[?(@.price<2000)].colors", "pink"
)
print(res14)  # >>> [3, 3]

res15 = r.json().get("bikes:inventory", "$..[*].colors")
print(
    res15
)  # >>> [['black', 'silver', 'pink'], ['black', 'white'], ['black', 'silver', 'pink']]
# STEP_END

# REMOVE_START
assert res15 == [
    ["black", "silver", "pink"],
    ["black", "white"],
    ["black", "silver", "pink"],
]
# REMOVE_END


Review comment (suggestion):
`res11`, `res12` and `res13` are also used below in the original code. If possible it would be cleaner to not reuse names. Also add asserts for these, either here, or below (only `res12` is asserted there).

Code after:
# REMOVE_START
assert res9 == ["Weywot", "Mimas"]
# REMOVE_END

# STEP_START filter3
res10 = r.json().get(
    "bikes:inventory", "$..[?(@.specs.material =~ '(?i)al')].model"
)
print(res10)  # >>> ['Quaoar', 'Weywot', 'Salacia', 'Mimas']
# STEP_END

# REMOVE_START
assert res10 == ["Quaoar", "Weywot", "Salacia", "Mimas"]
# REMOVE_END

# STEP_START filter4
res11 = r.json().set(
    "bikes:inventory", "$.inventory.mountain_bikes[0].regex_pat", "(?i)al"
)
res12 = r.json().set(
    "bikes:inventory", "$.inventory.mountain_bikes[1].regex_pat", "(?i)al"
)
res13 = r.json().set(
    "bikes:inventory", "$.inventory.mountain_bikes[2].regex_pat", "(?i)al"
)

res14 = r.json().get(
    "bikes:inventory",
    "$.inventory.mountain_bikes[?(@.specs.material =~ @.regex_pat)].model",
)
print(res14)  # >>> ['Quaoar', 'Weywot']
# STEP_END

# REMOVE_START
assert res14 == ["Quaoar", "Weywot"]
# REMOVE_END

# STEP_START update_bikes
res15 = r.json().get("bikes:inventory", "$..price")
print(res15)  # >>> [1920, 2072, 3264, 1475, 3941]

res16 = r.json().numincrby("bikes:inventory", "$..price", -100)
print(res16)  # >>> [1820, 1972, 3164, 1375, 3841]

res17 = r.json().numincrby("bikes:inventory", "$..price", 100)
print(res17)  # >>> [1920, 2072, 3264, 1475, 3941]
# STEP_END

# REMOVE_START
assert res15 == [1920, 2072, 3264, 1475, 3941]
assert res16 == [1820, 1972, 3164, 1375, 3841]
Repository: redis/redis-py
File: redis/retry.py (Python)

Code before:
import socket
from time import sleep
from typing import TYPE_CHECKING, Any, Iterable, TypeVar, Callable

from redis.exceptions import ConnectionError, TimeoutError

T = TypeVar("T")

if TYPE_CHECKING:
    from redis.backoff import AbstractBackoff


class Retry:
    """Retry a specific number of times after a failure"""

    def __init__(
        self,
        backoff: AbstractBackoff,
        retries: int,
        supported_errors: tuple[type[Exception], ...] = (
            ConnectionError,
            TimeoutError,
            socket.timeout,
        ),
    ):
        """
        Initialize a `Retry` object with a `Backoff` object
        that retries a maximum of `retries` times.
        `retries` can be negative to retry forever.
        You can specify the types of supported errors which trigger
        a retry with the `supported_errors` parameter.
        """
        self._backoff = backoff
        self._retries = retries
        self._supported_errors = supported_errors

    def update_supported_errors(
        self, specified_errors: Iterable[type[Exception]]
    ) -> None:
        """
        Updates the supported errors with the specified error types
        """
        self._supported_errors = tuple(

Review comment (performance):
Should `AbstractBackoff` be quoted here, when using `if TYPE_CHECKING`?

Tbh, it feels cleaner to just import `AbstractBackoff` always, it should not be a performance penalty.

Code after:
import socket
from time import sleep
from typing import TYPE_CHECKING, Any, Callable, Iterable, Tuple, Type, TypeVar

from redis.exceptions import ConnectionError, TimeoutError

T = TypeVar("T")

if TYPE_CHECKING:
    from redis.backoff import AbstractBackoff


class Retry:
    """Retry a specific number of times after a failure"""

    def __init__(
        self,
        backoff: "AbstractBackoff",
        retries: int,
        supported_errors: Tuple[Type[Exception], ...] = (
            ConnectionError,
            TimeoutError,
            socket.timeout,
        ),
    ):
        """
        Initialize a `Retry` object with a `Backoff` object
        that retries a maximum of `retries` times.
        `retries` can be negative to retry forever.
        You can specify the types of supported errors which trigger
        a retry with the `supported_errors` parameter.
        """
        self._backoff = backoff
        self._retries = retries
        self._supported_errors = supported_errors

    def update_supported_errors(
        self, specified_errors: Iterable[Type[Exception]]
    ) -> None:
        """
        Updates the supported errors with the specified error types
        """
        self._supported_errors = tuple(
Repository: redis/redis-py
File: redis/retry.py (Python)

Code before:
import socket
from time import sleep
from typing import TYPE_CHECKING, Any, Iterable, TypeVar, Callable

from redis.exceptions import ConnectionError, TimeoutError

T = TypeVar("T")

if TYPE_CHECKING:
    from redis.backoff import AbstractBackoff


class Retry:
    """Retry a specific number of times after a failure"""

    def __init__(
        self,
        backoff: AbstractBackoff,
        retries: int,
        supported_errors: tuple[type[Exception], ...] = (
            ConnectionError,
            TimeoutError,
            socket.timeout,
        ),
    ):
        """
        Initialize a `Retry` object with a `Backoff` object
        that retries a maximum of `retries` times.
        `retries` can be negative to retry forever.
        You can specify the types of supported errors which trigger
        a retry with the `supported_errors` parameter.
        """
        self._backoff = backoff
        self._retries = retries
        self._supported_errors = supported_errors

    def update_supported_errors(
        self, specified_errors: Iterable[type[Exception]]
    ) -> None:
        """
        Updates the supported errors with the specified error types
        """
        self._supported_errors = tuple(
            set(self._supported_errors + tuple(specified_errors))
        )

Review comment (suggestion):
We support Python 3.8, where this is not yet available. You can use `Tuple[Type[Exception], ...]` and import them from `typing`.

Code after:
import socket
from time import sleep
from typing import TYPE_CHECKING, Any, Callable, Iterable, Tuple, Type, TypeVar

from redis.exceptions import ConnectionError, TimeoutError

T = TypeVar("T")

if TYPE_CHECKING:
    from redis.backoff import AbstractBackoff


class Retry:
    """Retry a specific number of times after a failure"""

    def __init__(
        self,
        backoff: "AbstractBackoff",
        retries: int,
        supported_errors: Tuple[Type[Exception], ...] = (
            ConnectionError,
            TimeoutError,
            socket.timeout,
        ),
    ):
        """
        Initialize a `Retry` object with a `Backoff` object
        that retries a maximum of `retries` times.
        `retries` can be negative to retry forever.
        You can specify the types of supported errors which trigger
        a retry with the `supported_errors` parameter.
        """
        self._backoff = backoff
        self._retries = retries
        self._supported_errors = supported_errors

    def update_supported_errors(
        self, specified_errors: Iterable[Type[Exception]]
    ) -> None:
        """
        Updates the supported errors with the specified error types
        """
        self._supported_errors = tuple(
            set(self._supported_errors + tuple(specified_errors))
        )
Repository: redis/redis-py
File: redis/utils.py (Python)

Code before:
import logging
from contextlib import contextmanager
from functools import wraps
from typing import Any, Dict, Mapping, Union

try:
    import hiredis  # noqa

    # Only support Hiredis >= 3.0:
    HIREDIS_AVAILABLE = int(hiredis.__version__.split(".")[0]) >= 3
    if not HIREDIS_AVAILABLE:
        raise ImportError("hiredis package should be >= 3.0.0")
    HIREDIS_PACK_AVAILABLE = hasattr(hiredis, "pack_command")
    if not HIREDIS_PACK_AVAILABLE:
        raise ImportError(
            "pack_command is not available in the current version of hiredis."
        )
except ImportError:
    HIREDIS_AVAILABLE = False
    HIREDIS_PACK_AVAILABLE = False

try:
    import ssl  # noqa

    SSL_AVAILABLE = True
except ImportError:
    SSL_AVAILABLE = False

try:
    import cryptography  # noqa

    CRYPTOGRAPHY_AVAILABLE = True
except ImportError:
    CRYPTOGRAPHY_AVAILABLE = False

from importlib import metadata



Review comment (suggestion):
Can it actually happen that `hiredis-py` doesn't have the `pack` command if the  version is  >=3? If not, let's remove the redundant check.

Code after:
import logging
from contextlib import contextmanager
from functools import wraps
from typing import Any, Dict, Mapping, Union

try:
    import hiredis  # noqa

    # Only support Hiredis >= 3.0:
    HIREDIS_AVAILABLE = int(hiredis.__version__.split(".")[0]) >= 3
    if not HIREDIS_AVAILABLE:
        raise ImportError("hiredis package should be >= 3.0.0")
except ImportError:
    HIREDIS_AVAILABLE = False

try:
    import ssl  # noqa

    SSL_AVAILABLE = True
except ImportError:
    SSL_AVAILABLE = False

try:
    import cryptography  # noqa

    CRYPTOGRAPHY_AVAILABLE = True
except ImportError:
    CRYPTOGRAPHY_AVAILABLE = False

from importlib import metadata


def from_url(url, **kwargs):
    """
    Returns an active Redis client generated from the given database URL.

    Will attempt to extract the database id from the path url fragment, if
    none is provided.
Repository: redis/redis-py
File: redis/commands/search/commands.py (Python)

Code before:
import itertools
import time
from typing import Dict, List, Optional, Union

from redis.client import NEVER_DECODE, Pipeline
from redis.utils import deprecated_function

from ..helpers import get_protocol_version
from ._util import to_string
from .aggregation import AggregateRequest, AggregateResult, Cursor
from .document import Document
from .field import Field
from .indexDefinition import IndexDefinition
from .profileInformation import ProfileInformation
from .query import Query
from .result import Result
from .suggestion import SuggestionParser

NUMERIC = "NUMERIC"

CREATE_CMD = "FT.CREATE"
ALTER_CMD = "FT.ALTER"
SEARCH_CMD = "FT.SEARCH"
ADD_CMD = "FT.ADD"
ADDHASH_CMD = "FT.ADDHASH"
DROP_CMD = "FT.DROP"
DROPINDEX_CMD = "FT.DROPINDEX"
EXPLAIN_CMD = "FT.EXPLAIN"
EXPLAINCLI_CMD = "FT.EXPLAINCLI"
DEL_CMD = "FT.DEL"
AGGREGATE_CMD = "FT.AGGREGATE"
PROFILE_CMD = "FT.PROFILE"
CURSOR_CMD = "FT.CURSOR"

Review comment (suggestion):
It seems that `parse_to_dict` is a dead/unused code now. It should be removed if this is the case with the test case for it.

Code after:
import itertools
import time
from typing import Dict, List, Optional, Union

from redis.client import NEVER_DECODE, Pipeline
from redis.utils import deprecated_function

from ..helpers import get_protocol_version
from ._util import to_string
from .aggregation import AggregateRequest, AggregateResult, Cursor
from .document import Document
from .field import Field
from .index_definition import IndexDefinition
from .profile_information import ProfileInformation
from .query import Query
from .result import Result
from .suggestion import SuggestionParser

NUMERIC = "NUMERIC"

CREATE_CMD = "FT.CREATE"
ALTER_CMD = "FT.ALTER"
SEARCH_CMD = "FT.SEARCH"
ADD_CMD = "FT.ADD"
ADDHASH_CMD = "FT.ADDHASH"
DROP_CMD = "FT.DROP"
DROPINDEX_CMD = "FT.DROPINDEX"
EXPLAIN_CMD = "FT.EXPLAIN"
EXPLAINCLI_CMD = "FT.EXPLAINCLI"
DEL_CMD = "FT.DEL"
AGGREGATE_CMD = "FT.AGGREGATE"
PROFILE_CMD = "FT.PROFILE"
CURSOR_CMD = "FT.CURSOR"
Repository: redis/redis-py
File: tests/test_asyncio/test_connection.py (Python)

Code before:
            "Error connecting to unix:///tmp/redis.sock.",
        ),
        (
            UnixDomainSocketConnection(path="unix:///tmp/redis.sock"),
            OSError(12),
            "Error 12 connecting to unix:///tmp/redis.sock.",
        ),
        (
            UnixDomainSocketConnection(path="unix:///tmp/redis.sock"),
            OSError(12, "Some Error"),
            "Error 12 connecting to unix:///tmp/redis.sock. Some Error.",
        ),
    ],
)
async def test_format_error_message(conn, error, expected_message):
    """Test that the _error_message function formats errors correctly"""
    error_message = conn._error_message(error)
    assert error_message == expected_message


async def test_network_connection_failure():
    with pytest.raises(ConnectionError) as e:
        redis = Redis(host="127.0.0.1", port=9999)
        await redis.set("a", "b")
    expected_err = f"Error {ECONNREFUSED} connecting to 127.0.0.1:9999. Connect"
    assert str(e.value).startswith(expected_err)


async def test_unix_socket_connection_failure():
    with pytest.raises(ConnectionError) as e:
        redis = Redis(unix_socket_path="unix:///tmp/a.sock")
        await redis.set("a", "b")
    assert (
        str(e.value)
        == "Error 2 connecting to unix:///tmp/a.sock. No such file or directory."
    )


Review comment (suggestion):
I think we could use `pytest.raises` argument instead of this separate assertion. Same applicable for all error message assertions.

```
with pytest.raises(ConnectionError, match=f"Error {ECONNREFUSED} connecting to 127.0.0.1:9999. Connect")
```

Code after:
            "Error connecting to unix:///tmp/redis.sock.",
        ),
        (
            UnixDomainSocketConnection(path="unix:///tmp/redis.sock"),
            OSError(12),
            "Error 12 connecting to unix:///tmp/redis.sock.",
        ),
        (
            UnixDomainSocketConnection(path="unix:///tmp/redis.sock"),
            OSError(12, "Some Error"),
            "Error 12 connecting to unix:///tmp/redis.sock. Some Error.",
        ),
    ],
)
async def test_format_error_message(conn, error, expected_message):
    """Test that the _error_message function formats errors correctly"""
    error_message = conn._error_message(error)
    assert error_message == expected_message


async def test_network_connection_failure():
    exp_err = rf"^Error {ECONNREFUSED} connecting to 127.0.0.1:9999.(.+)$"
    with pytest.raises(ConnectionError, match=exp_err):
        redis = Redis(host="127.0.0.1", port=9999)
        await redis.set("a", "b")


async def test_unix_socket_connection_failure():
    exp_err = "Error 2 connecting to unix:///tmp/a.sock. No such file or directory."
    with pytest.raises(ConnectionError, match=exp_err):
        redis = Redis(unix_socket_path="unix:///tmp/a.sock")
        await redis.set("a", "b")
Repository: redis/redis-py
File: tests/test_asyncio/test_commands.py (Python)

Code before:
    skip_if_server_version_lt,
    skip_unless_arch_bits,
)

if sys.version_info >= (3, 11, 3):
    from asyncio import timeout as async_timeout
else:
    from async_timeout import timeout as async_timeout

REDIS_6_VERSION = "5.9.0"


@pytest_asyncio.fixture()
async def r_teardown(r: redis.Redis):
    """
    A special fixture which removes the provided names from the database after use
    """
    usernames = []

    def factory(username):
        usernames.append(username)
        return r

    yield factory
    try:
        current_user = await r.client_info()
    except exceptions.NoPermissionError:
        current_user = {}
    if "default" != current_user.get("user"):
        await r.auth("", "default")
    for username in usernames:
        await r.acl_deluser(username)


@pytest_asyncio.fixture()
async def slowlog(r: redis.Redis):
    current_config = await r.config_get()
    old_slower_than_value = current_config["slowlog-log-slower-than"]
    old_max_legnth_value = current_config["slowlog-max-len"]

    await r.config_set("slowlog-log-slower-than", 0)
    await r.config_set("slowlog-max-len", 128)

    yield

    await r.config_set("slowlog-log-slower-than", old_slower_than_value)
    await r.config_set("slowlog-max-len", old_max_legnth_value)


async def redis_server_time(client: redis.Redis):
    seconds, milliseconds = await client.time()

Review comment (suggestion):
It's a bit misleading, variable should be called something like `info` or `client_info`, user is actually `await r.client_info().get('user')`

Code after:
    skip_if_server_version_lt,
    skip_unless_arch_bits,
)

if sys.version_info >= (3, 11, 3):
    from asyncio import timeout as async_timeout
else:
    from async_timeout import timeout as async_timeout

REDIS_6_VERSION = "5.9.0"


@pytest_asyncio.fixture()
async def r_teardown(r: redis.Redis):
    """
    A special fixture which removes the provided names from the database after use
    """
    usernames = []

    def factory(username):
        usernames.append(username)
        return r

    yield factory
    try:
        client_info = await r.client_info()
    except exceptions.NoPermissionError:
        client_info = {}
    if "default" != client_info.get("user", ""):
        await r.auth("", "default")
    for username in usernames:
        await r.acl_deluser(username)


@pytest_asyncio.fixture()
async def slowlog(r: redis.Redis):
    current_config = await r.config_get()
    old_slower_than_value = current_config["slowlog-log-slower-than"]
    old_max_legnth_value = current_config["slowlog-max-len"]

    await r.config_set("slowlog-log-slower-than", 0)
    await r.config_set("slowlog-max-len", 128)

    yield

    await r.config_set("slowlog-log-slower-than", old_slower_than_value)
    await r.config_set("slowlog-max-len", old_max_legnth_value)


async def redis_server_time(client: redis.Redis):
    seconds, milliseconds = await client.time()
Repository: redis/redis-py
File: setup.py (Python)

Code before:
#!/usr/bin/env python
from setuptools import find_packages, setup

setup(
    name="redis",
    description="Python client for Redis database and key-value store",
    long_description=open("README.md").read().strip(),
    long_description_content_type="text/markdown",
    keywords=["Redis", "key-value store", "database"],
    license="MIT",
    version="5.3.0",
    packages=find_packages(
        include=[
            "redis",
            "redis._parsers",
            "redis.asyncio",
            "redis.auth",
            "redis.commands",
            "redis.commands.bf",
            "redis.commands.json",
            "redis.commands.search",
            "redis.commands.timeseries",
            "redis.commands.graph",
            "redis.parsers",
        ]
    ),
    package_data={"redis": ["py.typed"]},
    include_package_data=True,
    url="https://github.com/redis/redis-py",
    project_urls={
        "Documentation": "https://redis.readthedocs.io/en/latest/",
        "Changes": "https://github.com/redis/redis-py/releases",
        "Code": "https://github.com/redis/redis-py",
        "Issue tracker": "https://github.com/redis/redis-py/issues",
    },
    author="Redis Inc.",

Review comment (suggestion):
Version should be 5.2.1 as master always refers to latest stable version. 5.3.0 is in beta so it's only for now in 5.3 version branch

Code after:
#!/usr/bin/env python
from setuptools import find_packages, setup

setup(
    name="redis",
    description="Python client for Redis database and key-value store",
    long_description=open("README.md").read().strip(),
    long_description_content_type="text/markdown",
    keywords=["Redis", "key-value store", "database"],
    license="MIT",
    version="5.2.1",
    packages=find_packages(
        include=[
            "redis",
            "redis._parsers",
            "redis.asyncio",
            "redis.auth",
            "redis.commands",
            "redis.commands.bf",
            "redis.commands.json",
            "redis.commands.search",
            "redis.commands.timeseries",
            "redis.commands.graph",
            "redis.parsers",
        ]
    ),
    package_data={"redis": ["py.typed"]},
    include_package_data=True,
    url="https://github.com/redis/redis-py",
    project_urls={
        "Documentation": "https://redis.readthedocs.io/en/latest/",
        "Changes": "https://github.com/redis/redis-py/releases",
        "Code": "https://github.com/redis/redis-py",
        "Issue tracker": "https://github.com/redis/redis-py/issues",
    },
    author="Redis Inc.",
Repository: redis/redis-py
File: redis/utils.py (Python)

Code before:
        for _ in range(diff):
            num_versions2.append(0)
    elif len(num_versions1) < len(num_versions2):
        diff = len(num_versions2) - len(num_versions1)
        for _ in range(diff):
            num_versions1.append(0)

    for i, ver in enumerate(num_versions1):
        if num_versions1[i] > num_versions2[i]:
            return -1
        elif num_versions1[i] < num_versions2[i]:
            return 1

    return 0


def ensure_string(key):
    if isinstance(key, bytes):
        return key.decode("utf-8")
    elif isinstance(key, str):
        return key
    else:
        raise TypeError("Key must be either a string or bytes")


def truncate_command_for_exception(self, command, max_length=100):
    if len(command) > max_length:
        return command[: max_length - 3] + "..."
    return command


Review comment (suggestion):
This function can be used for all types of text truncation and would be better if its name is changed to something like truncate_txt, for example, and also fix the names of the used parameters. 

Code after:
    num_versions1 = list(map(int, version1.split(".")))
    num_versions2 = list(map(int, version2.split(".")))

    if len(num_versions1) > len(num_versions2):
        diff = len(num_versions1) - len(num_versions2)
        for _ in range(diff):
            num_versions2.append(0)
    elif len(num_versions1) < len(num_versions2):
        diff = len(num_versions2) - len(num_versions1)
        for _ in range(diff):
            num_versions1.append(0)

    for i, ver in enumerate(num_versions1):
        if num_versions1[i] > num_versions2[i]:
            return -1
        elif num_versions1[i] < num_versions2[i]:
            return 1

    return 0


def ensure_string(key):
    if isinstance(key, bytes):
        return key.decode("utf-8")
    elif isinstance(key, str):
        return key
    else:
        raise TypeError("Key must be either a string or bytes")


def extract_expire_flags(
    ex: Optional[ExpiryT] = None,
    px: Optional[ExpiryT] = None,
    exat: Optional[AbsExpiryT] = None,
    pxat: Optional[AbsExpiryT] = None,
) -> List[EncodableT]:
    exp_options: list[EncodableT] = []
    if ex is not None:
        exp_options.append("EX")
        if isinstance(ex, datetime.timedelta):
            exp_options.append(int(ex.total_seconds()))
        elif isinstance(ex, int):
            exp_options.append(ex)
        elif isinstance(ex, str) and ex.isdigit():
            exp_options.append(int(ex))
        else:
            raise DataError("ex must be datetime.timedelta or int")
    elif px is not None:
        exp_options.append("PX")
        if isinstance(px, datetime.timedelta):
            exp_options.append(int(px.total_seconds() * 1000))