ronantakizawa/codereview-bench huggingface dataset:
Repository: matplotlib/matplotlib
File: lib/matplotlib/text.py (Python)

Code before:
"""
Classes for including text in a figure.
"""

from collections.abc import Sequence
import functools
import logging
import math
import itertools
import weakref
import numpy as np
from numbers import Real
from functools import lru_cache

import matplotlib as mpl
from . import _api, artist, cbook, _docstring, colors as mcolors
from .artist import Artist
from .font_manager import FontProperties
from .patches import FancyArrowPatch, FancyBboxPatch, Rectangle
from .textpath import TextPath, TextToPath  # noqa # Logically located here
from .transforms import (
    Affine2D, Bbox, BboxBase, BboxTransformTo, IdentityTransform, Transform)


_log = logging.getLogger(__name__)


@lru_cache(maxsize=128)
def _rotate(theta):
    """
    Return an Affine2D object that rotates by the given angle in radians.
    """
    return Affine2D().rotate(theta)


def _rotate_point(angle, x, y):
    """
    Rotate point (x, y) by rotation angle in degrees

Review comment (bug):
This re-sorting is incorrect. NumPy is third-party and doesn't belong in the standard library block. Also don't know why `numbers` had to be moved if it wasn't kept sorted against `functools`.

Code after:
"""
Classes for including text in a figure.
"""

from collections.abc import Sequence
import functools
import itertools
import logging
import math
from numbers import Real
import weakref

import numpy as np

import matplotlib as mpl
from . import _api, artist, cbook, _docstring, colors as mcolors
from .artist import Artist
from .font_manager import FontProperties
from .patches import FancyArrowPatch, FancyBboxPatch, Rectangle
from .textpath import TextPath, TextToPath  # noqa # Logically located here
from .transforms import (
    Affine2D, Bbox, BboxBase, BboxTransformTo, IdentityTransform, Transform)


_log = logging.getLogger(__name__)


@functools.lru_cache(maxsize=128)
def _rotate(theta):
    """
    Return an Affine2D object that rotates by the given angle in radians.
    """
    return Affine2D().rotate(theta)


def _rotate_point(angle, x, y):
    """
    Rotate point (x, y) by rotation angle in degrees
Repository: matplotlib/matplotlib
File: lib/matplotlib/bezier.py (Python)

Code before:
"""
A module providing some utility functions regarding Bézier path manipulation.
"""

from functools import lru_cache
from math import comb, factorial
import warnings

import numpy as np

from matplotlib import _api


@lru_cache(maxsize=16)
def _get_coeff_matrix(n):
    """
    Compute the matrix for converting Bezier control points to polynomial
    coefficients for a curve of degree n.

    The matrix M is such that M @ control_points gives polynomial coefficients.
    Entry M[j, i] = C(n, j) * (-1)^(i+j) * C(j, i) where C is the binomial
    coefficient.
    """
    def _comb(n, k):
        return np.vectorize(comb)(n, k)

    j = np.arange(n + 1)[:, None]
    i = np.arange(n + 1)[None, :]  # _comb is non-zero for i <= j
    prefactor = (-1) ** (i + j) * _comb(j, i)  # j on axis 0, i on axis 1
    return (_comb(n, j) * prefactor).astype(float)


Review comment (suggestion):
```suggestion
import math
```
Let's stick with the module level import. Using the qualified names down below increases readability.

Code after:
"""
A module providing some utility functions regarding Bézier path manipulation.
"""

from functools import lru_cache
import math
import warnings

import numpy as np

from matplotlib import _api


