ronantakizawa/codereview-bench huggingface dataset:
Repository: scikit-learn/scikit-learn
File: build_tools/update_environments_and_lock_files.py (Python)

Code before:
    # introduced in 7.0.0, see https://github.com/pytest-dev/pytest-cov?tab=readme-ov-file#upgrading-from-pytest-cov-63
    "pytest-cov": "<=6.3.0",
}


def remove_from(alist, to_remove):
    return [each for each in alist if each not in to_remove]


build_metadata_list = [
    {
        "name": "pylatest_conda_forge_cuda_array-api_linux-64",
        "type": "conda",
        "tag": "cuda",
        "folder": "build_tools/github",
        "platform": "linux-64",
        "channels": ["conda-forge"],
        "conda_dependencies": common_dependencies
        + [
            "ccache",
            "pytorch-gpu",
            "polars",
            "pyarrow",
            "cupy",
            "rapidsai::cuvs",
            "array-api-strict",
        ],
    },
    {
        "name": "pylatest_conda_forge_mkl_linux-64",
        "type": "conda",
        "tag": "main-ci",
        "folder": "build_tools/azure",
        "platform": "linux-64",
        "channels": ["conda-forge"],
        "conda_dependencies": common_dependencies
        + [
            "ccache",
            "pytorch",
            "pytorch-cpu",
            "polars",
            "pyarrow",
            "array-api-strict",
            "scipy-doctest",
            "pytest-playwright",
        ],
        "package_constraints": {
            "blas": "[build=mkl]",
        },
    },
    {

Review comment (suggestion):
```suggestion
            "rapidsai::cuvs",  # needed by cupy's cdist function
```

Code after:
    # introduced in 7.0.0, see https://github.com/pytest-dev/pytest-cov?tab=readme-ov-file#upgrading-from-pytest-cov-63
    "pytest-cov": "<=6.3.0",
}


def remove_from(alist, to_remove):
    return [each for each in alist if each not in to_remove]


build_metadata_list = [
    {
        "name": "pylatest_conda_forge_cuda_array-api_linux-64",
        "type": "conda",
        "tag": "cuda",
        "folder": "build_tools/github",
        "platform": "linux-64",
        "channels": ["conda-forge"],
        "conda_dependencies": common_dependencies
        + [
            "ccache",
            "pytorch-gpu",
            "polars",
            "pyarrow",
            "cupy",
            # cuvs is needed for cupyx.scipy.spatial.distance.cdist and friends
            "rapidsai::cuvs",
            "array-api-strict",
        ],
        "virtual_package_spec": True,
    },
    {
        "name": "pylatest_conda_forge_mkl_linux-64",
        "type": "conda",
        "tag": "main-ci",
        "folder": "build_tools/azure",
        "platform": "linux-64",
        "channels": ["conda-forge"],
        "conda_dependencies": common_dependencies
        + [
            "ccache",
            "pytorch",
            "pytorch-cpu",
            "polars",
            "pyarrow",
            "array-api-strict",
            "scipy-doctest",
            "pytest-playwright",
        ],
        "package_constraints": {
            "blas": "[build=mkl]",
        },
Repository: scikit-learn/scikit-learn
File: sklearn/inspection/_plot/tests/test_boundary_decision_display.py (Python)

Code before:
        ("decision_function", "contour"),
        ("predict_proba", "contour"),
        ("predict", "contour"),
        ("predict", "contourf"),
    ],
)
def test_multiclass_levels(pyplot, y, response_method, plot_method):
    # non-regression test for issue #32866

    X = np.array([[-1, -1], [-2, -1], [1, 1], [2, 1], [2, 2], [3, 2]])

    clf = LogisticRegression().fit(X, y)

    disp = DecisionBoundaryDisplay.from_estimator(
        clf,
        X,
        response_method=response_method,
        plot_method=plot_method,
    )

    if plot_method == "contour":
        expected_levels = np.arange(6)
    else:
        expected_levels = np.arange(7) - 0.5

    assert np.array_equal(disp.surface_.levels, expected_levels)


# estimator classes for non-regression test cases for issue #33194
class CustomBinaryEstimator(BaseEstimator):
    def fit(self, X, y):
        self.fitted_ = True
        return self

    def predict(self, X):
        return np.arange(X.shape[0]) % 2


class CustomMulticlassEstimator(BaseEstimator):
    def fit(self, X, y):
        self.fitted_ = True
        return self

    def predict(self, X):
        return np.arange(X.shape[0]) % 7


class CustomContinuousEstimator(BaseEstimator):
    def fit(self, X, y):
        self.fitted_ = True
        return self

Review comment (bug):
Better use `assert_allclose` to compare arrays with floating point values with some tolerance for rounding errors and a more informative error message in case of failure.


```suggestion
    assert_allclose(disp.surface_.levels, expected_levels)
```

Code after:
        ("decision_function", "contour"),
        ("predict_proba", "contour"),
        ("predict", "contour"),
        ("predict", "contourf"),
    ],
)
def test_multiclass_levels(pyplot, y, response_method, plot_method):
    # non-regression test for issue #32866

    X = np.array([[-1, -1], [-2, -1], [1, 1], [2, 1], [2, 2], [3, 2]])

    clf = LogisticRegression().fit(X, y)

    disp = DecisionBoundaryDisplay.from_estimator(
        clf,
        X,
        response_method=response_method,
        plot_method=plot_method,
    )

    if plot_method == "contour":
        expected_levels = np.arange(6)
    else:
        expected_levels = np.arange(7) - 0.5

    assert_allclose(disp.surface_.levels, expected_levels)


