ronantakizawa/codereview-bench huggingface dataset:
Repository: psf/requests
File: tests/test_requests.py (Python)

Code before:
        r = requests.get(httpbin("bytes/20"))
        with pytest.raises(requests.exceptions.JSONDecodeError) as excinfo:
            r.json()
        assert excinfo.value.doc == r.text

    def test_status_code_425(self):
        r1 = requests.codes.get("TOO_EARLY")
        r2 = requests.codes.get("too_early")
        r3 = requests.codes.get("UNORDERED")
        r4 = requests.codes.get("unordered")
        r5 = requests.codes.get("UNORDERED_COLLECTION")
        r6 = requests.codes.get("unordered_collection")

        assert r1 == 425
        assert r2 == 425
        assert r3 == 425
        assert r4 == 425
        assert r5 == 425
        assert r6 == 425


def test_json_decode_errors_are_serializable_deserializable():
    json_decode_error = requests.exceptions.JSONDecodeError(
        "Extra data",
        '{"responseCode":["706"],"data":null}{"responseCode":["706"],"data":null}',
        36,
    )
    deserialized_error = pickle.loads(pickle.dumps(json_decode_error))
    assert repr(json_decode_error) == repr(deserialized_error)


Review comment (suggestion):
There may not be a better way to test this but I don't know if we have other tests that require contacting a live site with TLS disabled. That may have some durability issues and means we're going to take the first response we get back. Probably minor, but figured I'd call it out.

Code after:
        r = requests.get(httpbin("bytes/20"))
        with pytest.raises(requests.exceptions.JSONDecodeError) as excinfo:
            r.json()
        assert excinfo.value.doc == r.text

    def test_status_code_425(self):
        r1 = requests.codes.get("TOO_EARLY")
        r2 = requests.codes.get("too_early")
        r3 = requests.codes.get("UNORDERED")
        r4 = requests.codes.get("unordered")
        r5 = requests.codes.get("UNORDERED_COLLECTION")
        r6 = requests.codes.get("unordered_collection")

        assert r1 == 425
        assert r2 == 425
        assert r3 == 425
        assert r4 == 425
        assert r5 == 425
        assert r6 == 425

    def test_different_connection_pool_for_tls_settings(self):
        s = requests.Session()
        r1 = s.get("https://invalid.badssl.com", verify=False)
        assert r1.status_code == 421
        with pytest.raises(requests.exceptions.SSLError):
            s.get("https://invalid.badssl.com")


def test_json_decode_errors_are_serializable_deserializable():
    json_decode_error = requests.exceptions.JSONDecodeError(
        "Extra data",
        '{"responseCode":["706"],"data":null}{"responseCode":["706"],"data":null}',
        36,
    )
    deserialized_error = pickle.loads(pickle.dumps(json_decode_error))
    assert repr(json_decode_error) == repr(deserialized_error)
Repository: psf/requests
File: src/requests/compat.py (Python)

Code before:
"""
requests.compat
~~~~~~~~~~~~~~~

This module previously handled import compatibility issues
between Python 2 and Python 3. It remains for backwards
compatibility until the next major version.
"""

try:
    import chardet
except ImportError:
    import charset_normalizer as chardet

import sys

# -------
# Pythons
# -------

# Syntax sugar.
_ver = sys.version_info

#: Python 2.x?
is_py2 = _ver[0] == 2

#: Python 3.x?
is_py3 = _ver[0] == 3

# json/simplejson module import resolution
has_simplejson = False
try:
    import simplejson as json

    has_simplejson = True
except ImportError:
    import json

if has_simplejson:
    from simplejson import JSONDecodeError
else:
    from json import JSONDecodeError

# Keep OrderedDict for backwards compatibility.
from collections import OrderedDict
from collections.abc import Callable, Mapping, MutableMapping

Review comment (suggestion):
While this keeps the logic of the earlier code, I think that the library which is required on should be tried first. https://github.com/psf/requests/pull/6714 addresses this.

Code after:
"""
requests.compat
~~~~~~~~~~~~~~~

This module previously handled import compatibility issues
between Python 2 and Python 3. It remains for backwards
compatibility until the next major version.
"""

