Talip7/scikit-learn-issues-multilabel huggingface dataset:
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
really nice thread and very good questions @jameslamb @nicklamiller . 

It's true that in scikit-learn, we almost always call `_validate_data`, unless in rare occasions it's not suitable. But it's also true that this is not documented, and one could argue that by convention, anything starting with a single `_` in sklearn is considered private.

However, there are things in sklearn which are _private_ in a sense, but one really needs to use them (or have a variant re-implemented for themselves) to have a completely sklearn compatible estimator. This is far from ideal, and we recognize that.

Note that when we consider something _private_, we can change its functionality and signature w/o a prior warning / deprecation cycle. Therefore we can't responsibly advise people to use it. Another example is `_more_tags` which really helps, but is also private.

Fixing these issues is in our immediate timeline with high priority. And I'm personally working on it.

So in short:
- scikit-learn estimators all add `n_features_in_` and feature_names_in`, and `get_feature_names_out()`, and used in meta-estimators / tools whenever appropriate if available
- we have not enforced this as a part of the contract via `check_estimator` tests and things shouldn't break (yet) if they don't exist

We're also working on creating a better test suite to separate API / compatibility tests from common tests which are really more of a scikit-learn internal tests. (@glemaitre has had a stab at it and is WIP).

I know this is not a perfect answer, but I thank you for the thread and it's triggering us to raise the priority of this front further.

Labels: New Feature, Developer API
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
Thanks very much for reading through this whole thread and for that response.

We'll continue over in LightGBM with implementing `feature_names_in_` and `get_feature_names_out()`. We'll do that without diretly calling `_validate_data()`, to avoid relying on a method that seems like it may be considered private.

Please feel free to `@` me any time if you'd like another third-party library developer opinion on this or any of the other issues in https://github.com/orgs/scikit-learn/projects/13.

Labels: New Feature, Developer API
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
@jameslamb note that in the meantime you can use `sklearn.utils.validation.validate_data` which is now intended to be used by third party developers.

Labels: New Feature, Developer API
Enforce `feature_names_in_` and `n_features_in_` in `check_estimator` post SLEP007 implementation

### Describe the workflow you want to enable

I would like to propose an enhancement to the `check_estimator` function, particularly in light of the implementation of SLEP007. As per SLEP007, which was integrated into scikit-learn starting from version 1.1.0, estimators are expected to support `feature_names_in_` and `n_features_in_` attributes. However, it appears that the `check_estimator` utility does not currently enforce the presence of these attributes.

### Describe your proposed solution

I propose that the `check_estimator` function be updated to include checks for the existence of the `feature_names_in_` and `n_features_in_` attributes.

### Additional context

- SLEP007:[ https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html](https://scikit-learn-enhancement-proposals.readthedocs.io/en/latest/slep007/proposal.html)
- Version of scikit-learn where SLEP007 was implemented: 1.1.0
- Example of `check_estimator` not enforcing these attributes: https://github.com/microsoft/LightGBM/issues/6279

COMMENT:
Thanks very much for the `@`!

Echoing what I just wrote here: https://github.com/microsoft/LightGBM/pull/6651#discussion_r1772663511

... over in `lightgbm`, I'm proposing we skip that check and avoid usage of that function for now. LightGBM tries to support a wide range of `scikit-learn` versions so calling something (`validate_data()`) that was just added in v1.6 (which is not even released yet) would introduce a need to try-catch. I'm not convinced the complexity is worth it, especially given that LightGBM has other mechanisms for checking the shape of scoring data against the shape of training data.

Labels: New Feature, Developer API
⚠️ CI failed on Linux.pymin_conda_defaults_openblas ⚠️

**CI failed on [Linux.pymin_conda_defaults_openblas](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=63344&view=logs&j=66042141-7fd2-581d-812e-1a1b1d5e0f0c)** (Feb 01, 2024)
- sklearn.linear_model._ridge.ridge_regression

COMMENT:
Doctest with `SKLEARN_TESTS_GLOBAL_RANDOM_SEED="86"` easy to fix by using `1.9...` instead of `1.97...` in the expected output.

```
____________ [doctest] sklearn.linear_model._ridge.ridge_regression ____________
[gw1] linux -- Python 3.9.18 /usr/share/miniconda/envs/testvenv/bin/python
511     Examples
512     --------
513     >>> import numpy as np
514     >>> from sklearn.datasets import make_regression
515     >>> from sklearn.linear_model import ridge_regression
516     >>> rng = np.random.RandomState(0)
517     >>> X = rng.randn(100, 4)
518     >>> y = 2.0 * X[:, 0] - 1.0 * X[:, 1] + 0.1 * rng.standard_normal(100)
519     >>> coef, intercept = ridge_regression(X, y, alpha=1.0, return_intercept=True)
520     >>> list(coef)
Expected:
    [1.97..., -1.00..., -0.0..., -0.0...]
