Talip7/scikit-learn-issues-multilabel huggingface dataset:
Sparse Bernoulli Naive Bayes

### Describe the workflow you want to enable

The idea seems simple enough as described in this paper: https://arxiv.org/pdf/1905.09884.pdf

> Due to its linear complexity, naive Bayes classification remains an attractive su-
> pervised learning method, especially in very large-scale settings. We propose a
> sparse version of naive Bayes, which can be used for feature selection. This leads
> to a combinatorial maximum-likelihood problem, for which we provide an exact
> solution in the case of binary data, or a bound in the multinomial case. We prove
> that our bound becomes tight as the marginal contribution of additional features
> decreases. Both binary and multinomial sparse models are solvable in time almost
> linear in problem size, representing a very small extra relative cost compared to
> the classical naive Bayes. Numerical experiments on text data show that the naive
> Bayes feature selection method is as statistically effective as state-of-the-art feature
> selection methods such as recursive feature elimination, l1-penalized logistic re-
> gression and LASSO, while being orders of magnitude faster. For a large data set,
> having more than with 1.6 million training points and about 12 million features,
> and with a non-optimized CPU implementation, our sparse naive Bayes model can
> be trained in less than 15 seconds.

### Describe your proposed solution

Add a regularization parameter to the current implementation of BernoulliNB that allows to control sparsity of the solution similar to regularization strength or "C" in LogisticRegression with a L1 penalty.

Make the necessary adjustments to the algorithm.

COMMENT:
I haven't considered how this feature would affect the online learning component of the algorithm.

Labels: New Feature, module:naive_bayes, Needs Decision - Include Feature
Sparse Bernoulli Naive Bayes

### Describe the workflow you want to enable

The idea seems simple enough as described in this paper: https://arxiv.org/pdf/1905.09884.pdf

> Due to its linear complexity, naive Bayes classification remains an attractive su-
> pervised learning method, especially in very large-scale settings. We propose a
> sparse version of naive Bayes, which can be used for feature selection. This leads
> to a combinatorial maximum-likelihood problem, for which we provide an exact
> solution in the case of binary data, or a bound in the multinomial case. We prove
> that our bound becomes tight as the marginal contribution of additional features
> decreases. Both binary and multinomial sparse models are solvable in time almost
> linear in problem size, representing a very small extra relative cost compared to
> the classical naive Bayes. Numerical experiments on text data show that the naive
> Bayes feature selection method is as statistically effective as state-of-the-art feature
> selection methods such as recursive feature elimination, l1-penalized logistic re-
> gression and LASSO, while being orders of magnitude faster. For a large data set,
> having more than with 1.6 million training points and about 12 million features,
> and with a non-optimized CPU implementation, our sparse naive Bayes model can
> be trained in less than 15 seconds.

### Describe your proposed solution

Add a regularization parameter to the current implementation of BernoulliNB that allows to control sparsity of the solution similar to regularization strength or "C" in LogisticRegression with a L1 penalty.

Make the necessary adjustments to the algorithm.