# estimator classes for non-regression test cases for issue #33194
class CustomBinaryEstimator(BaseEstimator):
    def fit(self, X, y):
        self.fitted_ = True
        return self

    def predict(self, X):
        return np.arange(X.shape[0]) % 2


class CustomMulticlassEstimator(BaseEstimator):
    def fit(self, X, y):
        self.fitted_ = True
        return self

    def predict(self, X):
        return np.arange(X.shape[0]) % 7


class CustomContinuousEstimator(BaseEstimator):
    def fit(self, X, y):
        self.fitted_ = True
        return self
Repository: scikit-learn/scikit-learn
File: sklearn/manifold/tests/test_mds.py (Python)

Code before:

    # providing both metric and dissimilarity raises an error
    with pytest.raises(ValueError, match="provided both `dissimilarity`"):
        mds.MDS(
            metric="cosine", dissimilarity="euclidean", init="random", n_init=1
        ).fit(X)


# TODO(1.9): remove warning filter
@pytest.mark.filterwarnings("ignore::FutureWarning")
def test_classical_mds_init_to_mds():
    X, _ = load_iris(return_X_y=True)

    cmds = ClassicalMDS()
    Z_classical = cmds.fit_transform(X)

    mds1 = mds.MDS(init="classical_mds")
    Z1 = mds1.fit_transform(X)

    mds2 = mds.MDS(init="random")
    Z2 = mds1.fit_transform(X, init=Z_classical)

    assert_allclose(Z1, Z2)


def test_classical_mds_init_correct_components():
    X, _ = load_iris(return_X_y=True)

    mds1 = mds.MDS(init="classical_mds", n_components=3, n_init=1)
    Z1 = mds1.fit_transform(X)

    assert Z1.shape[1] == 3


Review comment (suggestion):
I would suggest to add `@pytest.mark.parametrize("init", ["random", "classical_mds"])` `@pytest.mark.parametrize("n_components", [1, 2, 5, 10])` or something like that.

Code after:

    # providing both metric and dissimilarity raises an error
    with pytest.raises(ValueError, match="provided both `dissimilarity`"):
        mds.MDS(
            metric="cosine", dissimilarity="euclidean", init="random", n_init=1
        ).fit(X)


# TODO(1.9): remove warning filter
@pytest.mark.filterwarnings("ignore::FutureWarning")
def test_classical_mds_init_to_mds():
    X, _ = load_iris(return_X_y=True)

    cmds = ClassicalMDS()
    Z_classical = cmds.fit_transform(X)

    mds1 = mds.MDS(init="classical_mds")
    Z1 = mds1.fit_transform(X)

    mds2 = mds.MDS(init="random")
    Z2 = mds1.fit_transform(X, init=Z_classical)

    assert_allclose(Z1, Z2)