Got:
    [1.9699158003101864, -1.0031065875403011, -0.0027493998515681233, -0.0008975207465662833]

/home/vsts/work/1/s/sklearn/linear_model/_ridge.py:520: DocTestFailure
```

Labels: Needs Triage
Migrate macOS arm64 wheel building and testing CI to GitHub Actions runner

Our Cirrus CI account often gets rate limited because we tend to exhaust the credits allocated for free to Open Source project.

This can slow down the release process quite significantly.

Fortunately, GitHub Actions just introduced new M1-based runners in their offer.

https://github.blog/changelog/2024-01-30-github-actions-introducing-the-new-m1-macos-runner-available-to-open-source/

For convenience, here are the existing config with a relevant in-line comment with more details about our current wheel CI:

https://github.com/scikit-learn/scikit-learn/blob/bb8776874410d65f510717c97b8027331bb0f3ff/.github/workflows/wheels.yml#L106-L119

and the dual section in our current Cirrus CI config.

https://github.com/scikit-learn/scikit-learn/blob/bb8776874410d65f510717c97b8027331bb0f3ff/build_tools/cirrus/arm_wheel.yml#L17-L25

We should consider migrating to our wheel building to the new GitHub Actions macOS/M1 workers to decrease the credit consumption of our Cirrus CI account: it would only be used for Linux/arm64 tests and wheel building.

Side note: we could also use those new for the Linux arm64 wheels via docker and not rely on Cirrus CI at all any more, but using macOS workers to build and test Linux wheels via docker might seem a bit too convoluted for our taste :)

COMMENT:
Removing the "help wanted" label because I realize that updating GitHub Actions workflow config can be security sensitive and requires manual approval in PRs opened by first time contributor.

Therefore this issue would better be addressed by returning contributors who already have contributed to scikit-learn in the past.

Also, here is the documentation on how to trigger the wheel building CI by pushing commits with specific markers in PRs:

- https://scikit-learn.org/stable/developers/contributing.html#continuous-integration-ci

Labels: Build / CI
TimeSeriesSplit Train Size formula correction

### Describe the issue linked to the documentation

The documentation states:
> The training set has size `i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)` in the `i`th split, with a test set of size `n_samples // (n_splits + 1)` by default, where `n_samples` is the number of samples.

The equation for the training set looks like it is flawed. Given a split with:

```python
test_set_size = 10
n_splits = X.shape[0] // test_set_size
max_train_size = 10000

cv = TimeSeriesSplit(n_splits=n_splits, test_size=test_size, max_train_size=max_train_size)

pairs
for i, (train_index, test_index) in enumerate(cv.split(X)):
    pairs.append([i, len(train_index), len(test_index)])
```

The previous code generates a list similar to this:
```
[[0, 7, 10],
[1, 17, 10],
[2, 27, 10],
[3, 37, 10],
...
[996, 9967, 10],
[997, 9977, 10],
[998, 9987, 10],
[999, 9997, 10],
[1000, 10000, 10],
[1001, 10000, 10],
...
[3177, 10000, 10],
[3178, 10000, 10]]
```

I tried using the formula in the documentation to determine the size of the training set at a given iteration:
```python
def get_train_size(i, n_samples, n_splits):
    return i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)

get_train_size(0, 31797, 3179)  # 3177, which is incorrect, it should be 7!


def get_train_size(i, n_samples, n_splits, max_train_size=float("inf")):
    start_value = n_samples % n_splits

    increment = n_samples // n_splits

    return int(min(start_value + i * increment, max_train_size))

get_train_size(0, 31797, 3179)  # 7, correct
get_train_size(999, 31797, 3179)  # 9997, correct
get_train_size(1000, 31797, 3179)  # 10007, correct
get_train_size(1000, 31797, 3179, 10000)  # 10000, correct
```

### Suggest a potential alternative/fix

So the correction to the formula should be that the training set size is equal to:
`i * (n_samples // n_splits) + n_samples % n_splits`
instead of:
`i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)`

COMMENT:
Thanks for the report @ZiadAmerr. Note that this note gives the formula when the parameters are left to their default values, but here you set a non-default ``test_size`` and ``max_train_size``.

When leaving params to their default values, I think the formula is correct. Perhaps the confusion comes from the fact that in the formula ``i`` starts at 1 (and not 0): it's used to count folds and refers to the ``ith`` fold.