COMMENT:
I do not think the proposed method meets [scikit-learn's inclusion criterion](https://scikit-learn.org/stable/faq.html#what-are-the-inclusion-criteria-for-new-algorithms). Every feature we include has a [maintenance cost](http://scikit-learn.org/dev/faq.html#why-are-you-so-selective-on-what-algorithms-you-include-in-scikit-learn) and our maintainers are mostly volunteers. For a new feature to be included, we need evidence that it is often useful and, ideally, [well-established](http://scikit-learn.org/dev/faq.html#what-are-the-inclusion-criteria-for-new-algorithms) in the literature or in practice.

Labels: New Feature, module:naive_bayes, Needs Decision - Include Feature
SLEP006: mypy can't find `set_{method}_request` methods

`set_{method}_request` methods are added to all estimators inheriting from `BaseEstimator`, around this part of the code:

https://github.com/scikit-learn/scikit-learn/blob/e9f5676c5ed025d0e71c0a3970d8544a25075933/sklearn/utils/_metadata_requests.py#L1004-L1013

Since `mypy` passes the code to AST and that's how they check for types and available methods, any code using these methods such as `est.set_fit_request(sample_weight=True)` would make `mypy` to fail with and error similar to:

```
error: "LinearRegression" has no attribute "set_fit_request"
```
There are at least two ways to fix this issue:

1. Write a plugin for `mypy` to somehow fix the issues:
- `mypy`'s reference: https://mypy.readthedocs.io/en/stable/extending_mypy.html
- `numpy`'s example: https://numpy.org/devdocs/reference/typing.html

2. Declare all possible methods for all objects for type checkers even if they don't exist. This would silence the type checker errors, but it's a bit hackish. For this, we'd need to change the declaration of the `_MetadataRequester` and add:

``` python
from typing import TYPE_CHECKING

class _MetadataRequester:
    """Mixin class for adding metadata request functionality.

    .. versionadded:: 1.2
    """

    if TYPE_CHECKING:
        set_fit_request: Callable[..., Any]
        set_partial_fit_request: Callable[..., Any]
        set_predict_request: Callable[..., Any]
        set_predict_proba_request: Callable[..., Any]
        ...

    def __init_subclass__(cls, **kwargs):
        ...
```

This wouldn't pollute the runtime env since in runtime `TYPE_CHECKING` is always `False`.

I'm in favor of adding the second option as a temporary solution until somebody implemented the plugin, if they do.

COMMENT:
We likely need to go through the second option always. The mypy plugin system does not work with other typing checking tools such as Pylance.

Labels: New Feature
Inconsistence between components in `PCA` and `SparsePCA`

The components provided by `PCA` and `SparsePCA` are not consistent when we explicitly do not impose any sparsity constraint in `SparsePCA`. It is only a sign flipped meaning that this might not be dramatic.

```python
import numpy as np
from sklearn.decomposition import PCA, SparsePCA

rng = np.random.default_rng(0)
X = rng.normal(size=(100, 4))

n_components = 2
pca = PCA(
    n_components=n_components,
    svd_solver="randomized",
    random_state=0,
).fit(X)
spca = SparsePCA(
    n_components=n_components,
    method="lars",
    ridge_alpha=0,
    alpha=0,
    random_state=0,
).fit(X)

print(pca.components_)
print(spca.components_)
```

```
[[ 0.04020159 -0.111852   -0.25031026 -0.9608422 ]
 [-0.50546893 -0.01489746 -0.83939039  0.19925612]]
[[-0.04020159  0.111852    0.25031026  0.9608422 ]
 [ 0.50546893  0.01489746  0.83939039 -0.19925612]]
```

I am wondering if we should or should not solve this issue and if we should at least document this behaviour in a note in `SparsePCA`.

COMMENT:
ping @agramfort (since they are some SVD flip and LARS/CD solver under the hood :))

Labels: Needs Triage
Inconsistence between components in `PCA` and `SparsePCA`

The components provided by `PCA` and `SparsePCA` are not consistent when we explicitly do not impose any sparsity constraint in `SparsePCA`. It is only a sign flipped meaning that this might not be dramatic.

```python
import numpy as np
from sklearn.decomposition import PCA, SparsePCA

rng = np.random.default_rng(0)
X = rng.normal(size=(100, 4))

n_components = 2
pca = PCA(
    n_components=n_components,
    svd_solver="randomized",
    random_state=0,
).fit(X)
spca = SparsePCA(
    n_components=n_components,
    method="lars",
    ridge_alpha=0,
    alpha=0,
    random_state=0,
).fit(X)

print(pca.components_)
print(spca.components_)
```

```
[[ 0.04020159 -0.111852   -0.25031026 -0.9608422 ]
 [-0.50546893 -0.01489746 -0.83939039  0.19925612]]
[[-0.04020159  0.111852    0.25031026  0.9608422 ]
 [ 0.50546893  0.01489746  0.83939039 -0.19925612]]
```

I am wondering if we should or should not solve this issue and if we should at least document this behaviour in a note in `SparsePCA`.

COMMENT:
I would consider using the svd_flip function in SparsePCA like we do in PCA. See

https://github.com/scikit-learn/scikit-learn/blob/main/sklearn/decomposition/_pca.py#L527

It would make the sign deterministic in SparsePCA as otherwise any sign change applied both to loadings and components is valid.