@pytest.mark.parametrize("init", ["random", "classical_mds"])
@pytest.mark.parametrize("n_components", [1, 2, 5, 10])
def test_correct_n_components(init, n_components):
    X, _ = make_blobs(n_features=10)

    model = mds.MDS(init=init, n_components=n_components, n_init=1)
    Z = model.fit_transform(X)

    assert Z.shape[1] == n_components
Repository: scikit-learn/scikit-learn
File: sklearn/callback/_callback_support.py (Python)

Code before:
# Authors: The scikit-learn developers
# SPDX-License-Identifier: BSD-3-Clause

from sklearn.callback._base import Callback
from sklearn.callback._callback_context import CallbackContext


class CallbackSupportMixin:
    """Mixin class to add callback support to an estimator."""

    def set_callbacks(self, callbacks):
        """Set callbacks for the estimator.

        Parameters
        ----------
        callbacks : callback or list of callbacks
            The callbacks to set.

        Returns
        -------
        self : estimator instance
            The estimator instance itself.
        """
        if not isinstance(callbacks, list):
            callbacks = [callbacks]

        if not all(isinstance(callback, Callback) for callback in callbacks):
            raise TypeError("callbacks must follow the Callback protocol.")

        self._skl_callbacks = callbacks

        return self

    def _init_callback_context(self, task_name="fit", task_id=0, max_subtasks=0):
        """Initialize the callback context for the estimator.

        This method should be called once, at the beginning of the fit method.


Review comment (suggestion):
Just curious, do you have a use case in mind where `get_callback_manager` would be used in a multi-threaded context?

TIL about the [double-checked locking](https://en.wikipedia.org/wiki/Double-checked_locking) pattern ...

Code after:
# Authors: The scikit-learn developers
# SPDX-License-Identifier: BSD-3-Clause

from multiprocessing import Manager
from threading import Lock

from sklearn.callback._base import Callback
from sklearn.callback._callback_context import CallbackContext


class _CallbackManagerState:
    manager = None
    lock = Lock()


def get_callback_manager():
    """Return the global multiprocessing manager dedicated to callbacks.

    The manager is initialized lazily on first access and reused afterwards.
    """
    if _CallbackManagerState.manager is None:
        with _CallbackManagerState.lock:
            if _CallbackManagerState.manager is None:
                _CallbackManagerState.manager = Manager()

    return _CallbackManagerState.manager


class CallbackSupportMixin:
    """Mixin class to add callback support to an estimator."""

    def set_callbacks(self, callbacks):
        """Set callbacks for the estimator.

        Parameters
        ----------
        callbacks : callback or list of callbacks
            The callbacks to set.
Repository: scipy/scipy
File: scipy/sparse/linalg/tests/test_interface.py (Python)

Code before:
    assert_equal(A.ndim, 2)

def test_transpose_noconjugate():
    X = np.array([[1j]])
    A = interface.aslinearoperator(X)

    B = 1j * A
    Y = 1j * X

    v = np.array([1])

    assert_equal(B.dot(v), Y.dot(v))
    assert_equal(B.T.dot(v), Y.T.dot(v))

def test_sparse_matmat_exception():
    A = interface.LinearOperator((2, 2), matvec=lambda x: x)
    B = sparse.identity(2)
    msg = "Unable to multiply a LinearOperator with a sparse matrix."
    with assert_raises(TypeError, match=msg):
        A * B
    with assert_raises(TypeError, match=msg):
        B * A
    with assert_raises(ValueError):
        A * np.identity(4)
    with assert_raises(ValueError):
        np.identity(4) * A


Review comment (suggestion):
Might it be better to use ``@`` instead of ``*`` for multiplication here? I know that ``*`` overloads to matmul for LinearOperator and spmatrix datatypes, but my understanding is that ``@`` is preferable for future-proofing code.

Code after:
    assert_equal(A.ndim, 2)

def test_transpose_noconjugate():
    X = np.array([[1j]])
    A = interface.aslinearoperator(X)

    B = 1j * A
    Y = 1j * X

    v = np.array([1])

    assert_equal(B.dot(v), Y.dot(v))
    assert_equal(B.T.dot(v), Y.T.dot(v))

def test_sparse_matmat_exception():
    A = interface.LinearOperator((2, 2), matvec=lambda x: x)
    B = sparse.identity(2)
    msg = "Unable to multiply a LinearOperator with a sparse matrix."
    with assert_raises(TypeError, match=msg):
        A @ B
    with assert_raises(TypeError, match=msg):
        B @ A
    with assert_raises(ValueError):
        A @ np.identity(4)
    with assert_raises(ValueError):
        np.identity(4) @ A