Labels: Documentation
TimeSeriesSplit Train Size formula correction

### Describe the issue linked to the documentation

The documentation states:
> The training set has size `i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)` in the `i`th split, with a test set of size `n_samples // (n_splits + 1)` by default, where `n_samples` is the number of samples.

The equation for the training set looks like it is flawed. Given a split with:

```python
test_set_size = 10
n_splits = X.shape[0] // test_set_size
max_train_size = 10000

cv = TimeSeriesSplit(n_splits=n_splits, test_size=test_size, max_train_size=max_train_size)

pairs
for i, (train_index, test_index) in enumerate(cv.split(X)):
    pairs.append([i, len(train_index), len(test_index)])
```

The previous code generates a list similar to this:
```
[[0, 7, 10],
[1, 17, 10],
[2, 27, 10],
[3, 37, 10],
...
[996, 9967, 10],
[997, 9977, 10],
[998, 9987, 10],
[999, 9997, 10],
[1000, 10000, 10],
[1001, 10000, 10],
...
[3177, 10000, 10],
[3178, 10000, 10]]
```

I tried using the formula in the documentation to determine the size of the training set at a given iteration:
```python
def get_train_size(i, n_samples, n_splits):
    return i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)

get_train_size(0, 31797, 3179)  # 3177, which is incorrect, it should be 7!


def get_train_size(i, n_samples, n_splits, max_train_size=float("inf")):
    start_value = n_samples % n_splits

    increment = n_samples // n_splits

    return int(min(start_value + i * increment, max_train_size))

get_train_size(0, 31797, 3179)  # 7, correct
get_train_size(999, 31797, 3179)  # 9997, correct
get_train_size(1000, 31797, 3179)  # 10007, correct
get_train_size(1000, 31797, 3179, 10000)  # 10000, correct
```

### Suggest a potential alternative/fix

So the correction to the formula should be that the training set size is equal to:
`i * (n_samples // n_splits) + n_samples % n_splits`
instead of:
`i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)`

COMMENT:
Hi @jeremiedbb, thanks for the note.

I think that since it is written in the documentation without any explicit explanation for the parameters being left to their default values then we should either:
- Explicitly mention that this formula is only valid if `test_size` and `max_train_size` are set to their default values
- Provide a more concrete formula that works given non-default values for either `test_size` or `max_train_size`

Thanks again! :)

Labels: Documentation
TimeSeriesSplit Train Size formula correction

### Describe the issue linked to the documentation

The documentation states:
> The training set has size `i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)` in the `i`th split, with a test set of size `n_samples // (n_splits + 1)` by default, where `n_samples` is the number of samples.

The equation for the training set looks like it is flawed. Given a split with:

```python
test_set_size = 10
n_splits = X.shape[0] // test_set_size
max_train_size = 10000

cv = TimeSeriesSplit(n_splits=n_splits, test_size=test_size, max_train_size=max_train_size)

pairs
for i, (train_index, test_index) in enumerate(cv.split(X)):
    pairs.append([i, len(train_index), len(test_index)])
```

The previous code generates a list similar to this:
```
[[0, 7, 10],
[1, 17, 10],
[2, 27, 10],
[3, 37, 10],
...
[996, 9967, 10],
[997, 9977, 10],
[998, 9987, 10],
[999, 9997, 10],
[1000, 10000, 10],
[1001, 10000, 10],
...
[3177, 10000, 10],
[3178, 10000, 10]]
```

I tried using the formula in the documentation to determine the size of the training set at a given iteration:
```python
def get_train_size(i, n_samples, n_splits):
    return i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)

get_train_size(0, 31797, 3179)  # 3177, which is incorrect, it should be 7!


def get_train_size(i, n_samples, n_splits, max_train_size=float("inf")):
    start_value = n_samples % n_splits

    increment = n_samples // n_splits

    return int(min(start_value + i * increment, max_train_size))

get_train_size(0, 31797, 3179)  # 7, correct
get_train_size(999, 31797, 3179)  # 9997, correct
get_train_size(1000, 31797, 3179)  # 10007, correct
get_train_size(1000, 31797, 3179, 10000)  # 10000, correct
```

### Suggest a potential alternative/fix

So the correction to the formula should be that the training set size is equal to:
`i * (n_samples // n_splits) + n_samples % n_splits`
instead of:
`i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)`

COMMENT:
>  without any explicit explanation for the parameters being left to their default 

It is written: "The training set has size ``i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)`` in the ith split, with a test set of size ``n_samples // (n_splits + 1)`` **by default**".

