ronantakizawa/codereview-bench huggingface dataset:
Repository: scikit-learn/scikit-learn
File: sklearn/svm/_classes.py (Python)

Code before:
    For a comparison of the SVC with other classifiers see:
    :ref:`sphx_glr_auto_examples_classification_plot_classification_probability.py`.
    """

    _impl = "c_svc"

    def __init__(
        self,
        *,
        C=1.0,
        kernel="rbf",
        degree=3,
        gamma="scale",
        coef0=0.0,
        shrinking=True,
        probability="deprecated",
        tol=1e-3,
        cache_size=200,
        class_weight=None,
        verbose=False,
        max_iter=-1,
        decision_function_shape="ovr",
        break_ties=False,
        random_state=None,
    ):
        if probability != "deprecated":
            warnings.warn(
                "parameter `probability` will be deprecated in version 1.8, "
                "use `CalibratedClassifierCV(SVC(), ensemble=False)` instead "
                "of `SVC(probability=True)`",
                FutureWarning,
            )
        else:
            probability = False

        super().__init__(
            kernel=kernel,
            degree=degree,
            gamma=gamma,
            coef0=coef0,
            tol=tol,
            C=C,
            nu=0.0,
            shrinking=shrinking,
            probability=probability,
            cache_size=cache_size,
            class_weight=class_weight,
            verbose=verbose,
            max_iter=max_iter,
            decision_function_shape=decision_function_shape,
            break_ties=break_ties,

Review comment (suggestion):
This should be moved to the `fit` method. As per the scikit-learn estimator API, the constructor parameters should always be stored unchanged as attributes. Any validation logic should be deferred to the `fit` method.

Code after:
    For a comparison of the SVC with other classifiers see:
    :ref:`sphx_glr_auto_examples_classification_plot_classification_probability.py`.
    """

    _impl = "c_svc"

    def __init__(
        self,
        *,
        C=1.0,
        kernel="rbf",
        degree=3,
        gamma="scale",
        coef0=0.0,
        shrinking=True,
        probability="deprecated",
        tol=1e-3,
        cache_size=200,
        class_weight=None,
        verbose=False,
        max_iter=-1,
        decision_function_shape="ovr",
        break_ties=False,
        random_state=None,
    ):
        super().__init__(
            kernel=kernel,
            degree=degree,
            gamma=gamma,
            coef0=coef0,
            tol=tol,
            C=C,
            nu=0.0,
            shrinking=shrinking,
            probability=probability,
            cache_size=cache_size,
            class_weight=class_weight,
            verbose=verbose,
            max_iter=max_iter,
            decision_function_shape=decision_function_shape,
            break_ties=break_ties,
            random_state=random_state,
        )