@lru_cache(maxsize=16)
def _get_coeff_matrix(n):
    """
    Compute the matrix for converting Bezier control points to polynomial
    coefficients for a curve of degree n.

    The matrix M is such that M @ control_points gives polynomial coefficients.
    Entry M[j, i] = C(n, j) * (-1)^(i+j) * C(j, i) where C is the binomial
    coefficient.
    """
    def _comb(n, k):
        return np.vectorize(math.comb)(n, k)

    j = np.arange(n + 1)[:, None]
    i = np.arange(n + 1)[None, :]  # _comb is non-zero for i <= j
    prefactor = (-1) ** (i + j) * _comb(j, i)  # j on axis 0, i on axis 1
    return (_comb(n, j) * prefactor).astype(float)
Repository: matplotlib/matplotlib
File: lib/matplotlib/tests/test_backend_cairo.py (Python)

Code before:
    ax.set_xlim([-1, 2])
    ax.set_ylim([-1, 2])
    patch = mpatches.PathPatch(cut_star1,
                               linewidth=5, linestyle='dashdot',
                               facecolor=(1, 0, 0, 0.5),
                               edgecolor=(0, 0, 1, 0.75))
    ax.add_patch(patch)
    patch = mpatches.PathPatch(cut_star2,
                               linewidth=5, linestyle='dashdot',
                               facecolor=(1, 0, 0, 0.5),
                               edgecolor=(0, 0, 1, 0.75))
    ax.add_patch(patch)

    # Test: path collection
    ax = fig_test.subplots()
    ax.set_xlim([-1, 2])
    ax.set_ylim([-1, 2])
    col = mcollections.PathCollection([cut_star1, cut_star2],
                                      linewidth=5, linestyles='dashdot',
                                      facecolor=(1, 0, 0, 0.5),
                                      edgecolor=(0, 0, 1, 0.75))
    ax.add_collection(col)

    # TODO: Look into why the figures need to be pyplot figures to match
    plt.figure(fig_ref)
    plt.figure(fig_test)


Review comment (suggestion):
I have no idea why this is necessary.  Rendered figures using the Cairo backend are apparently slightly different depending on whether the figures are pyplot-managed or not.  `fig_ref` and `fig_test` do not match when the figures are not pyplot-managed, but they do match when the figures are pyplot-managed.

For comparison, with the Agg backend instead, `fig_ref` and `fig_test` do not match regardless of whether the figures are pyplot-managed or not.

Code after:
    ax.set_xlim([-1, 2])
    ax.set_ylim([-1, 2])
    patch = mpatches.PathPatch(cut_star1,
                               linewidth=5, linestyle='dashdot',
                               facecolor=(1, 0, 0, 0.5),
                               edgecolor=(0, 0, 1, 0.75))
    ax.add_patch(patch)
    patch = mpatches.PathPatch(cut_star2,
                               linewidth=5, linestyle='dashdot',
                               facecolor=(1, 0, 0, 0.5),
                               edgecolor=(0, 0, 1, 0.75))
    ax.add_patch(patch)

    # Test: path collection
    ax = fig_test.subplots()
    ax.set_xlim([-1, 2])
    ax.set_ylim([-1, 2])
    col = mcollections.PathCollection([cut_star1, cut_star2],
                                      linewidth=5, linestyles='dashdot',
                                      facecolor=(1, 0, 0, 0.5),
                                      edgecolor=(0, 0, 1, 0.75))
    ax.add_collection(col)

    # Have pyplot manage the figures to ensure the cairo backend is used
    plt.figure(fig_ref)
    plt.figure(fig_test)
Repository: matplotlib/matplotlib
File: lib/matplotlib/tests/test_axes.py (Python)

Code before:
    with pytest.raises(ValueError, match='Wedge sizes must be finite numbers'):
        ax.pie(df, labels=['A', 'B', 'C'])


def test_pie_all_zeros():
    fig, ax = plt.subplots()
    with pytest.raises(ValueError, match="All wedge sizes are zero"):
        ax.pie([0, 0], labels=["A", "B"])