Labels: Needs Triage
TimeSeriesSplit Needs a version that allows users to specify initial time

### Describe the workflow you want to enable

I would like to add a new `TimeSeriesSplit` [link](https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.TimeSeriesSplit.html) called `TimeSeriesRollingOriginSplit`. In the current `TimeSeriesSplit` it is not possible for users to get an initial length.

I am happy to open a PR for this myself if it is approved.


### Describe your proposed solution


This is the proposed new `TimeSeriesRollingOriginSplit`
`class sklearn.model_selection.TimeSeriesRollingOriginSplit(initial = 5, test_size = 1, gap = 0)`

`initial`: the initial number of samples to use. it is often common to start with a larger training period.

`test_size`: Used to set the test_size

`gap`: Number of samples to exclude from the end of each train set before the test set.


### Describe alternatives you've considered, if relevant

I currently use `TimeSeriesSplit` in a loop, but exclude the initial splits until the train set is the size I want.

### Additional context

Again. I am happy to contribute this feature.

COMMENT:
To better understand the proposed API, can you show an example of what the train and test indices look like when `initial=5` with the proposed splitter?

Labels: New Feature, module:model_selection, Needs Decision - Include Feature
TimeSeriesSplit Needs a version that allows users to specify initial time

### Describe the workflow you want to enable

I would like to add a new `TimeSeriesSplit` [link](https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.TimeSeriesSplit.html) called `TimeSeriesRollingOriginSplit`. In the current `TimeSeriesSplit` it is not possible for users to get an initial length.

I am happy to open a PR for this myself if it is approved.


### Describe your proposed solution


This is the proposed new `TimeSeriesRollingOriginSplit`
`class sklearn.model_selection.TimeSeriesRollingOriginSplit(initial = 5, test_size = 1, gap = 0)`

`initial`: the initial number of samples to use. it is often common to start with a larger training period.

`test_size`: Used to set the test_size

`gap`: Number of samples to exclude from the end of each train set before the test set.


### Describe alternatives you've considered, if relevant

I currently use `TimeSeriesSplit` in a loop, but exclude the initial splits until the train set is the size I want.

### Additional context

Again. I am happy to contribute this feature.

COMMENT:
I have a PR with the initial set at 21. The output is in commented out code, but I will run with the initial set to 5 if that would be useful.

https://github.com/scikit-learn/scikit-learn/pull/23931/files

Labels: New Feature, module:model_selection, Needs Decision - Include Feature
TimeSeriesSplit Needs a version that allows users to specify initial time

### Describe the workflow you want to enable

I would like to add a new `TimeSeriesSplit` [link](https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.TimeSeriesSplit.html) called `TimeSeriesRollingOriginSplit`. In the current `TimeSeriesSplit` it is not possible for users to get an initial length.

I am happy to open a PR for this myself if it is approved.


### Describe your proposed solution


This is the proposed new `TimeSeriesRollingOriginSplit`
`class sklearn.model_selection.TimeSeriesRollingOriginSplit(initial = 5, test_size = 1, gap = 0)`

`initial`: the initial number of samples to use. it is often common to start with a larger training period.

`test_size`: Used to set the test_size

`gap`: Number of samples to exclude from the end of each train set before the test set.


### Describe alternatives you've considered, if relevant

I currently use `TimeSeriesSplit` in a loop, but exclude the initial splits until the train set is the size I want.

### Additional context

Again. I am happy to contribute this feature.

COMMENT:
I think it is better to do the API discussion here, before going into the implementation. As noted in https://github.com/scikit-learn/scikit-learn/pull/23931/files#r925724550 it is unclear what `initial` does.

I want to have some concrete examples of what the proposed API is before going into a implementation. You can propose an API here without having a working implementation.

Labels: New Feature, module:model_selection, Needs Decision - Include Feature
TimeSeriesSplit Needs a version that allows users to specify initial time

### Describe the workflow you want to enable

I would like to add a new `TimeSeriesSplit` [link](https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.TimeSeriesSplit.html) called `TimeSeriesRollingOriginSplit`. In the current `TimeSeriesSplit` it is not possible for users to get an initial length.

I am happy to open a PR for this myself if it is approved.