import importlib
import sys

# -------------------
# Character Detection
# -------------------


def _resolve_char_detection():
    """Find supported character detection libraries."""
    chardet = None
    for lib in ("chardet", "charset_normalizer"):
        if chardet is None:
            try:
                chardet = importlib.import_module(lib)
            except ImportError:
                pass
    return chardet


chardet = _resolve_char_detection()

# -------
# Pythons
# -------

# Syntax sugar.
_ver = sys.version_info

#: Python 2.x?
is_py2 = _ver[0] == 2

#: Python 3.x?
is_py3 = _ver[0] == 3

# json/simplejson module import resolution
has_simplejson = False
Repository: quilljs/quill
File: packages/quill/src/core/quill.ts (TypeScript)

Code before:
  // Special case toolbar shorthand
  if (
    modules != null &&
    modules.toolbar &&
    modules.toolbar.constructor !== Object
  ) {
    modules.toolbar = {
      container: modules.toolbar,
    };
  }

  const config = { ...quillDefaults, ...themeDefaults, ...options };

  let registry = options.registry;
  if (registry) {
    if (options.formats) {
      debug.warn('Ignoring "formats" option because "registry" is specified');
    }
  } else {
    registry = options.formats
      ? createRegistryWithFormats(
          options.formats,
          config.registry,
          (errorMessage) => {
            debug.error(errorMessage);
          },
        )
      : config.registry;
  }

  return {
    ...config,
    registry,
    container,
    theme,
    modules: Object.entries(modules).reduce(
      (modulesWithDefaults, [name, value]) => {
        if (!value) return modulesWithDefaults;

        const moduleClass = Quill.import(`modules/${name}`);
        if (moduleClass == null) {
          debug.error(
            `Cannot load ${name} module. Are you sure you registered it?`,
          );
          return modulesWithDefaults;
        }
        return {
          ...modulesWithDefaults,
          // @ts-expect-error
          [name]: merge({}, moduleClass.DEFAULTS || {}, value),
        };

Review comment (bug):
I'd probably just pass in the `debug` object for ease and flexibility

Code after:
  // Special case toolbar shorthand
  if (
    modules != null &&
    modules.toolbar &&
    modules.toolbar.constructor !== Object
  ) {
    modules.toolbar = {
      container: modules.toolbar,
    };
  }

  const config = { ...quillDefaults, ...themeDefaults, ...options };

  let registry = options.registry;
  if (registry) {
    if (options.formats) {
      debug.warn('Ignoring "formats" option because "registry" is specified');
    }
  } else {
    registry = options.formats
      ? createRegistryWithFormats(options.formats, config.registry, debug)
      : config.registry;
  }

  return {
    ...config,
    registry,
    container,
    theme,
    modules: Object.entries(modules).reduce(
      (modulesWithDefaults, [name, value]) => {
        if (!value) return modulesWithDefaults;

        const moduleClass = Quill.import(`modules/${name}`);
        if (moduleClass == null) {
          debug.error(
            `Cannot load ${name} module. Are you sure you registered it?`,
          );
          return modulesWithDefaults;
        }
        return {
          ...modulesWithDefaults,
          // @ts-expect-error
          [name]: merge({}, moduleClass.DEFAULTS || {}, value),
        };
      },
      {},
    ),
    bounds: resolveSelector(config.bounds),
  };
}
Repository: refinedev/refine
File: src/components/crud/show/showTab/index.tsx (TypeScript)

Code before:
import React, { FC } from "react";
import { Typography, Tabs, TabsProps } from "antd";

import { BaseRecord } from "@interfaces";

type ShowTabProps = TabsProps & {
    record?: BaseRecord;
};

export const ShowTab: FC<ShowTabProps> = ({ children, record, ...rest }) => {
    const renderChildrenWithProps = () => {
        return React.Children.map(children, (child) => {
            if (React.isValidElement(child)) {
                return React.cloneElement(child, {
                    record,
                });
            }
            return child;
        });
    };

    return <Tabs {...rest}>{record && renderChildrenWithProps()}</Tabs>;
};

export { Tab } from "./tab";


Review comment (suggestion):
```suggestion
import { Tabs, TabsProps } from "antd";
```
Typography hiç kullanılmamış

Code after:
import React, { FC } from "react";
import { Tabs, TabsProps } from "antd";

import { BaseRecord } from "@interfaces";

type ShowTabProps = TabsProps & {
    record?: BaseRecord;
};

export const ShowTab: FC<ShowTabProps> = ({ children, record, ...rest }) => {
    const renderChildrenWithProps = () => {
        return React.Children.map(children, (child) => {
            if (React.isValidElement(child)) {
                return React.cloneElement(child, {
                    record,
                });
            }
            return child;
        });
    };

    return <Tabs {...rest}>{record && renderChildrenWithProps()}</Tabs>;
};

export { Tab } from "./tab";
Repository: refinedev/refine
File: examples/app-crm/src/routes/companies/list.tsx (TypeScript)

Code before:
import { FC, PropsWithChildren, useState } from "react";

import { List, useTable } from "@refinedev/antd";
import { HttpError, getDefaultFilter } from "@refinedev/core";
import { GetFieldsFromList } from "@refinedev/nestjs-query";

import {
  AppstoreOutlined,
  SearchOutlined,
  UnorderedListOutlined,
} from "@ant-design/icons";
import { Form, Grid, Input, Radio, Space, Spin } from "antd";
import debounce from "lodash/debounce";

import { ListTitleButton } from "@/components";
import { CompaniesTableQuery } from "@/graphql/types";

import { CompaniesCardView, CompaniesTableView } from "./components";
import { COMPANIES_TABLE_QUERY } from "./queries";

type View = "card" | "table";

export const CompanyListPage: FC<PropsWithChildren> = ({ children }) => {
  const [view, setView] = useState<View>("card");
  const screens = Grid.useBreakpoint();

  const {
    tableProps,
    tableQueryResult,

Review comment (suggestion):
```suggestion
import { getDefaultFilter, HttpError } from "@refinedev/core";
```

Code after:
import { FC, PropsWithChildren, useState } from "react";

import { List, useTable } from "@refinedev/antd";
import { getDefaultFilter, HttpError } from "@refinedev/core";
import { GetFieldsFromList } from "@refinedev/nestjs-query";

import {
  AppstoreOutlined,
  SearchOutlined,
  UnorderedListOutlined,
} from "@ant-design/icons";
import { Form, Grid, Input, Radio, Space, Spin } from "antd";
import debounce from "lodash/debounce";

import { ListTitleButton } from "@/components";
import { CompaniesTableQuery } from "@/graphql/types";

import { CompaniesCardView, CompaniesTableView } from "./components";
import { COMPANIES_TABLE_QUERY } from "./queries";

type View = "card" | "table";

export const CompanyListPage: FC<PropsWithChildren> = ({ children }) => {
  const [view, setView] = useState<View>("card");
  const screens = Grid.useBreakpoint();

  const {
    tableProps,
    tableQueryResult,
Repository: refinedev/refine
File: examples/app-crm/src/routes/administration/audit-log.tsx (TypeScript)

Code before:
            defaultFilteredValue={getDefaultFilter("action", filters, "eq")}
          />
          <Table.Column dataIndex="targetEntity" title="Entity" />
          <Table.Column dataIndex="targetId" title="Entity ID" />
          <Table.Column<Audit>
            dataIndex="changes"
            title="Changes"
            render={(_, record) => <ActionCell record={record} />}
          />
          <Table.Column
            dataIndex="createdAt"
            title="Date & Time"
            width="15%"
            render={(value) => (
              <DateField
                style={{ verticalAlign: "middle" }}
                value={value}
                format="MM.DD.YYYY - hh:mm"
              />
            )}
            filterDropdown={(props) => (
              <FilterDropdown
                {...props}
                mapValue={(selectedKeys, event) => {
                  return rangePickerFilterMapper(selectedKeys, event);
                }}
              >
                <DatePicker.RangePicker />
              </FilterDropdown>
            )}
            sorter
            defaultFilteredValue={getDefaultFilter(
              "createdAt",
              filters,
              "between",
            )}
            defaultSortOrder={getDefaultSortOrder("createdAt", sorters)}
          />
        </Table>
      </List>
    </div>
  );
};


Review comment (suggestion):
```suggestion
                mapValue={rangePickerFilterMapper}
```

Code after:
            defaultFilteredValue={getDefaultFilter("action", filters, "eq")}
          />
          <Table.Column dataIndex="targetEntity" title="Entity" />
          <Table.Column dataIndex="targetId" title="Entity ID" />
          <Table.Column<Audit>
            dataIndex="changes"
            title="Changes"
            render={(_, record) => <ActionCell record={record} />}
          />
          <Table.Column
            dataIndex="createdAt"
            title="Date & Time"
            width="15%"
            render={(value) => (
              <DateField
                style={{ verticalAlign: "middle" }}
                value={value}
                format="MM.DD.YYYY - hh:mm"
              />
            )}
            filterDropdown={(props) => (
              <FilterDropdown {...props} mapValue={rangePickerFilterMapper}>
                <DatePicker.RangePicker />
              </FilterDropdown>
            )}
            sorter
            defaultFilteredValue={getDefaultFilter(
              "createdAt",
              filters,
              "between",
            )}
            defaultSortOrder={getDefaultSortOrder("createdAt", sorters)}
          />
        </Table>
      </List>
    </div>
  );
};
Repository: refinedev/refine
File: packages/mui/src/hooks/useDataGrid/index.ts (TypeScript)

Code before:
import {
  BaseRecord,
  CrudFilters,
  HttpError,
  Pagination,
  pickNotDeprecated,
  Prettify,
  UseFormProps,
  useLiveMode,
  useTable as useTableCore,
  useTableProps as useTablePropsCore,
  useTableReturnType as useTableReturnTypeCore,
  useUpdate,
} from "@refinedev/core";
import { useState } from "react";

import type {
  DataGridProps,
  GridFilterModel,
  GridSortModel,
} from "@mui/x-data-grid";

import { darken, useTheme } from "@mui/material/styles";

import differenceWith from "lodash/differenceWith";
import isEqual from "lodash/isEqual";

import {
  transformCrudFiltersToFilterModel,
  transformCrudSortingToSortModel,
  transformFilterModelToCrudFilters,
  transformSortModelToCrudSorting,
} from "@definitions";

Review comment (suggestion):
No need for the `UseFormProps` type here, we can use `UseUpdateProps` but I'll elaborate in a later comment below.

Code after:
import {
  useUpdate,
  useLiveMode,
  pickNotDeprecated,
  useTable as useTableCore,
  type BaseRecord,
  type CrudFilters,
  type HttpError,
  type Pagination,
  type Prettify,
  type UseUpdateProps,
  type useTableProps as useTablePropsCore,
  type useTableReturnType as useTableReturnTypeCore,
  useResourceParams,
} from "@refinedev/core";
import { useState } from "react";

import type {
  DataGridProps,
  GridFilterModel,
  GridSortModel,
} from "@mui/x-data-grid";

import { darken, useTheme } from "@mui/material/styles";

import differenceWith from "lodash/differenceWith";
import isEqual from "lodash/isEqual";

import {
  transformCrudFiltersToFilterModel,
  transformCrudSortingToSortModel,
  transformFilterModelToCrudFilters,
  transformSortModelToCrudSorting,
Repository: refinedev/refine
File: packages/mui/src/hooks/useDataGrid/index.ts (TypeScript)

Code before:
  transformCrudSortingToSortModel,
  transformFilterModelToCrudFilters,
  transformSortModelToCrudSorting,
} from "@definitions";

type DataGridPropsType = Required<
  Pick<
    DataGridProps,
    | "rows"
    | "loading"
    | "rowCount"
    | "sortingMode"
    | "sortModel"
    | "onSortModelChange"
    | "filterMode"
    | "onFilterModelChange"
    | "sx"
    | "disableRowSelectionOnClick"
    | "onStateChange"
    | "paginationMode"
    | "processRowUpdate"
  >
> &
  Pick<
    DataGridProps,
    "paginationModel" | "onPaginationModelChange" | "filterModel"
  >;

export type UseDataGridProps<
  TQueryFnData extends BaseRecord,
  TError extends HttpError,
  TSearchVariables,
  TData,
> = Omit<
  useTablePropsCore<TQueryFnData, TError, TData>,
  "pagination" | "filters"
> & {
  onSearch?: (data: TSearchVariables) => CrudFilters | Promise<CrudFilters>;
  pagination?: Prettify<
    Omit<Pagination, "pageSize"> & {
      /**
       * Initial number of items per page
       * @default 25
       */
      pageSize?: number;
    }
  >;
  filters?: Prettify<
    Omit<
      NonNullable<useTablePropsCore<TQueryFnData, TError, TData>["filters"]>,
      "defaultBehavior"

Review comment (refactor):
```suggestion
  >
> &
  Pick<
    DataGridProps,
    "paginationModel" | "onPaginationModelChange" | "filterModel" | "processRowUpdate"
```

Let's move this here, I don't think this needs to be present for all `useDataGrid` usages. Placing it here will mark it as optional.

I think we can add `editable: boolean` prop to `useDataGrid` and make a conditional return for `processRowUpdate`. I'm not sure but sending this even without any editable columns may cause issues with current users implemented editable mode in their codebase.

Code after:
  transformCrudFiltersToFilterModel,
  transformCrudSortingToSortModel,
  transformFilterModelToCrudFilters,
  transformSortModelToCrudSorting,
} from "@definitions";

type DataGridPropsType = Required<
  Pick<
    DataGridProps,
    | "rows"
    | "loading"
    | "rowCount"
    | "sortingMode"
    | "sortModel"
    | "onSortModelChange"
    | "filterMode"
    | "onFilterModelChange"
    | "sx"
    | "disableRowSelectionOnClick"
    | "onStateChange"
    | "paginationMode"
  >
> &
  Pick<
    DataGridProps,
    | "paginationModel"
    | "onPaginationModelChange"
    | "filterModel"
    | "processRowUpdate"
  >;

export type UseDataGridProps<
  TQueryFnData,
  TError extends HttpError,
  TSearchVariables,
  TData extends BaseRecord,
> = Omit<
  useTablePropsCore<TQueryFnData, TError, TData>,
  "pagination" | "filters"
> & {
  onSearch?: (data: TSearchVariables) => CrudFilters | Promise<CrudFilters>;
  pagination?: Prettify<
    Omit<Pagination, "pageSize"> & {
      /**
       * Initial number of items per page
       * @default 25
       */
      pageSize?: number;
    }
  >;
  filters?: Prettify<
Repository: refinedev/refine
File: packages/mui/src/hooks/useDataGrid/index.ts (TypeScript)

Code before:
            return [key, (value as any).type];
          }),
        );
        const isStateChanged = !isEqual(newColumnsTypes, columnsTypes);

        if (isStateChanged) {
          setColumnsType(newColumnsTypes);
        }
      },
      sx: {
        border: "none",
        "& .MuiDataGrid-columnHeaders": {
          background: darken(theme.palette.background.paper, 0.05),
          borderBottom: `1px solid ${darken(
            theme.palette.background.paper,
            0.1,
          )}`,
        },
        "& .MuiDataGrid-cell": {
          borderBottom: `1px solid ${darken(
            theme.palette.background.paper,
            0.05,
          )}`,
        },
      },
      processRowUpdate,
    },
    current,
    setCurrent,
    pageSize,
    setPageSize,
    pageCount,
    sorters,
    setSorters,
    sorter,
    setSorter,
    filters,
    setFilters,
    search,
    createLinkForSyncWithLocation,
    overtime,
    formProps: {
      processRowUpdate,
      formLoading,
    },
  };
}