Although maybe kind of hidden :)
I'd be okay with a PR clarifying it a bit.

Regarding providing a formula for the general case, I think it will quickly get messy with several conditions depending on each parameter, so I'm not sure about that.

Labels: Documentation
TimeSeriesSplit Train Size formula correction

### Describe the issue linked to the documentation

The documentation states:
> The training set has size `i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)` in the `i`th split, with a test set of size `n_samples // (n_splits + 1)` by default, where `n_samples` is the number of samples.

The equation for the training set looks like it is flawed. Given a split with:

```python
test_set_size = 10
n_splits = X.shape[0] // test_set_size
max_train_size = 10000

cv = TimeSeriesSplit(n_splits=n_splits, test_size=test_size, max_train_size=max_train_size)

pairs
for i, (train_index, test_index) in enumerate(cv.split(X)):
    pairs.append([i, len(train_index), len(test_index)])
```

The previous code generates a list similar to this:
```
[[0, 7, 10],
[1, 17, 10],
[2, 27, 10],
[3, 37, 10],
...
[996, 9967, 10],
[997, 9977, 10],
[998, 9987, 10],
[999, 9997, 10],
[1000, 10000, 10],
[1001, 10000, 10],
...
[3177, 10000, 10],
[3178, 10000, 10]]
```

I tried using the formula in the documentation to determine the size of the training set at a given iteration:
```python
def get_train_size(i, n_samples, n_splits):
    return i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)

get_train_size(0, 31797, 3179)  # 3177, which is incorrect, it should be 7!


def get_train_size(i, n_samples, n_splits, max_train_size=float("inf")):
    start_value = n_samples % n_splits

    increment = n_samples // n_splits

    return int(min(start_value + i * increment, max_train_size))

get_train_size(0, 31797, 3179)  # 7, correct
get_train_size(999, 31797, 3179)  # 9997, correct
get_train_size(1000, 31797, 3179)  # 10007, correct
get_train_size(1000, 31797, 3179, 10000)  # 10000, correct
```

### Suggest a potential alternative/fix

So the correction to the formula should be that the training set size is equal to:
`i * (n_samples // n_splits) + n_samples % n_splits`
instead of:
`i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)`

COMMENT:
> Regarding providing a formula for the general case, I think it will quickly get messy with several conditions depending on each parameter

Yes, that does make sense :)

So should I modify this PR to have a clarification that this formula only applies when default values are passed to the other parameters?

Thanks! :)

Labels: Documentation
TimeSeriesSplit Train Size formula correction

### Describe the issue linked to the documentation

The documentation states:
> The training set has size `i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)` in the `i`th split, with a test set of size `n_samples // (n_splits + 1)` by default, where `n_samples` is the number of samples.

The equation for the training set looks like it is flawed. Given a split with:

```python
test_set_size = 10
n_splits = X.shape[0] // test_set_size
max_train_size = 10000

cv = TimeSeriesSplit(n_splits=n_splits, test_size=test_size, max_train_size=max_train_size)

pairs
for i, (train_index, test_index) in enumerate(cv.split(X)):
    pairs.append([i, len(train_index), len(test_index)])
```

The previous code generates a list similar to this:
```
[[0, 7, 10],
[1, 17, 10],
[2, 27, 10],
[3, 37, 10],
...
[996, 9967, 10],
[997, 9977, 10],
[998, 9987, 10],
[999, 9997, 10],
[1000, 10000, 10],
[1001, 10000, 10],
...
[3177, 10000, 10],
[3178, 10000, 10]]
```

I tried using the formula in the documentation to determine the size of the training set at a given iteration:
```python
def get_train_size(i, n_samples, n_splits):
    return i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)

get_train_size(0, 31797, 3179)  # 3177, which is incorrect, it should be 7!


def get_train_size(i, n_samples, n_splits, max_train_size=float("inf")):
    start_value = n_samples % n_splits

    increment = n_samples // n_splits

    return int(min(start_value + i * increment, max_train_size))

get_train_size(0, 31797, 3179)  # 7, correct
get_train_size(999, 31797, 3179)  # 9997, correct
get_train_size(1000, 31797, 3179)  # 10007, correct
get_train_size(1000, 31797, 3179, 10000)  # 10000, correct
```

### Suggest a potential alternative/fix

So the correction to the formula should be that the training set size is equal to:
`i * (n_samples // n_splits) + n_samples % n_splits`
instead of:
`i * n_samples // (n_splits + 1) + n_samples % (n_splits + 1)`

COMMENT:
Sure you can repurpose #28322

Labels: Documentation