ronantakizawa/codereview-bench huggingface dataset:
Repository: django/django
File: django/contrib/postgres/search.py (Python)

Code before:
from django.db.models import (
    CharField,
    Expression,
    Field,
    FloatField,
    Func,
    Lookup,
    TextField,
    Value,
)
from django.db.models.expressions import CombinedExpression, register_combinable_fields
from django.db.models.functions import Cast, Coalesce

from .utils import CheckPostgresInstalledMixin


class SearchVectorExact(Lookup):
    lookup_name = "exact"

    def process_rhs(self, qn, connection):
        if not isinstance(self.rhs, (SearchQuery, CombinedSearchQuery)):
            config = getattr(self.lhs, "config", None)
            self.rhs = SearchQuery(self.rhs, config=config)
        rhs, rhs_params = super().process_rhs(qn, connection)
        return rhs, rhs_params

    def as_sql(self, qn, connection):
        lhs, lhs_params = self.process_lhs(qn, connection)
        rhs, rhs_params = self.process_rhs(qn, connection)
        params = (*lhs_params, *rhs_params)
        return "%s @@ %s" % (lhs, rhs), params


class SearchVectorField(CheckPostgresInstalledMixin, Field):
    def db_type(self, connection):
        return "tsvector"


class SearchQueryField(CheckPostgresInstalledMixin, Field):
    def db_type(self, connection):
        return "tsquery"


class _Float4Field(Field):
    def db_type(self, connection):
        return "float4"


class SearchConfig(Expression):
    def __init__(self, config):

Review comment (suggestion):
```suggestion
    """Converts multiple spaces to single and strips from both sides."""
```


Code after:
from django.db.backends.postgresql.psycopg_any import is_psycopg3
from django.db.models import (
    CharField,
    Expression,
    Field,
    FloatField,
    Func,
    Lookup,
    TextField,
    Value,
)
from django.db.models.expressions import CombinedExpression, register_combinable_fields
from django.db.models.functions import Cast, Coalesce
from django.utils.regex_helper import _lazy_re_compile

from .utils import CheckPostgresInstalledMixin

if is_psycopg3:
    from psycopg.adapt import Dumper

    class UTF8Dumper(Dumper):
        def dump(self, obj):
            return bytes(obj, "utf-8")

    def quote_lexeme(value):
        return UTF8Dumper(str).quote(psql_escape(value)).decode()

else:
    from psycopg2.extensions import adapt

    def quote_lexeme(value):
        adapter = adapt(psql_escape(value))
        adapter.encoding = "utf-8"
        return adapter.getquoted().decode()


spec_chars_re = _lazy_re_compile(r"['\0\[\]()|&:*!@<>\\]")
multiple_spaces_re = _lazy_re_compile(r"\s{2,}")


def normalize_spaces(val):
    """Convert multiple spaces to single and strip from both sides."""
    if not (val := val.strip()):
        return None
    return multiple_spaces_re.sub(" ", val)


def psql_escape(query):
    """Replace chars not fit for use in search queries with a single space."""
    query = spec_chars_re.sub(" ", query)
Repository: django/django
File: django/contrib/postgres/search.py (Python)

Code before:
from .utils import CheckPostgresInstalledMixin


class SearchVectorExact(Lookup):
    lookup_name = "exact"

    def process_rhs(self, qn, connection):
        if not isinstance(self.rhs, (SearchQuery, CombinedSearchQuery)):
            config = getattr(self.lhs, "config", None)
            self.rhs = SearchQuery(self.rhs, config=config)
        rhs, rhs_params = super().process_rhs(qn, connection)
        return rhs, rhs_params

    def as_sql(self, qn, connection):
        lhs, lhs_params = self.process_lhs(qn, connection)
        rhs, rhs_params = self.process_rhs(qn, connection)
        params = (*lhs_params, *rhs_params)
        return "%s @@ %s" % (lhs, rhs), params


class SearchVectorField(CheckPostgresInstalledMixin, Field):
    def db_type(self, connection):
        return "tsvector"


class SearchQueryField(CheckPostgresInstalledMixin, Field):
    def db_type(self, connection):
        return "tsquery"


class _Float4Field(Field):
    def db_type(self, connection):
        return "float4"


class SearchConfig(Expression):
    def __init__(self, config):
        super().__init__()
        if not hasattr(config, "resolve_expression"):
            config = Value(config)
        self.config = config

    @classmethod
    def from_parameter(cls, config):
        if config is None or isinstance(config, cls):
            return config
        return cls(config)

    def get_source_expressions(self):
        return [self.config]


Review comment (suggestion):
```suggestion
    return multiple_spaces_re.sub(" ", val.strip()) if val else None
```