Repository: scipy/scipy
File: scipy/optimize/tests/test_trustregion_exact.py (Python)

Code before:
              1.06851340e-01, 7.38151960e-01, 5.44531151e00, 8.81448081e00,
              -8.32946970e-01, -2.64962727e00, -6.29295273e00, 9.82625204e-01,
              2.22934659e01, -5.09020820e-01, 4.09964606e00]]
        )
        J = np.array([
            -2.53298102e-07, 1.76392040e-06, 1.74776130e-06, -4.19479903e-10,
            1.44167498e-05, 1.41703911e-08, 1.76392030e-06, 4.96030153e-06,
            -2.35771675e-07, -1.68844985e-06, 4.29218258e-07, 6.65445159e-07,
            -3.87045830e-08, -3.17236594e-07, -1.21120169e-06, 4.59717313e-08,
            1.67123246e-06, 1.46624675e-08, 4.22723383e-08
        ])

        subproblem_maxiter = 10  # without this, this test hangs.

        # Solve Subproblem
        subprob = IterativeSubproblem(
            x=0,
            fun=lambda x: 0,
            jac=lambda x: J,
            hess=lambda x: H,
            k_easy=0.1,
            k_hard=0.2,
            maxiter=subproblem_maxiter,
        )
        trust_radius = 1
        p, hits_boundary = subprob.solve(trust_radius)


Review comment (suggestion):
I suggest something like 
```suggestion
        subproblem_maxiter = [0, 10]  # without this, this test hangs.

	for maxiter in subproblem_maxiter:
	        # Solve Subproblem
	        subprob = IterativeSubproblem(
	            x=0,
	            fun=lambda x: 0,
	            jac=lambda x: J,
	            hess=lambda x: H,
	            k_easy=0.1,
	            k_hard=0.2,
	            maxiter=maxiter,
	        )
	        trust_radius = 1
	        p, hits_boundary = subprob.solve(trust_radius)
	        
	        assert subprob.niter == maxiter
```
so you can actually test both the default value and a given value (10).

Code after:
              -8.32946970e-01, -2.64962727e00, -6.29295273e00, 9.82625204e-01,
              2.22934659e01, -5.09020820e-01, 4.09964606e00]]
        )
        J = np.array([
            -2.53298102e-07, 1.76392040e-06, 1.74776130e-06, -4.19479903e-10,
            1.44167498e-05, 1.41703911e-08, 1.76392030e-06, 4.96030153e-06,
            -2.35771675e-07, -1.68844985e-06, 4.29218258e-07, 6.65445159e-07,
            -3.87045830e-08, -3.17236594e-07, -1.21120169e-06, 4.59717313e-08,
            1.67123246e-06, 1.46624675e-08, 4.22723383e-08
        ])

        subproblem_maxiter = [None, 10]
        for maxiter in subproblem_maxiter:
            # Solve Subproblem
            subprob = IterativeSubproblem(
                x=0,
                fun=lambda x: 0,
                jac=lambda x: J,
                hess=lambda x: H,
                k_easy=0.1,
                k_hard=0.2,
                maxiter=maxiter,
            )
            trust_radius = 1
            p, hits_boundary = subprob.solve(trust_radius)

            if maxiter is None:
                assert subprob.niter <= IterativeSubproblem.MAXITER_DEFAULT
            else:
                assert subprob.niter <= maxiter
Repository: scipy/scipy
File: scipy/integrate/_quadrature.py (Python)

Code before:
import numpy as np
import math
import warnings
from collections import namedtuple
from collections.abc import Callable

from scipy.special import roots_legendre
from scipy.special import gammaln, logsumexp
from scipy._lib._util import _rng_spawn
from scipy._lib._array_api import (_asarray, array_namespace, xp_result_type, xp_copy,
                                   xp_capabilities, xp_promote, xp_swapaxes, is_numpy,
<<<<<<< array-api-simpson
                                   xp_size, is_lazy_array)
import scipy._lib.array_api_extra as xpx
=======
                                   is_lazy_array)
import scipy._external.array_api_extra as xpx
>>>>>>> main