### Describe your proposed solution


This is the proposed new `TimeSeriesRollingOriginSplit`
`class sklearn.model_selection.TimeSeriesRollingOriginSplit(initial = 5, test_size = 1, gap = 0)`

`initial`: the initial number of samples to use. it is often common to start with a larger training period.

`test_size`: Used to set the test_size

`gap`: Number of samples to exclude from the end of each train set before the test set.


### Describe alternatives you've considered, if relevant

I currently use `TimeSeriesSplit` in a loop, but exclude the initial splits until the train set is the size I want.

### Additional context

Again. I am happy to contribute this feature.

COMMENT:
Sorry, I just saw this comment. We can move the discussion here. I will follow your lead.

Labels: New Feature, module:model_selection, Needs Decision - Include Feature
TimeSeriesSplit Needs a version that allows users to specify initial time

### Describe the workflow you want to enable

I would like to add a new `TimeSeriesSplit` [link](https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.TimeSeriesSplit.html) called `TimeSeriesRollingOriginSplit`. In the current `TimeSeriesSplit` it is not possible for users to get an initial length.

I am happy to open a PR for this myself if it is approved.


### Describe your proposed solution


This is the proposed new `TimeSeriesRollingOriginSplit`
`class sklearn.model_selection.TimeSeriesRollingOriginSplit(initial = 5, test_size = 1, gap = 0)`

`initial`: the initial number of samples to use. it is often common to start with a larger training period.

`test_size`: Used to set the test_size

`gap`: Number of samples to exclude from the end of each train set before the test set.


### Describe alternatives you've considered, if relevant

I currently use `TimeSeriesSplit` in a loop, but exclude the initial splits until the train set is the size I want.

### Additional context

Again. I am happy to contribute this feature.

COMMENT:
Yea, let's move the discussion here. From https://github.com/scikit-learn/scikit-learn/pull/23931#discussion_r925744547

> It looks like you want a walk forward as in https://github.com/scikit-learn/scikit-learn/pull/23780 but with an additional `min_train_size`.

Labels: New Feature, module:model_selection, Needs Decision - Include Feature
TimeSeriesSplit Needs a version that allows users to specify initial time

### Describe the workflow you want to enable

I would like to add a new `TimeSeriesSplit` [link](https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.TimeSeriesSplit.html) called `TimeSeriesRollingOriginSplit`. In the current `TimeSeriesSplit` it is not possible for users to get an initial length.

I am happy to open a PR for this myself if it is approved.


### Describe your proposed solution


This is the proposed new `TimeSeriesRollingOriginSplit`
`class sklearn.model_selection.TimeSeriesRollingOriginSplit(initial = 5, test_size = 1, gap = 0)`

`initial`: the initial number of samples to use. it is often common to start with a larger training period.

`test_size`: Used to set the test_size

`gap`: Number of samples to exclude from the end of each train set before the test set.


### Describe alternatives you've considered, if relevant

I currently use `TimeSeriesSplit` in a loop, but exclude the initial splits until the train set is the size I want.

### Additional context

Again. I am happy to contribute this feature.

COMMENT:
Come from https://github.com/scikit-learn/scikit-learn/pull/23931#discussion_r925744547:

@alexhallam With initial=21, should the first split have training indices `[0, 1, 2, 3, 4, ..., 20]`, and the test set be `[21, 22, .., 27]` because `test_size=7`? In other words, `initial` sets the size of the training set of the first split.

Labels: New Feature, module:model_selection, Needs Decision - Include Feature
TimeSeriesSplit Needs a version that allows users to specify initial time

### Describe the workflow you want to enable

I would like to add a new `TimeSeriesSplit` [link](https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.TimeSeriesSplit.html) called `TimeSeriesRollingOriginSplit`. In the current `TimeSeriesSplit` it is not possible for users to get an initial length.

I am happy to open a PR for this myself if it is approved.


### Describe your proposed solution


This is the proposed new `TimeSeriesRollingOriginSplit`
`class sklearn.model_selection.TimeSeriesRollingOriginSplit(initial = 5, test_size = 1, gap = 0)`

`initial`: the initial number of samples to use. it is often common to start with a larger training period.