def test_animated_artists_not_drawn_by_default():
    fig, (ax1, ax2) = plt.subplots(ncols=2)

    imdata = np.random.random((20, 20))
    lndata = imdata[0]

    im = ax1.imshow(imdata, animated=True)
    (ln,) = ax2.plot(lndata, animated=True)

    with (unittest.mock.patch.object(im, "draw", name="im.draw") as mocked_im_draw,
          unittest.mock.patch.object(ln, "draw", name="ln.draw") as mocked_ln_draw):
        fig.draw_without_rendering()

    mocked_im_draw.assert_not_called()
    mocked_ln_draw.assert_not_called()

def test_errorbar_uses_rcparams():
    with mpl.rc_context({
        "errorbar.capsize": 5.0,
        "errorbar.capthick": 2.5,
        "errorbar.elinewidth": 1.75,
    }):
        fig, ax = plt.subplots()
        eb = ax.errorbar([0, 1, 2], [1, 2, 3], yerr=[0.1, 0.2, 0.3], fmt="none")

    data_line, caplines, barlinecols = eb.lines
    assert data_line is None
    assert caplines 

    assert_allclose([cap.get_markersize() for cap in caplines], 10.0)
    assert_allclose([cap.get_markeredgewidth() for cap in caplines], 2.5)
    for barcol in barlinecols:
        assert_allclose(barcol.get_linewidths(), 1.75)