__all__ = ['fixed_quad', 'romb',
           'trapezoid', 'simpson',
           'cumulative_trapezoid', 'newton_cotes',
           'qmc_quad', 'cumulative_simpson']


@xp_capabilities(skip_backends=[('jax.numpy',
                                 "JAX arrays do not support item assignment")])
def trapezoid(y, x=None, dx=1.0, axis=-1):
    r"""
    Integrate along the given axis using the composite trapezoidal rule.

    If `x` is provided, the integration happens in sequence along its
    elements - they are not sorted.

    Integrate `y` (`x`) along each 1d slice on the given axis, compute
    :math:`\int y(x) dx`.
    When `x` is specified, this integrates along the parametric curve,

Review comment (suggestion):
```suggestion
                                   xp_size,
```

Code after:
import numpy as np
import math
import warnings
from collections import namedtuple
from collections.abc import Callable

from scipy.special import roots_legendre
from scipy.special import gammaln, logsumexp
from scipy._lib._util import _rng_spawn
from scipy._lib._array_api import (_asarray, array_namespace, xp_result_type, xp_copy,
                                   xp_capabilities, xp_promote, xp_swapaxes, is_numpy,
                                   xp_size,
                                   is_lazy_array)
import scipy._external.array_api_extra as xpx


__all__ = ['fixed_quad', 'romb',
           'trapezoid', 'simpson',
           'cumulative_trapezoid', 'newton_cotes',
           'qmc_quad', 'cumulative_simpson']


@xp_capabilities(skip_backends=[('jax.numpy',
                                 "JAX arrays do not support item assignment")])
def trapezoid(y, x=None, dx=1.0, axis=-1):
    r"""
    Integrate along the given axis using the composite trapezoidal rule.

    If `x` is provided, the integration happens in sequence along its
    elements - they are not sorted.

    Integrate `y` (`x`) along each 1d slice on the given axis, compute
    :math:`\int y(x) dx`.
    When `x` is specified, this integrates along the parametric curve,
    computing :math:`\int_t y(t) dt =
    \int_t y(t) \left.\frac{dx}{dt}\right|_{x=x(t)} dt`.

    Parameters
Repository: scipy/scipy
File: scipy/special/boost_special_functions.h (C/C++)

Code before:
}


template<typename Real>
Real
f_cdf_wrap(const Real dfn, const Real dfd, const Real x)
{
    if (std::isnan(x) || std::isnan(dfn) || std::isnan(dfd)) {
	return NAN;
    }
    if ((dfn <= 0) || (dfd <= 0) || (x < 0)) {
	sf_error("fdtr", SF_ERROR_DOMAIN, NULL);
	return NAN;
    }
    if (std::isinf(x)) {
	// inf => 1. We've already returned if x < 0, so this can only be +inf.
	return 1.0;
    }
    Real y;
    try {
	y = boost::math::cdf(
                boost::math::fisher_f_distribution<Real, SpecialPolicy>(dfn, dfd), x);
    } catch (...) {
	/* Boost was unable to produce a result. Can happen when one or both
	 * of v1 and v2 is very small and x is very large. e.g.
	 * ncdtr(1e-100, 3, 1.5, 1e100). */
        sf_error("fdtr", SF_ERROR_NO_RESULT, NULL);
        y = NAN;
    }
    if ((y < 0) || (y > 1)) {
	/* Boost can return results far out of bounds when dfd and dfn are both large
	 * and of similar magnitude. Return NAN if the result is out of bounds because
	 * the answer cannot be trusted. */
	sf_error("fdtr", SF_ERROR_NO_RESULT, NULL);
        y = NAN;
    }
    return y;
}

float
f_cdf_float(float dfn, float dfd, float x)
{
    return f_cdf_wrap(dfn, dfd, x);
}

double
f_cdf_double(double dfn, double dfd, double x)
{
    return f_cdf_wrap(dfn, dfd, x);
}


Review comment (suggestion):
```suggestion
	/* Boost was unable to produce a result. */
```


Code after:
}