Review comment (suggestion):
As I mentioned in the above comments, we can return this conditionally by looking for the `editable: boolean` prop. 

Code after:
      filterMode: isServerSideFilteringEnabled ? "server" : "client",
      filterModel: transformCrudFiltersToFilterModel(
        differenceWith(muiCrudFilters, preferredPermanentFilters, isEqual),
        columnsTypes,
      ),
      onFilterModelChange: handleFilterModelChange,
      onStateChange: (state) => {
        const newColumnsTypes = Object.fromEntries(
          Object.entries(state.columns.lookup).map(([key, value]) => {
            return [key, (value as any).type];
          }),
        );
        const isStateChanged = !isEqual(newColumnsTypes, columnsTypes);

        if (isStateChanged) {
          setColumnsType(newColumnsTypes);
        }
      },
      sx: {
        border: "none",
        "& .MuiDataGrid-columnHeaders": {
          background: darken(theme.palette.background.paper, 0.05),
          borderBottom: `1px solid ${darken(
            theme.palette.background.paper,
            0.1,
          )}`,
        },
        "& .MuiDataGrid-cell": {
          borderBottom: `1px solid ${darken(
            theme.palette.background.paper,
            0.05,
          )}`,
        },
      },
      processRowUpdate: editable ? processRowUpdate : undefined,
    },
    current,
    setCurrent,
    pageSize,
    setPageSize,
    pageCount,
    sorters,
    setSorters,
    sorter,
    setSorter,
    filters,
    setFilters,
    search,
    createLinkForSyncWithLocation,
    overtime,
  };