Code after:
from django.utils.regex_helper import _lazy_re_compile

from .utils import CheckPostgresInstalledMixin

if is_psycopg3:
    from psycopg.adapt import Dumper

    class UTF8Dumper(Dumper):
        def dump(self, obj):
            return bytes(obj, "utf-8")

    def quote_lexeme(value):
        return UTF8Dumper(str).quote(psql_escape(value)).decode()

else:
    from psycopg2.extensions import adapt

    def quote_lexeme(value):
        adapter = adapt(psql_escape(value))
        adapter.encoding = "utf-8"
        return adapter.getquoted().decode()


spec_chars_re = _lazy_re_compile(r"['\0\[\]()|&:*!@<>\\]")
multiple_spaces_re = _lazy_re_compile(r"\s{2,}")


def normalize_spaces(val):
    """Convert multiple spaces to single and strip from both sides."""
    if not (val := val.strip()):
        return None
    return multiple_spaces_re.sub(" ", val)


def psql_escape(query):
    """Replace chars not fit for use in search queries with a single space."""
    query = spec_chars_re.sub(" ", query)
    return normalize_spaces(query)


class SearchVectorExact(Lookup):
    lookup_name = "exact"

    def process_rhs(self, qn, connection):
        if not isinstance(self.rhs, (SearchQuery, CombinedSearchQuery)):
            config = getattr(self.lhs, "config", None)
            self.rhs = SearchQuery(self.rhs, config=config)
        rhs, rhs_params = super().process_rhs(qn, connection)
        return rhs, rhs_params

    def as_sql(self, qn, connection):
Repository: django/django
File: tests/postgres_tests/test_search.py (Python)

Code before:
"""
Test PostgreSQL full text search.

These tests use dialogue from the 1975 film Monty Python and the Holy Grail.
All text copyright Python (Monty) Pictures. Thanks to sacred-texts.com for the
transcript.
"""

from django.db.models import F, Value

from . import PostgreSQLSimpleTestCase, PostgreSQLTestCase
from .models import Character, Line, LineSavedSearch, Scene

try:
    from django.contrib.postgres.search import (
        SearchConfig,
        SearchHeadline,
        SearchQuery,
        SearchRank,
        SearchVector,
    )
except ImportError:
    pass


class GrailTestData:
    @classmethod
    def setUpTestData(cls):
        cls.robin = Scene.objects.create(
            scene="Scene 10", setting="The dark forest of Ewing"
        )

Review comment (suggestion):
In this field I would remove empty lines and use docstring instead of comments to put valuable information.

Code after:
"""
Test PostgreSQL full text search.

These tests use dialogue from the 1975 film Monty Python and the Holy Grail.
All text copyright Python (Monty) Pictures. Thanks to sacred-texts.com for the
transcript.
"""

from django.db import connection
from django.db.models import F, Value

from . import PostgreSQLSimpleTestCase, PostgreSQLTestCase
from .models import Character, Line, LineSavedSearch, Scene

try:
    from django.contrib.postgres.search import (
        Lexeme,
        SearchConfig,
        SearchHeadline,
        SearchQuery,
        SearchRank,
        SearchVector,
        quote_lexeme,
    )
except ImportError:
    pass


class GrailTestData:
    @classmethod
    def setUpTestData(cls):
Repository: django/django
File: django/contrib/postgres/search.py (Python)

Code before:
        if not isinstance(value, str):
            raise TypeError(
                f"Lexeme value must be a string, got {value.__class__.__name__}."
            )

        if weight is not None and (
            not isinstance(weight, str) or weight.lower() not in {"a", "b", "c", "d"}
        ):
            raise ValueError(
                f"Weight must be one of 'A', 'B', 'C', and 'D', got '{weight}'."
            )

        self.prefix = prefix
        self.invert = invert
        self.weight = weight
        super().__init__(value, output_field=output_field)

    def process_rhs(self, compiler, connection):
        escaped_value = psql_escape(self.value)
        if is_psycopg3:
            from psycopg.adapt import Dumper

            class StringDumper(Dumper):
                def dump(self, obj):
                    return bytes(obj, "utf-8")

            param = StringDumper(str).quote(escaped_value).decode()
        else:
            from psycopg2.extensions import QuotedString

            quoted = QuotedString(escaped_value)
            quoted.prepare(None)
            param = quoted.getquoted().decode()

        label = ""
        if self.prefix:
            label += "*"
        if self.weight:
            label += self.weight

        if label:
            param = f"{param}:{label}"
        if self.invert:
            param = f"!{param}"

        return "%s", (param,)

    def as_sql(self, compiler, connection):
        return self.process_rhs(compiler, connection)

    def __invert__(self):