template<typename Real>
Real
f_cdf_wrap(const Real dfn, const Real dfd, const Real x)
{
    if (std::isnan(x) || std::isnan(dfn) || std::isnan(dfd)) {
	return NAN;
    }
    if ((dfn <= 0) || (dfd <= 0) || (x < 0)) {
	sf_error("fdtr", SF_ERROR_DOMAIN, NULL);
	return NAN;
    }
    if (std::isinf(x)) {
	// inf => 1. We've already returned if x < 0, so this can only be +inf.
	return 1.0;
    }
    Real y;
    try {
	y = boost::math::cdf(
                boost::math::fisher_f_distribution<Real, SpecialPolicy>(dfn, dfd), x);
    } catch (...) {
	/* Boost was unable to produce a result. */
        sf_error("fdtr", SF_ERROR_NO_RESULT, NULL);
        y = NAN;
    }
    if ((y < 0) || (y > 1)) {
	sf_error("fdtr", SF_ERROR_NO_RESULT, NULL);
        y = NAN;
    }
    return y;
}

float
f_cdf_float(float dfn, float dfd, float x)
{
    return f_cdf_wrap(dfn, dfd, x);
}

double
f_cdf_double(double dfn, double dfd, double x)
{
    return f_cdf_wrap(dfn, dfd, x);
}

template<typename Real>
Real
f_ppf_wrap(const Real dfn, const Real dfd, const Real x)
{
    if (std::isnan(x) || std::isnan(dfn) || std::isnan(dfd)) {
Repository: scipy/scipy
File: benchmarks/benchmarks/stats.py (Python)

Code before:

    def time_kurtosis(self, order, size, bias):
        stats.kurtosis(self.x, bias=bias)


class BenchQMCDiscrepancy(Benchmark):
    param_names = ['method']
    params = [
        ["CD", "WD", "MD", "L2-star",]
    ]

    def setup(self, method):
        rng = np.random.default_rng(1234)
        sample = rng.random((1000, 10))
        self.sample = sample

    def time_discrepancy(self, method):
        stats.qmc.discrepancy(self.sample, method=method)


class BenchQMCGeometricDiscrepancy(Benchmark):
    param_names = ['method', 'metric', 'ndims']
    params = [
        ['mindist', 'mst'],
        ['euclidean', 'cityblock', 'chebyshev', 'cosine'],
        [3, 10],
    ]

    def setup(self, method, metric, ndims):
        rng = np.random.default_rng(1234)
        sample = rng.random((1000, ndims))
        self.sample = sample

    def time_geo_discrepancy(self, method, metric, ndims):
        stats.qmc.geometric_discrepancy(self.sample, method=method, metric=metric)

    def peakmem_geo_discrepancy(self, method, metric, ndims):
        stats.qmc.geometric_discrepancy(self.sample, method=method, metric=metric)


class BenchQMCHalton(Benchmark):
    param_names = ['d', 'scramble', 'n', 'workers']
    params = [
        [1, 10],
        [True, False],
        [10, 1_000, 100_000],
        [1, 4]
    ]

    def setup(self, d, scramble, n, workers):
        self.rng = np.random.default_rng(1234)

Review comment (suggestion):
Adding `ndim=2` as suggested.
```suggestion
        [2, 3, 10],
```

Code after:

class BenchMoment(Benchmark):
    params = [
        [1, 2, 3, 8],
        [100, 1000, 10000],
    ]
    param_names = ["order", "size"]

    def setup(self, order, size):
        np.random.random(1234)
        self.x = np.random.random(size)

    def time_moment(self, order, size):
        stats.moment(self.x, order)


class BenchSkewKurtosis(Benchmark):
    params = [
        [1, 2, 3, 8],
        [100, 1000, 10000],
        [False, True]
    ]
    param_names = ["order", "size", "bias"]

    def setup(self, order, size, bias):
        np.random.random(1234)
        self.x = np.random.random(size)

    def time_skew(self, order, size, bias):
        stats.skew(self.x, bias=bias)

    def time_kurtosis(self, order, size, bias):
        stats.kurtosis(self.x, bias=bias)


class BenchQMCDiscrepancy(Benchmark):
    param_names = ['method']
    params = [
        ["CD", "WD", "MD", "L2-star",]
    ]

    def setup(self, method):
        rng = np.random.default_rng(1234)
        sample = rng.random((1000, 10))
        self.sample = sample

    def time_discrepancy(self, method):
        stats.qmc.discrepancy(self.sample, method=method)


class BenchQMCGeometricDiscrepancy(Benchmark):