Repository: refinedev/refine
File: packages/mui/src/hooks/useDataGrid/index.ts (TypeScript)

Code before:

  const dataGridPaginationValues = (): Pick<
    DataGridProps,
    "paginationModel" | "onPaginationModelChange"
  > &
    Required<Pick<DataGridProps, "paginationMode">> => {
    if (isPaginationEnabled) {
      return {
        paginationMode: "server" as const,
        paginationModel: {
          page: current - 1,
          pageSize,
        },
        onPaginationModelChange: (model) => {
          handlePageChange(model.page);
          handlePageSizeChange(model.pageSize);
        },
      };
    }

    return {
      paginationMode: "client" as const,
    };
  };

  const { mutate, isLoading: formLoading } = useUpdate();

  const processRowUpdate = async (newRow: TData, oldRow: TData) => {
    if (!editable) {
      return Promise.resolve(oldRow);
    }

    return new Promise((resolve, reject) => {
      mutate(
        {
          resource: resourceFromProp as string,
          id: newRow.id as string,
          values: newRow,
        },
        {
          onError: (error) => {
            reject(error);
          },
          onSuccess: (data) => {
            resolve(newRow);
          },
        },
      );
    });
  };


Review comment (suggestion):
We can also use generics here just like I've mentioned above for the `updateMutationOptions` 🤔 