class NuSVC(BaseSVC):
    """Nu-Support Vector Classification.

    Similar to SVC but uses a parameter to control the number of support
    vectors.
Repository: scikit-learn/scikit-learn
File: sklearn/svm/tests/test_sparse.py (Python)

Code before:
    dense_svm = svm.SVC(
        C=1, kernel=lambda x, y: np.dot(x, y.T), probability=True, random_state=0
    )
    pred_dense = dense_svm.fit(X, Y).predict(X)
    assert_array_equal(pred_dense, pred)
    # b.decision_function(X_sp)  # XXX : should be supported


@pytest.mark.parametrize("lil_container", LIL_CONTAINERS)
@pytest.mark.filterwarnings("ignore::FutureWarning")
def test_timeout(lil_container):
    sp = svm.SVC(
        C=1, kernel=lambda x, y: x @ y.T, probability=True, random_state=0, max_iter=1
    )
    warning_msg = (
        r"Solver terminated early \(max_iter=1\).  Consider pre-processing "
        r"your data with StandardScaler or MinMaxScaler."
    )
    with pytest.warns(ConvergenceWarning, match=warning_msg):
        sp.fit(lil_container(X), Y)


# XXX: probability=True is not thread-safe:
# https://github.com/scikit-learn/scikit-learn/issues/31885
@pytest.mark.thread_unsafe
@pytest.mark.filterwarnings("ignore::FutureWarning")
def test_consistent_proba():
    a = svm.SVC(probability=True, max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_1 = a.fit(X, Y).predict_proba(X)
    a = svm.SVC(probability=True, max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_2 = a.fit(X, Y).predict_proba(X)
    assert_allclose(proba_1, proba_2)


Review comment (suggestion):
I think the purpose of this test it to test convergence warning which should not be related to the fact that we enable the builtin Platt scaling or not. Let's directly update the test to:

- use the implicit default value of probability (to avoid raising the warning);
- replace calls to `predict_proba` by calls to `decision_function`.

```suggestion
```



Code after:
    pred = b.predict(X_sp)
    b.predict_proba(X_sp)

    dense_svm = svm.SVC(
        C=1, kernel=lambda x, y: np.dot(x, y.T), probability=True, random_state=0
    )
    pred_dense = dense_svm.fit(X, Y).predict(X)
    assert_array_equal(pred_dense, pred)
    # b.decision_function(X_sp)  # XXX : should be supported


@pytest.mark.parametrize("lil_container", LIL_CONTAINERS)
def test_timeout(lil_container):
    sp = svm.SVC(C=1, kernel=lambda x, y: x @ y.T, random_state=0, max_iter=1)
    warning_msg = (
        r"Solver terminated early \(max_iter=1\).  Consider pre-processing "
        r"your data with StandardScaler or MinMaxScaler."
    )
    with pytest.warns(ConvergenceWarning, match=warning_msg):
        sp.fit(lil_container(X), Y)


def test_consistent_decision_function():
    a = svm.SVC(max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_1 = a.fit(X, Y).decision_function(X)
    a = svm.SVC(max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_2 = a.fit(X, Y).decision_function(X)
    assert_allclose(proba_1, proba_2)
Repository: scikit-learn/scikit-learn
File: sklearn/svm/tests/test_sparse.py (Python)

Code before:

    X_sp = lil_container(X)
    b.fit(X_sp, Y)
    pred = b.predict(X_sp)
    b.predict_proba(X_sp)

    dense_svm = svm.SVC(
        C=1, kernel=lambda x, y: np.dot(x, y.T), probability=True, random_state=0
    )
    pred_dense = dense_svm.fit(X, Y).predict(X)
    assert_array_equal(pred_dense, pred)
    # b.decision_function(X_sp)  # XXX : should be supported


@pytest.mark.parametrize("lil_container", LIL_CONTAINERS)
def test_timeout(lil_container):
    sp = svm.SVC(C=1, kernel=lambda x, y: x @ y.T, random_state=0, max_iter=1)
    warning_msg = (
        r"Solver terminated early \(max_iter=1\).  Consider pre-processing "
        r"your data with StandardScaler or MinMaxScaler."
    )
    with pytest.warns(ConvergenceWarning, match=warning_msg):
        sp.fit(lil_container(X), Y)


@pytest.mark.filterwarnings("ignore::FutureWarning")
def test_consistent_proba():
    a = svm.SVC(probability=True, max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_1 = a.fit(X, Y).predict_proba(X)
    a = svm.SVC(probability=True, max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_2 = a.fit(X, Y).predict_proba(X)
    assert_allclose(proba_1, proba_2)


Review comment (bug):
The deprecation alone is not enough to make this test thread safe. We need to keep the marker to avoid getting a failure when running the test with `pytest-run-parallel` enabled, as we do for the `[free-threaded]` build.


```suggestion
# TODO(1.10): remove this test. 
XXX: probability=True is not thread-safe
@pytest.mark.thread_unsafe
@pytest.mark.filterwarnings("ignore::FutureWarning")
```

Code after:

    X_sp = lil_container(X)
    b.fit(X_sp, Y)
    pred = b.predict(X_sp)
    b.predict_proba(X_sp)

    dense_svm = svm.SVC(
        C=1, kernel=lambda x, y: np.dot(x, y.T), probability=True, random_state=0
    )
    pred_dense = dense_svm.fit(X, Y).predict(X)
    assert_array_equal(pred_dense, pred)
    # b.decision_function(X_sp)  # XXX : should be supported


@pytest.mark.parametrize("lil_container", LIL_CONTAINERS)
def test_timeout(lil_container):
    sp = svm.SVC(C=1, kernel=lambda x, y: x @ y.T, random_state=0, max_iter=1)
    warning_msg = (
        r"Solver terminated early \(max_iter=1\).  Consider pre-processing "
        r"your data with StandardScaler or MinMaxScaler."
    )
    with pytest.warns(ConvergenceWarning, match=warning_msg):
        sp.fit(lil_container(X), Y)


def test_consistent_decision_function():
    a = svm.SVC(max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_1 = a.fit(X, Y).decision_function(X)
    a = svm.SVC(max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_2 = a.fit(X, Y).decision_function(X)
    assert_allclose(proba_1, proba_2)
Repository: scikit-learn/scikit-learn
File: sklearn/svm/tests/test_sparse.py (Python)

Code before:
    X_sp = lil_container(X)
    b.fit(X_sp, Y)
    pred = b.predict(X_sp)
    b.predict_proba(X_sp)

    dense_svm = svm.SVC(
        C=1, kernel=lambda x, y: np.dot(x, y.T), probability=True, random_state=0
    )
    pred_dense = dense_svm.fit(X, Y).predict(X)
    assert_array_equal(pred_dense, pred)
    # b.decision_function(X_sp)  # XXX : should be supported


@pytest.mark.parametrize("lil_container", LIL_CONTAINERS)
def test_timeout(lil_container):
    sp = svm.SVC(C=1, kernel=lambda x, y: x @ y.T, random_state=0, max_iter=1)
    warning_msg = (
        r"Solver terminated early \(max_iter=1\).  Consider pre-processing "
        r"your data with StandardScaler or MinMaxScaler."
    )
    with pytest.warns(ConvergenceWarning, match=warning_msg):
        sp.fit(lil_container(X), Y)


# TODO(1.10): remove this test. 
XXX: probability=True is not thread-safe
@pytest.mark.thread_unsafe
@pytest.mark.filterwarnings("ignore::FutureWarning")
def test_consistent_proba():
    a = svm.SVC(probability=True, max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_1 = a.fit(X, Y).predict_proba(X)
    a = svm.SVC(probability=True, max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_2 = a.fit(X, Y).predict_proba(X)
    assert_allclose(proba_1, proba_2)


Review comment (suggestion):
```suggestion
# XXX: probability=True is not thread-safe
```

Code after:
    X_sp = lil_container(X)
    b.fit(X_sp, Y)
    pred = b.predict(X_sp)
    b.predict_proba(X_sp)

    dense_svm = svm.SVC(
        C=1, kernel=lambda x, y: np.dot(x, y.T), probability=True, random_state=0
    )
    pred_dense = dense_svm.fit(X, Y).predict(X)
    assert_array_equal(pred_dense, pred)
    # b.decision_function(X_sp)  # XXX : should be supported


@pytest.mark.parametrize("lil_container", LIL_CONTAINERS)
def test_timeout(lil_container):
    sp = svm.SVC(C=1, kernel=lambda x, y: x @ y.T, random_state=0, max_iter=1)
    warning_msg = (
        r"Solver terminated early \(max_iter=1\).  Consider pre-processing "
        r"your data with StandardScaler or MinMaxScaler."
    )
    with pytest.warns(ConvergenceWarning, match=warning_msg):
        sp.fit(lil_container(X), Y)


def test_consistent_decision_function():
    a = svm.SVC(max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_1 = a.fit(X, Y).decision_function(X)
    a = svm.SVC(max_iter=1, random_state=0)
    with ignore_warnings(category=ConvergenceWarning):
        proba_2 = a.fit(X, Y).decision_function(X)
    assert_allclose(proba_1, proba_2)
Repository: scikit-learn/scikit-learn
File: examples/ensemble/plot_stack_predictors.py (Python)

Code before:
"""
=================================
Combine predictors using stacking
=================================