Review comment (suggestion):
Another opinion is welcome before you make this change, but you might pull this class definition out into the module level.

Code after:

    def __rand__(self, other):
        return self._combine(other, self.BITAND, True)


class Lexeme(LexemeCombinable, Value):
    _output_field = SearchQueryField()

    def __init__(
        self, value, output_field=None, *, invert=False, prefix=False, weight=None
    ):
        if value == "":
            raise ValueError("Lexeme value cannot be empty.")

        if not isinstance(value, str):
            raise TypeError(
                f"Lexeme value must be a string, got {value.__class__.__name__}."
            )

        if weight is not None and (
            not isinstance(weight, str) or weight.lower() not in {"a", "b", "c", "d"}
        ):
            raise ValueError(
                f"Weight must be one of 'A', 'B', 'C', and 'D', got {weight!r}."
            )

        self.prefix = prefix
        self.invert = invert
        self.weight = weight
        super().__init__(value, output_field=output_field)

    def as_sql(self, compiler, connection):
        param = quote_lexeme(self.value)
        label = ""
        if self.prefix:
            label += "*"
        if self.weight:
            label += self.weight

        if label:
            param = f"{param}:{label}"
        if self.invert:
            param = f"!{param}"

        return "%s", (param,)

    def __invert__(self):
        cloned = self.copy()
        cloned.invert = not self.invert
        return cloned
Repository: django/django
File: django/test/runner.py (Python)

Code before:
def parallel_type(value):
    """Parse value passed to the --parallel option."""
    if value == "auto":
        return value
    try:
        return int(value)
    except ValueError:
        raise argparse.ArgumentTypeError(
            f"{value!r} is not an integer or the string 'auto'"
        )


_worker_id = 0


def _init_worker(
    counter,
    initial_settings=None,
    serialized_contents=None,
    process_setup=None,
    process_setup_args=None,
    debug_mode=None,
    used_aliases=None,
):
    """
    Switch to databases dedicated to this worker.

    This helper lives at module-level because of the multiprocessing module's
    requirements.
    """

    global _worker_id

    with counter.get_lock():
        counter.value += 1
        _worker_id = counter.value

    is_spawn_or_forkserver = multiprocessing.get_start_method() in {
        "forkserver",
        "spawn",
    }

    if is_spawn_or_forkserver:
        if process_setup and callable(process_setup):
            if process_setup_args is None:
                process_setup_args = ()
            process_setup(*process_setup_args)
        django.setup()
        setup_test_environment(debug=debug_mode)

    db_aliases = used_aliases if used_aliases is not None else connections

Review comment (suggestion):
Thanks. When running in parallel and multiprocessing method is "spawn", can we skip the call to `run_checks()` in DiscoverRunner? It seems wasteful to do it in both places. (And add a test case asserting against that.)

Code after:
def parallel_type(value):
    """Parse value passed to the --parallel option."""
    if value == "auto":
        return value
    try:
        return int(value)
    except ValueError:
        raise argparse.ArgumentTypeError(
            f"{value!r} is not an integer or the string 'auto'"
        )


_worker_id = 0