```suggestion
  const { mutate, isLoading: formLoading } = useUpdate<TData, TError, TData>();
```

Code after:

  const search = async (value: TSearchVariables) => {
    if (onSearchProp) {
      const searchFilters = await onSearchProp(value);
      setMuiCrudFilters(searchFilters);
      setFilters(searchFilters.filter((f) => f.value !== ""));
      if (isPaginationEnabled) {
        setCurrent(1);
      }
    }
  };

  const dataGridPaginationValues = (): Pick<
    DataGridProps,
    "paginationModel" | "onPaginationModelChange"
  > &
    Required<Pick<DataGridProps, "paginationMode">> => {
    if (isPaginationEnabled) {
      return {
        paginationMode: "server" as const,
        paginationModel: {
          page: current - 1,
          pageSize,
        },
        onPaginationModelChange: (model) => {
          handlePageChange(model.page);
          handlePageSizeChange(model.pageSize);
        },
      };
    }

    return {
      paginationMode: "client" as const,
    };
  };

  const { mutate } = useUpdate<TData, TError, TData>({
    mutationOptions: updateMutationOptions,
  });

  const processRowUpdate = async (newRow: TData, oldRow: TData) => {
    if (!editable) {
      return Promise.resolve(oldRow);
    }

    if (!identifier) {
      return Promise.reject(new Error("Resource is not defined"));
    }

    return new Promise((resolve, reject) => {
      mutate(