.. currentmodule:: sklearn

Stacking is an :ref:`ensemble method <ensemble>`. In this strategy, the
out-of-fold predictions from several base estimators are used to train a
meta-model that combines their outputs at inference time. Unlike
:class:`~sklearn.ensemble.VotingRegressor`, which averages predictions with
fixed (optionally user-specified) weights,
:class:`~sklearn.ensemble.StackingRegressor` learns the combination through its
`final_estimator`.

In this example, we illustrate the use case in which different regressors are
stacked together and a final linear penalized regressor is used to output the
prediction. We compare the performance of each individual regressor with the
stacking strategy. Here, stacking slightly improves the overall performance.

"""

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

# %%
# Generate data
# #############
#
# We use synthetic data generated from a sinusoid plus a linear trend with
# heteroscedastic Gaussian noise. A sudden drop is introduced, as it cannot be
# described by a linear model, but a tree-based model can naturally deal with
# it.

import numpy as np
import pandas as pd

rng = np.random.RandomState(42)
X = rng.uniform(-3, 3, size=500)
trend = 2.4 * X
seasonal = 3.1 * np.sin(3.2 * X)
drop = 10.0 * (X > 2).astype(float)

Review comment (suggestion):

```suggestion
stacked together and a final regularized linear regressor is used to output the
```

Code after:
"""
=================================
Combine predictors using stacking
=================================

.. currentmodule:: sklearn