Review comment (suggestion):
```suggestion


```
Would satisfy linting rule [`E302`](https://docs.astral.sh/ruff/rules/blank-lines-top-level/) from [*ruff*](https://docs.astral.sh/ruff/).

Code after:
    with pytest.raises(ValueError, match='Wedge sizes must be finite numbers'):
        ax.pie(df, labels=['A', 'B', 'C'])


def test_pie_all_zeros():
    fig, ax = plt.subplots()
    with pytest.raises(ValueError, match="All wedge sizes are zero"):
        ax.pie([0, 0], labels=["A", "B"])


def test_animated_artists_not_drawn_by_default():
    fig, (ax1, ax2) = plt.subplots(ncols=2)

    imdata = np.random.random((20, 20))
    lndata = imdata[0]

    im = ax1.imshow(imdata, animated=True)
    (ln,) = ax2.plot(lndata, animated=True)

    with (unittest.mock.patch.object(im, "draw", name="im.draw") as mocked_im_draw,
          unittest.mock.patch.object(ln, "draw", name="ln.draw") as mocked_ln_draw):
        fig.draw_without_rendering()

    mocked_im_draw.assert_not_called()
    mocked_ln_draw.assert_not_called()


def test_errorbar_uses_rcparams():
    with mpl.rc_context({
        "errorbar.capsize": 5.0,
        "errorbar.capthick": 2.5,
        "errorbar.elinewidth": 1.75,
    }):
        fig, ax = plt.subplots()
        eb = ax.errorbar([0, 1, 2], [1, 2, 3], yerr=[0.1, 0.2, 0.3], fmt="none")

    data_line, caplines, barlinecols = eb.lines
    assert data_line is None
    assert caplines

    assert_allclose([cap.get_markersize() for cap in caplines], 10.0)
    assert_allclose([cap.get_markeredgewidth() for cap in caplines], 2.5)
    for barcol in barlinecols:
        assert_allclose(barcol.get_linewidths(), 1.75)
Repository: matplotlib/matplotlib
File: lib/matplotlib/tests/test_axes.py (Python)

Code before:

    imdata = np.random.random((20, 20))
    lndata = imdata[0]

    im = ax1.imshow(imdata, animated=True)
    (ln,) = ax2.plot(lndata, animated=True)

    with (unittest.mock.patch.object(im, "draw", name="im.draw") as mocked_im_draw,
          unittest.mock.patch.object(ln, "draw", name="ln.draw") as mocked_ln_draw):
        fig.draw_without_rendering()

    mocked_im_draw.assert_not_called()
    mocked_ln_draw.assert_not_called()

def test_errorbar_uses_rcparams():
    with mpl.rc_context({
        "errorbar.capsize": 5.0,
        "errorbar.capthick": 2.5,
        "errorbar.elinewidth": 1.75,
    }):
        fig, ax = plt.subplots()
        eb = ax.errorbar([0, 1, 2], [1, 2, 3], yerr=[0.1, 0.2, 0.3], fmt="none")

    data_line, caplines, barlinecols = eb.lines
    assert data_line is None
    assert caplines 

    assert_allclose([cap.get_markersize() for cap in caplines], 10.0)
    assert_allclose([cap.get_markeredgewidth() for cap in caplines], 2.5)
    for barcol in barlinecols:
        assert_allclose(barcol.get_linewidths(), 1.75)


Review comment (suggestion):
```suggestion
    assert caplines
```
Would satisfy linting rule [`W291`](https://docs.astral.sh/ruff/rules/trailing-whitespace/) from [*ruff*](https://docs.astral.sh/ruff/).

Code after:

    imdata = np.random.random((20, 20))
    lndata = imdata[0]

    im = ax1.imshow(imdata, animated=True)
    (ln,) = ax2.plot(lndata, animated=True)

    with (unittest.mock.patch.object(im, "draw", name="im.draw") as mocked_im_draw,
          unittest.mock.patch.object(ln, "draw", name="ln.draw") as mocked_ln_draw):
        fig.draw_without_rendering()

    mocked_im_draw.assert_not_called()
    mocked_ln_draw.assert_not_called()


def test_errorbar_uses_rcparams():
    with mpl.rc_context({
        "errorbar.capsize": 5.0,
        "errorbar.capthick": 2.5,
        "errorbar.elinewidth": 1.75,
    }):
        fig, ax = plt.subplots()
        eb = ax.errorbar([0, 1, 2], [1, 2, 3], yerr=[0.1, 0.2, 0.3], fmt="none")

    data_line, caplines, barlinecols = eb.lines
    assert data_line is None
    assert caplines

    assert_allclose([cap.get_markersize() for cap in caplines], 10.0)
    assert_allclose([cap.get_markeredgewidth() for cap in caplines], 2.5)
    for barcol in barlinecols:
        assert_allclose(barcol.get_linewidths(), 1.75)
Repository: medusajs/medusa
File: packages/admin/dashboard/src/lib/number-helper.ts (TypeScript)

Code before:
/**
 * Gets the number of decimal places in a number
 * @param num - The number for which we are getting the number of decimal places
 * @returns The number of decimal places
 *
 * @example
 * getDecimalPlaces(123.456) // 3
 * getDecimalPlaces(42) // 0
 * getDecimalPlaces(10.0000) // 0
 */
export function getNumberOfDecimalPlaces(num: number | string): number {
  // Convert to string and check if it contains a decimal point
  const str = num.toString()
  if (str.indexOf(".") === -1) {
    return 0
  }
  // Return the length of the part after the decimal point
  return str.split(".")[1].length
}


Review comment (bug):
suggestion(non-blocking): might be better to always expect a number here because if a string is passed formatted like: `12.000,50`, for certain locales this might produce an incorrect result. We could separate normalisation/conversion from this or account for formatting differences.

Code after:
/**
 * Gets the number of decimal places in a number
 * @param num - The number for which we are getting the number of decimal places
 * @returns The number of decimal places
 *
 * @example
 * getDecimalPlaces(123.456) // 3
 * getDecimalPlaces(42) // 0
 * getDecimalPlaces(10.0000) // 0
 */
export function getNumberOfDecimalPlaces(num: number): number {
  // Convert to string and check if it contains a decimal point
  const str = num.toString()
  if (str.indexOf(".") === -1) {
    return 0
  }
  // Return the length of the part after the decimal point
  return str.split(".")[1].length
}
Repository: medusajs/medusa
File: packages/modules/providers/auth-emailpass/src/services/emailpass.ts (TypeScript)

Code before:
  async register(
    userData: AuthenticationInput,
    authIdentityService: AuthIdentityProviderService
  ): Promise<AuthenticationResponse> {
    const { email, password } = userData.body ?? {}

    if (!password || !isString(password)) {
      return {
        success: false,
        error: "Password should be a string",
      }
    }

    if (!email || !isString(email)) {
      return {
        success: false,
        error: "Email should be a string",
      }
    }

    try {
      const identity = await authIdentityService.retrieve({
        entity_id: email,
      })

      if (!identity.app_metadata) {
        const updatedAuthIdentity = await this.upsertAuthIdentity('update', {
          email,
          password,
          authIdentityService,
        })

        return {
          success: true,
          authIdentity: updatedAuthIdentity,
        }
      }

      return {
        success: false,
        error: "Identity with email already exists",
      }
    } catch (error) {
      if (error.type === MedusaError.Types.NOT_FOUND) {
        const createdAuthIdentity = await this.upsertAuthIdentity('create', {
          email,
          password,
          authIdentityService,
        })

        return {

Review comment (security):
suggestion(non-blocking): Should we add a short comment as to what and why we are doing this? The assumption is that when there is no `app_metadata`, the auth identity is still "claimable"

Code after:

  async register(
    userData: AuthenticationInput,
    authIdentityService: AuthIdentityProviderService
  ): Promise<AuthenticationResponse> {
    const { email, password } = userData.body ?? {}

    if (!password || !isString(password)) {
      return {
        success: false,
        error: "Password should be a string",
      }
    }

    if (!email || !isString(email)) {
      return {
        success: false,
        error: "Email should be a string",
      }
    }

    try {
      const identity = await authIdentityService.retrieve({
        entity_id: email,
      })

      // If app_metadata is not defined or empty, it means no actor was assigned to the auth_identity yet (still "claimable")
      if (!isPresent(identity.app_metadata)) {
        const updatedAuthIdentity = await this.upsertAuthIdentity('update', {
          email,
          password,
          authIdentityService,
        })

        return {
          success: true,
          authIdentity: updatedAuthIdentity,
        }
      }

      return {
        success: false,
        error: "Identity with email already exists",
      }
    } catch (error) {
      if (error.type === MedusaError.Types.NOT_FOUND) {
        const createdAuthIdentity = await this.upsertAuthIdentity('create', {
          email,
          password,
          authIdentityService,
        })
Repository: medusajs/medusa
File: packages/modules/payment/src/migrations/Migration20250924135437.ts (TypeScript)

Code before:
import { Migration } from "@mikro-orm/migrations"
import { ulid } from "ulid"

export class Migration20250924135437 extends Migration {
  override async up(): Promise<void> {
    const existingReasonIds = await this.execute(`
      SELECT id FROM "refund_reason"
    `)

    if (existingReasonIds.length === 0) {
      // 2. Create default shipping option type
      await this.execute(`
        INSERT INTO "refund_reason" (id, label, description)
        VALUES 
          ('refr_${ulid()}', 'Shipping Issue', 'Refund due to lost, delayed, or misdelivered shipment'),
          ('refr_${ulid()}', 'Customer Care Adjustment', 'Refund given as goodwill or compensation for inconvenience'),
          ('refr_${ulid()}', 'Pricing Error', 'Refund to correct an overcharge, missing discount, or incorrect price');
      `)
    }
  }

  override async down(): Promise<void> {
    // Remove the default refund reasons we created
    this.addSql(`
      DELETE FROM "refund_reason"
      WHERE ("label", "description") IN (
        ('Shipping Issue', 'Refund due to lost, delayed, or misdelivered shipment'),
        ('Customer Care Adjustment', 'Refund given as goodwill or compensation for inconvenience'),
        ('Pricing Error', 'Refund to correct an overcharge, missing discount, or incorrect price')
      )
    `)
  }

Review comment (suggestion):
nit(non-blocking): don't think we need to fetch all
```suggestion
      SELECT 1 FROM "refund_reason"
```

Code after:
import { Migration } from "@mikro-orm/migrations"
import { ulid } from "ulid"

export class Migration20250924135437 extends Migration {
  override async up(): Promise<void> {
    const [existingReason] = await this.execute(`
      SELECT 1 as exists
      FROM "refund_reason"
      LIMIT 1
    `)

    if (!existingReason) {
      // 2. Create default shipping option type
      await this.execute(`
        INSERT INTO "refund_reason" (id, label, description)
        VALUES 
          ('refr_${ulid()}', 'Shipping Issue', 'Refund due to lost, delayed, or misdelivered shipment'),
          ('refr_${ulid()}', 'Customer Care Adjustment', 'Refund given as goodwill or compensation for inconvenience'),
          ('refr_${ulid()}', 'Pricing Error', 'Refund to correct an overcharge, missing discount, or incorrect price');
      `)
    }
  }

  override async down(): Promise<void> {
    // Remove the default refund reasons we created
    this.addSql(`
      DELETE FROM "refund_reason"
      WHERE ("label", "description") IN (
        ('Shipping Issue', 'Refund due to lost, delayed, or misdelivered shipment'),
        ('Customer Care Adjustment', 'Refund given as goodwill or compensation for inconvenience'),
        ('Pricing Error', 'Refund to correct an overcharge, missing discount, or incorrect price')
      )
Repository: medusajs/medusa
File: packages/admin/dashboard/src/components/inputs/combobox/combobox.tsx (TypeScript)

Code before:
      onChange(newValues)
    }

    setUncontrolledSearchValue("")
  }

  const handleSearchChange = (query: string) => {
    setUncontrolledSearchValue(query)

    if (onSearchValueChange) {
      onSearchValueChange(query)
    }
  }

  /**
   * Filter and sort the options based on the search value,
   * and whether the value is already selected.
   *
   * This is only used when the search value is uncontrolled.
   */
  const matches = useMemo(() => {
    if (isSearchControlled) {
      return []
    }

    // do not use mather if input is hidden and no search value can be entered
    if (forceHideInput) {
      return options
    }

    return matchSorter(options, defferedSearchValue, {
      keys: ["label"],
    })
  }, [options, defferedSearchValue, isSearchControlled, forceHideInput])

  const observer = useRef(
    new IntersectionObserver(
      (entries) => {
        const first = entries[0]
        if (first.isIntersecting) {
          fetchNextPage?.()
        }
      },
      { threshold: 1 }
    )
  )

  const lastOptionRef = useCallback(
    (node: HTMLDivElement) => {
      if (isFetchingNextPage) {
        return

Review comment (suggestion):
```suggestion
    // do not use `matcher` if the input is hidden
```

Code after:
      onChange(newValues)
    }

    setUncontrolledSearchValue("")
  }

  const handleSearchChange = (query: string) => {
    setUncontrolledSearchValue(query)

    if (onSearchValueChange) {
      onSearchValueChange(query)
    }
  }

  /**
   * Filter and sort the options based on the search value,
   * and whether the value is already selected.
   *
   * This is only used when the search value is uncontrolled.
   */
  const matches = useMemo(() => {
    if (isSearchControlled) {
      return []
    }

    // do not use `matcher` if the input is hidden
    if (forceHideInput) {
      return options
    }

    return matchSorter(options, defferedSearchValue, {
      keys: ["label"],
    })
  }, [options, defferedSearchValue, isSearchControlled, forceHideInput])

  const observer = useRef(
    new IntersectionObserver(
      (entries) => {
        const first = entries[0]
        if (first.isIntersecting) {
          fetchNextPage?.()
        }
      },
      { threshold: 1 }
    )
  )

  const lastOptionRef = useCallback(
    (node: HTMLDivElement) => {
      if (isFetchingNextPage) {
        return