`test_size`: Used to set the test_size

`gap`: Number of samples to exclude from the end of each train set before the test set.


### Describe alternatives you've considered, if relevant

I currently use `TimeSeriesSplit` in a loop, but exclude the initial splits until the train set is the size I want.

### Additional context

Again. I am happy to contribute this feature.

COMMENT:
Responding to https://github.com/scikit-learn/scikit-learn/pull/23931#discussion_r925749323:

> I am afraid that min_train_size means that I want a consistent train size. That is what I do not want. I want one train size for the initial training then a second train size for each iteration.

A `min_train_size` would only set the minimum. The train size can still grow up to `max_train_size`.

Labels: New Feature, module:model_selection, Needs Decision - Include Feature
TimeSeriesSplit Needs a version that allows users to specify initial time

### Describe the workflow you want to enable

I would like to add a new `TimeSeriesSplit` [link](https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.TimeSeriesSplit.html) called `TimeSeriesRollingOriginSplit`. In the current `TimeSeriesSplit` it is not possible for users to get an initial length.

I am happy to open a PR for this myself if it is approved.


### Describe your proposed solution


This is the proposed new `TimeSeriesRollingOriginSplit`
`class sklearn.model_selection.TimeSeriesRollingOriginSplit(initial = 5, test_size = 1, gap = 0)`

`initial`: the initial number of samples to use. it is often common to start with a larger training period.

`test_size`: Used to set the test_size

`gap`: Number of samples to exclude from the end of each train set before the test set.


### Describe alternatives you've considered, if relevant

I currently use `TimeSeriesSplit` in a loop, but exclude the initial splits until the train set is the size I want.

### Additional context

Again. I am happy to contribute this feature.

COMMENT:
> Come from https://github.com/scikit-learn/scikit-learn/pull/23931#discussion_r925744547:
> @alexhallam With initial=21, should the first split have training indices [0, 1, 2, 3, 4, ..., 20], and the test set be [21, 22, .., 27]  because test_size=7? In other words, initial sets the size of the training set of the first split.

https://github.com/scikit-learn/scikit-learn/blob/1aa6181ce03b9f860eb7fae6ec2608f2cb495754/sklearn/model_selection/_split.py#L1135-L1162

Yes, that is correct with `gap=0`. I must have pasted output from a run where gap was not 0. My mistake I can fix that.

Labels: New Feature, module:model_selection, Needs Decision - Include Feature
TimeSeriesSplit Needs a version that allows users to specify initial time

### Describe the workflow you want to enable

I would like to add a new `TimeSeriesSplit` [link](https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.TimeSeriesSplit.html) called `TimeSeriesRollingOriginSplit`. In the current `TimeSeriesSplit` it is not possible for users to get an initial length.

I am happy to open a PR for this myself if it is approved.


### Describe your proposed solution


This is the proposed new `TimeSeriesRollingOriginSplit`
`class sklearn.model_selection.TimeSeriesRollingOriginSplit(initial = 5, test_size = 1, gap = 0)`

`initial`: the initial number of samples to use. it is often common to start with a larger training period.

`test_size`: Used to set the test_size

`gap`: Number of samples to exclude from the end of each train set before the test set.


### Describe alternatives you've considered, if relevant

I currently use `TimeSeriesSplit` in a loop, but exclude the initial splits until the train set is the size I want.

### Additional context

Again. I am happy to contribute this feature.

COMMENT:
I am asking the question because I do not understand the code example. The first split is `TRAIN: [0] TEST: [1]` with default `initial=21` for `TimeSeriesInitialSplit`:

https://github.com/scikit-learn/scikit-learn/blob/1aa6181ce03b9f860eb7fae6ec2608f2cb495754/sklearn/model_selection/_split.py#L1137-L1145

This is not consistent with your description in https://github.com/scikit-learn/scikit-learn/issues/23923#issue-1306809804:

> initial: the initial number of samples to use. it is often common to start with a larger training period.

Is there a typo in the example? The second code example uses `TimeSeriesSplit(n_splits=3, test_size=2)`. Should it be `TimeSeriesInitialSplit`?

Labels: New Feature, module:model_selection, Needs Decision - Include Feature