Stacking is an :ref:`ensemble method <ensemble>`. In this strategy, the
out-of-fold predictions from several base estimators are used to train a
meta-model that combines their outputs at inference time. Unlike
:class:`~sklearn.ensemble.VotingRegressor`, which averages predictions with
fixed (optionally user-specified) weights,
:class:`~sklearn.ensemble.StackingRegressor` learns the combination through its
`final_estimator`.

In this example, we illustrate the use case in which different regressors are
stacked together and a final regularized linear regressor is used to output the
prediction. We compare the performance of each individual regressor with the
stacking strategy. Here, stacking slightly improves the overall performance.

"""

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

# %%
# Generate data
# #############
#
# We use synthetic data generated from a sinusoid plus a linear trend with
# heteroscedastic Gaussian noise. A sudden drop is introduced, as it cannot be
# described by a linear model, but a tree-based model can naturally deal with
# it.

import numpy as np
import pandas as pd

rng = np.random.RandomState(42)
X = rng.uniform(-3, 3, size=500)
trend = 2.4 * X
seasonal = 3.1 * np.sin(3.2 * X)
drop = 10.0 * (X > 2).astype(float)
Repository: scikit-learn/scikit-learn
File: sklearn/tree/tests/test_split.py (Python)

Code before:
    def best_split_naive(self, X, y, w):
        splits = []
        splits = list(self._generate_all_splits(X))
        split_impurities = [
            sum(self.compute_split_impurity(X, y, w, **split)) for split in splits
        ]

        return min(zip(split_impurities, splits), key=itemgetter(0))


def sparsify(X, density):
    X -= 0.5
    th_low = np.quantile(X.ravel(), q=density / 2)
    th_up = np.quantile(X.ravel(), q=1 - density / 2)
    X[(th_low < X) & (X < th_up)] = 0
    return csc_array(X)


def make_simple_dataset(
    n,
    d,
    with_nans,
    is_sparse,
    is_clf,
    n_classes,
    rng: np.random.Generator,
):
    X_dense = rng.random((n, d))
    y = rng.random(n) + X_dense.sum(axis=1)
    w = rng.integers(0, 5, size=n) if rng.uniform() < 0.5 else rng.random(n)

    with_duplicates = rng.integers(2) == 0
    if with_duplicates:
        X_dense = X_dense.round(1 if n < 50 else 2)
    if with_nans:
        for i in range(d):
            step = rng.integers(2, 10)
            X_dense[i::step, i] = np.nan
    if is_sparse:
        density = rng.uniform(0.05, 0.99)
        X = sparsify(X_dense, density)
    else:
        X = X_dense

    if is_clf:
        q = np.linspace(0, 1, num=n_classes + 1)[1:-1]
        y = np.searchsorted(np.quantile(y, q), y)

    return X_dense.astype("float32"), X, y, w



Review comment (suggestion):
I would rather not start introducing type annotations in this PR. If we do it, it should be a project wide decision but it would have a large impact on maintenance.

```suggestion
    rng,
```

Code after:
                    "threshold": th,
                    "missing_left": True,
                }

    def best_split_naive(self, X, y, w):
        splits = list(self._generate_all_splits(X))
        if len(splits) == 0:
            return (np.inf, None)

        split_impurities = [
            self.compute_split_impurity(X, y, w, **split) for split in splits
        ]

        return min(zip(split_impurities, splits), key=itemgetter(0))


def make_simple_dataset(
    n,
    d,
    with_nans,
    is_sparse,
    is_clf,
    n_classes,
    rng,
):
    X_dense = rng.random((n, d))
    y = rng.random(n) + X_dense.sum(axis=1)
    w = rng.integers(0, 5, size=n) if rng.uniform() < 0.5 else rng.random(n)

    with_duplicates = rng.integers(2) == 0
    if with_duplicates:
        X_dense = X_dense.round(1 if n < 50 else 2)
    if with_nans:
        nan_density = rng.uniform(0.05, 0.8)
        mask = rng.random(X_dense.shape) < nan_density
        X_dense[mask] = np.nan
    if is_sparse:
        density = rng.uniform(0.05, 0.99)
        X_dense -= 0.5
        mask = rng.random(X_dense.shape) > density
        X_dense[mask] = 0
        X = csc_array(X_dense)
    else:
        X = X_dense

    if is_clf:
        q = np.linspace(0, 1, num=n_classes + 1)[1:-1]
        y = np.searchsorted(np.quantile(y, q), y)

    # Trees cast X to float32 internally; match that dtype here to avoid
    # routing/impurity mismatches from rounding with `<=`.
Repository: scikit-learn/scikit-learn
File: sklearn/utils/estimator_checks.py (Python)

Code before:
        if y_type != 'binary':
            raise ValueError(
                'Only binary classification is supported. The type of the target '
                f'is {{y_type}}.'
        )
    """.format(name=name)
    err_msg = textwrap.dedent(err_msg)

    with raises(
        ValueError, match="Only binary classification is supported.", err_msg=err_msg
    ):
        estimator.fit(X, y)