def _init_worker(
    counter,
    initial_settings=None,
    serialized_contents=None,
    process_setup=None,
    process_setup_args=None,
    debug_mode=None,
    used_aliases=None,
):
    """
    Switch to databases dedicated to this worker and run system checks.

    This helper lives at module-level because of the multiprocessing module's
    requirements.
    """

    global _worker_id

    with counter.get_lock():
        counter.value += 1
        _worker_id = counter.value

    is_spawn_or_forkserver = multiprocessing.get_start_method() in {
        "forkserver",
        "spawn",
    }

    if is_spawn_or_forkserver:
        if process_setup and callable(process_setup):
            if process_setup_args is None:
                process_setup_args = ()
            process_setup(*process_setup_args)
        django.setup()
        setup_test_environment(debug=debug_mode)
        call_command(
            "check", stdout=io.StringIO(), stderr=io.StringIO(), databases=used_aliases
Repository: django/django
File: tests/admin_views/models.py (Python)

Code before:
    start_date = models.DateTimeField()
    price = models.IntegerField()


class FoodDelivery(models.Model):
    DRIVER_CHOICES = (
        ("bill", "Bill G"),
        ("steve", "Steve J"),
    )
    RESTAURANT_CHOICES = (
        ("indian", "A Taste of India"),
        ("thai", "Thai Pography"),
        ("pizza", "Pizza Mama"),
    )
    reference = models.CharField(max_length=100)
    driver = models.CharField(max_length=100, choices=DRIVER_CHOICES, blank=True)
    restaurant = models.CharField(
        max_length=100, choices=RESTAURANT_CHOICES, blank=True
    )

    class Meta:
        unique_together = (("driver", "restaurant"),)


class CoverLetter(models.Model):
    author = models.CharField(max_length=30, blank=True)
    date_written = models.DateField(null=True, blank=True)

    def __str__(self):
        return self.author


class Paper(models.Model):
    title = models.CharField(max_length=30)
    author = models.CharField(max_length=30, blank=True, null=True)


class ShortMessage(models.Model):
    content = models.CharField(max_length=140)
    timestamp = models.DateTimeField(null=True, blank=True)


class Telegram(models.Model):
    title = models.CharField(max_length=30)
    date_sent = models.DateField(null=True, blank=True)

    def __str__(self):
        return self.title


class Story(models.Model):

Review comment (suggestion):
Can you walk me through why this needed to change? I see the add form responds with a validation error if I revert this, but I wouldn't expect any form values to have changed with your edits.

Code after:
class Answer2(Answer):
    class Meta:
        proxy = True


class Reservation(models.Model):
    start_date = models.DateTimeField()
    price = models.IntegerField()


class FoodDelivery(models.Model):
    DRIVER_CHOICES = (
        ("bill", "Bill G"),
        ("steve", "Steve J"),
    )
    RESTAURANT_CHOICES = (
        ("indian", "A Taste of India"),
        ("thai", "Thai Pography"),
        ("pizza", "Pizza Mama"),
    )
    reference = models.CharField(max_length=100)
    driver = models.CharField(max_length=100, choices=DRIVER_CHOICES, blank=True)
    restaurant = models.CharField(
        max_length=100, choices=RESTAURANT_CHOICES, blank=True
    )

    class Meta:
        unique_together = (("driver", "restaurant"),)


class CoverLetter(models.Model):
    author = models.CharField(max_length=30)
    date_written = models.DateField(null=True, blank=True)

    def __str__(self):
        return self.author


class Paper(models.Model):
    title = models.CharField(max_length=30)
    author = models.CharField(max_length=30, blank=True, null=True)


class ShortMessage(models.Model):
    content = models.CharField(max_length=140)
    timestamp = models.DateTimeField(null=True, blank=True)


class Telegram(models.Model):
    title = models.CharField(max_length=30)
    date_sent = models.DateField(null=True, blank=True)
Repository: django/django
File: django/dispatch/dispatcher.py (Python)

Code before:
NONE_ID = _make_id(None)

# A marker for caching
NO_RECEIVERS = object()


async def _gather(*coros):
    if len(coros) == 0:
        return []

    if len(coros) == 1:
        return [await coros[0]]

    async def run(i, coro):
        results[i] = await coro

    try:
        async with asyncio.TaskGroup() as tg:
            results = [None] * len(coros)
            for i, coro in enumerate(coros):
                tg.create_task(run(i, coro))
        return results
    except BaseExceptionGroup as exception_group:
        if len(exception_group.exceptions) == 1:
            raise exception_group.exceptions[0]
        raise


class Signal:
    """
    Base class for all signals

    Internal attributes:

        receivers
            { receiverkey (id) : weakref(receiver) }
    """

    def __init__(self, use_caching=False):
        """
        Create a new signal.
        """
        self.receivers = []
        self.lock = threading.Lock()
        self.use_caching = use_caching
        # For convenience we create empty caches even if they are not used.
        # A note about caching: if use_caching is defined, then for each
        # distinct sender we cache the receivers that sender has in
        # 'sender_receivers_cache'. The cache is cleaned when .connect() or
        # .disconnect() is called and populated on send().
        self.sender_receivers_cache = weakref.WeakKeyDictionary() if use_caching else {}

Review comment (suggestion):
Can this be simplified to:

```suggestion
async def gather_tasks(*coros):
    async with asyncio.TaskGroup() as tg:
        tasks = [tg.create_task(coro) for coro in coros]
    return [task.result() for task in tasks]
```

unless there's reason for the error handling? (BaseExceptionGroup).

Also, it might be worth moving the function to a `utils.py` file, so that it can easily be reused by other places.

Code after:
NONE_ID = _make_id(None)

# A marker for caching
NO_RECEIVERS = object()


async def _gather(*coros):
    if len(coros) == 0:
        return []

    if len(coros) == 1:
        return [await coros[0]]

    async def run(i, coro):
        results[i] = await coro

    try:
        async with asyncio.TaskGroup() as tg:
            results = [None] * len(coros)
            for i, coro in enumerate(coros):
                tg.create_task(run(i, coro))
        return results
    except BaseExceptionGroup as exception_group:
        if len(exception_group.exceptions) == 1:
            raise exception_group.exceptions[0]
        raise


class Signal:
    """
    Base class for all signals

    Internal attributes:

        receivers:
            [
                (
                    (id(receiver), id(sender)),
                    ref(receiver),
                    ref(sender),
                    is_async,
                )
            ]
        sender_receivers_cache:
            WeakKeyDictionary[sender, list[receiver]]
    """

    def __init__(self, use_caching=False):
        """
        Create a new signal.
        """
Repository: django/django
File: django/contrib/contenttypes/checks.py (Python)

Code before:
from itertools import chain

from django.apps import apps
from django.core.checks import Error


def check_generic_foreign_keys(app_configs, **kwargs):
    from .fields import GenericForeignKey

    if app_configs is None:
        models = apps.get_models()
    else:
        models = chain.from_iterable(
            app_config.get_models() for app_config in app_configs
        )
    errors = []
    fields = (
        obj
        for model in models
        for obj in vars(model).values()
        if isinstance(obj, GenericForeignKey)
    )
    for field in fields:
        errors.extend(field.check())
    return errors


def check_model_name_lengths(app_configs, **kwargs):
    if app_configs is None:
        models = apps.get_models()
    else:
        models = chain.from_iterable(

Review comment (refactor):
PR #17886 attempts to remove this check function, though I haven't managed to get that into a mergeable state yet.

Code after:
from itertools import chain

from django.apps import apps
from django.core.checks import Error


def check_generic_foreign_keys(app_configs, **kwargs):
    from .fields import GenericForeignKeyDescriptor

    if app_configs is None:
        models = apps.get_models()
    else:
        models = chain.from_iterable(
            app_config.get_models() for app_config in app_configs
        )
    errors = []
    descriptors = (
        obj
        for model in models
        for obj in vars(model).values()
        if isinstance(obj, GenericForeignKeyDescriptor)
    )
    for descriptor in descriptors:
        errors.extend(descriptor.field.check())
    return errors


def check_model_name_lengths(app_configs, **kwargs):
    if app_configs is None:
        models = apps.get_models()
    else:
        models = chain.from_iterable(
Repository: django/django
File: django/conf/locale/__init__.py (Python)

Code before:
        "name_local": "Basque",
    },
    "fa": {
        "bidi": True,
        "code": "fa",
        "name": "Persian",
        "name_local": "فارسی",
    },
    "fi": {
        "bidi": False,
        "code": "fi",
        "name": "Finnish",
        "name_local": "suomi",
    },
    "fr": {
        "bidi": False,
        "code": "fr",
        "name": "French",
        "name_local": "français",
    },
    "fr-ca": {
        "bidi": False,
        "code": "fr-ca",
        "name": "Canadian French",
        "name_local": "français canadien",
    },
    "fy": {
        "bidi": False,
        "code": "fy",
        "name": "Frisian",
        "name_local": "frysk",
    },
    "ga": {
        "bidi": False,
        "code": "ga",
        "name": "Irish",
        "name_local": "Gaeilge",
    },
    "gd": {
        "bidi": False,
        "code": "gd",
        "name": "Scottish Gaelic",
        "name_local": "Gàidhlig",
    },
    "gl": {
        "bidi": False,
        "code": "gl",
        "name": "Galician",
        "name_local": "galego",
    },
    "he": {

Review comment (suggestion):
```suggestion
```
This shouldn't be required for this fix, I can remove and both the automated test and manual tests work without it

Code after:
        "name_local": "Basque",
    },
    "fa": {
        "bidi": True,
        "code": "fa",
        "name": "Persian",
        "name_local": "فارسی",
    },
    "fi": {
        "bidi": False,
        "code": "fi",
        "name": "Finnish",
        "name_local": "suomi",
    },
    "fr": {
        "bidi": False,
        "code": "fr",
        "name": "French",
        "name_local": "français",
    },
    "fy": {
        "bidi": False,
        "code": "fy",
        "name": "Frisian",
        "name_local": "frysk",
    },
    "ga": {
        "bidi": False,
        "code": "ga",
        "name": "Irish",
        "name_local": "Gaeilge",
    },
    "gd": {
        "bidi": False,
        "code": "gd",
        "name": "Scottish Gaelic",
        "name_local": "Gàidhlig",
    },
    "gl": {
        "bidi": False,
        "code": "gl",
        "name": "Galician",
        "name_local": "galego",
    },
    "he": {
        "bidi": True,
        "code": "he",
        "name": "Hebrew",
        "name_local": "עברית",
    },
    "hi": {