def check_all_zero_sample_weights_error(name, estimator_orig):
    """Check that estimator raises error when all sample weights are 0."""
    estimator = clone(estimator_orig)

    X, y = make_classification(random_state=42)
    X = _enforce_estimator_tags_X(estimator, X)
    y = _enforce_estimator_tags_y(estimator, y)

    sample_weight = np.zeros(_num_samples(X))

    with raises(ValueError, match=r"(.*weight.*zero.*)|(.*zero.*weight.*)"):
        estimator.fit(X, y, sample_weight=sample_weight)


Review comment (suggestion):
Let's add another sample weight check function to here:

https://github.com/scikit-learn/scikit-learn/blob/aae146987f3ded82dc631dc1f194863917c575b4/sklearn/utils/estimator_checks.py#L157-L159

You can probably use the other check functions as a template

Code after:

    X, y = make_classification(
        n_samples=100,
        n_classes=3,
        n_informative=3,
        n_clusters_per_class=1,
        random_state=0,
    )
    err_msg = """\
        The estimator tag `tags.classifier_tags.multi_class` is False for {name}
        which means it does not support multiclass classification. However, it does
        not raise the right `ValueError` when calling fit with a multiclass dataset,
        including the error message 'Only binary classification is supported.' This
        can be achieved by the following pattern:

        y_type = type_of_target(y, input_name='y', raise_unknown=True)
        if y_type != 'binary':
            raise ValueError(
                'Only binary classification is supported. The type of the target '
                f'is {{y_type}}.'
        )
    """.format(name=name)
    err_msg = textwrap.dedent(err_msg)

    with raises(
        ValueError, match="Only binary classification is supported.", err_msg=err_msg
    ):
        estimator.fit(X, y)
Repository: scikit-learn/scikit-learn
File: sklearn/utils/_array_api.py (Python)

Code before:

    If necessary the arrays are moved to the same namespace and device as
    the reference array.

    Parameters
    ----------
    reference : array
        Reference array.

    *arrays : array
        Arrays to check.

    Returns
    -------
    arrays : list
        Arrays with the same namespace and device as reference.
    """
    xp, is_array_api = get_namespace(reference)

    if is_array_api:
        device_ = device(reference)
        # Move arrays to the same namespace and device as the reference array.
        # Since `sample_weight` can be None we also allow None.
        return [
            xp.asarray(a, device=device_) if a is not None else None for a in arrays
        ]
    else:
        return arrays


def _remove_non_arrays(*arrays, remove_none=True, remove_types=(str,)):
    """Filter arrays to exclude None and/or specific types.

    Sparse arrays are always filtered out.

    Parameters
    ----------
    *arrays : array objects
        Array objects.

    remove_none : bool, default=True
        Whether to ignore None objects passed in arrays.

    remove_types : tuple or list, default=(str,)
        Types to ignore in the arrays.

    Returns
    -------
    filtered_arrays : list
        List of arrays filtered as requested. An empty list is returned if no input
        passes the filters.

Review comment (suggestion):
Just a FYI I have #31829 which deals with such things (and uses dlpack) but I think that will take a while to get merged.

Code after:

    If necessary the arrays are moved to the same namespace and device as
    the reference array.

    Parameters
    ----------
    reference : array
        Reference array.

    *arrays : array
        Arrays to check.

    Returns
    -------
    arrays : list
        Arrays with the same namespace and device as reference.
    """
    xp, is_array_api = get_namespace(reference)

    if is_array_api:
        device_ = device(reference)
        # Move arrays to the same namespace and device as the reference array.
        return [
            xp.asarray(a, device=device_) if a is not None else None for a in arrays
        ]
    else:
        return arrays


def _remove_non_arrays(*arrays, remove_none=True, remove_types=(str,)):
    """Filter arrays to exclude None and/or specific types.

    Sparse arrays are always filtered out.

    Parameters
    ----------
    *arrays : array objects
        Array objects.

    remove_none : bool, default=True
        Whether to ignore None objects passed in arrays.

    remove_types : tuple or list, default=(str,)
        Types to ignore in the arrays.

    Returns
    -------
    filtered_arrays : list
        List of arrays filtered as requested. An empty list is returned if no input
        passes the filters.
    """