Talip7/scikit-learn-issues-multilabel huggingface dataset:
Handle np.nan / missing values in SplineTransformer

I think it would be quite natural to add an option to `SplineTransformer` to accept inputs with missing values as follows:

- `handle_missing="error"` the default (keep current behavior)
- `handle_missing="zero"`/"constant": encode missing values by setting all output features for that input column to 0 (or some other constant, see discussion below),
- `handle_missing="indicator"`: append an extra binary feature as missingness indicator and encode missing values as 0 on the remaining output features.

Note that `handle_missing="indicator"` would be different and statistically more meaningful than  `SimpleImputer(strategy="mean", add_indicator=True)` with `SplineTransformer` and furthermore would make for leaner ML pipelines (better UX).

I am not sure if we need to add the `handle_missing="zero"` option. It would break the property the sum of output values of a given `SplineTransformer` encoding always sum to 1 while `handle_missing="indicator"` would preserve this property (in addition to make missingness more explicit to the downstream model in case missingness is informative one way or another).

If we want to preserve the sum to 1 property while not adding an explicit missingness indicator feature, maybe we could instead provide `handle_missing="constant"` (not sure about the name) that would encode missing values as `1 / n_outputs` to preserve the "sum to 1" property. Not entirely sure if this would result in a more interesting prior than the zero encoding.

COMMENT:
I think both are useful:
- `handle_missing="zero"` nudgesthe downstream model into not leveraging the predictive value of missingness (at the possible cost of model misspecification/underfitting) if this "missingness is not informative" assumption does not hold;
- `handle_missing="indicator"` nudges the downstream model into modeling the predictive value of missingness, at the possible cost of additional overfitting if the sample size is small, the number of feature with missing values is large and the "missingness is informative" assumption does not hold.

Labels: Enhancement
Handle np.nan / missing values in SplineTransformer

I think it would be quite natural to add an option to `SplineTransformer` to accept inputs with missing values as follows:

- `handle_missing="error"` the default (keep current behavior)
- `handle_missing="zero"`/"constant": encode missing values by setting all output features for that input column to 0 (or some other constant, see discussion below),
- `handle_missing="indicator"`: append an extra binary feature as missingness indicator and encode missing values as 0 on the remaining output features.

Note that `handle_missing="indicator"` would be different and statistically more meaningful than  `SimpleImputer(strategy="mean", add_indicator=True)` with `SplineTransformer` and furthermore would make for leaner ML pipelines (better UX).

I am not sure if we need to add the `handle_missing="zero"` option. It would break the property the sum of output values of a given `SplineTransformer` encoding always sum to 1 while `handle_missing="indicator"` would preserve this property (in addition to make missingness more explicit to the downstream model in case missingness is informative one way or another).

If we want to preserve the sum to 1 property while not adding an explicit missingness indicator feature, maybe we could instead provide `handle_missing="constant"` (not sure about the name) that would encode missing values as `1 / n_outputs` to preserve the "sum to 1" property. Not entirely sure if this would result in a more interesting prior than the zero encoding.

COMMENT:
Then I guess that was a semantics misunderstanding. I thought `handle_unknown="zero"` means assume `nan` was `0`. `handle_unknown="ignore"` might be a better name?

Labels: Enhancement
Handle np.nan / missing values in SplineTransformer

I think it would be quite natural to add an option to `SplineTransformer` to accept inputs with missing values as follows:

- `handle_missing="error"` the default (keep current behavior)
- `handle_missing="zero"`/"constant": encode missing values by setting all output features for that input column to 0 (or some other constant, see discussion below),
- `handle_missing="indicator"`: append an extra binary feature as missingness indicator and encode missing values as 0 on the remaining output features.

Note that `handle_missing="indicator"` would be different and statistically more meaningful than  `SimpleImputer(strategy="mean", add_indicator=True)` with `SplineTransformer` and furthermore would make for leaner ML pipelines (better UX).

I am not sure if we need to add the `handle_missing="zero"` option. It would break the property the sum of output values of a given `SplineTransformer` encoding always sum to 1 while `handle_missing="indicator"` would preserve this property (in addition to make missingness more explicit to the downstream model in case missingness is informative one way or another).

If we want to preserve the sum to 1 property while not adding an explicit missingness indicator feature, maybe we could instead provide `handle_missing="constant"` (not sure about the name) that would encode missing values as `1 / n_outputs` to preserve the "sum to 1" property. Not entirely sure if this would result in a more interesting prior than the zero encoding.

COMMENT:
> I thought handle_unknown="zero" means assume nan was 0.

It's true if you read this as "missing values are encoded with null spline feature values". Maybe `handle_unknown="zeros" (plural) would avoid the confusion.

> handle_unknown="ignore" might be a better name?

I am fine with "ignore" as well. What is important is to be explicit in the docstring.

Labels: Enhancement
Handle np.nan / missing values in SplineTransformer

I think it would be quite natural to add an option to `SplineTransformer` to accept inputs with missing values as follows:

- `handle_missing="error"` the default (keep current behavior)
- `handle_missing="zero"`/"constant": encode missing values by setting all output features for that input column to 0 (or some other constant, see discussion below),
- `handle_missing="indicator"`: append an extra binary feature as missingness indicator and encode missing values as 0 on the remaining output features.

Note that `handle_missing="indicator"` would be different and statistically more meaningful than  `SimpleImputer(strategy="mean", add_indicator=True)` with `SplineTransformer` and furthermore would make for leaner ML pipelines (better UX).

I am not sure if we need to add the `handle_missing="zero"` option. It would break the property the sum of output values of a given `SplineTransformer` encoding always sum to 1 while `handle_missing="indicator"` would preserve this property (in addition to make missingness more explicit to the downstream model in case missingness is informative one way or another).

If we want to preserve the sum to 1 property while not adding an explicit missingness indicator feature, maybe we could instead provide `handle_missing="constant"` (not sure about the name) that would encode missing values as `1 / n_outputs` to preserve the "sum to 1" property. Not entirely sure if this would result in a more interesting prior than the zero encoding.

COMMENT:
> @lorentzenchr , I'm really not sure what `handle_missing="zero"` brings. That doesn't even do anything for the linear models. Unless there's something I'm missing in your analysis.

Setting the whole row of spline basis functions to zero is what‘s not possible with SimpleImputer. That’s why it has highest prio for me. (Let‘s discuss naming in the PR)
Adding an indicator column is already possible with a SimpleImputer.

Labels: Enhancement
Handle np.nan / missing values in SplineTransformer

I think it would be quite natural to add an option to `SplineTransformer` to accept inputs with missing values as follows:

- `handle_missing="error"` the default (keep current behavior)
- `handle_missing="zero"`/"constant": encode missing values by setting all output features for that input column to 0 (or some other constant, see discussion below),
- `handle_missing="indicator"`: append an extra binary feature as missingness indicator and encode missing values as 0 on the remaining output features.

Note that `handle_missing="indicator"` would be different and statistically more meaningful than  `SimpleImputer(strategy="mean", add_indicator=True)` with `SplineTransformer` and furthermore would make for leaner ML pipelines (better UX).

I am not sure if we need to add the `handle_missing="zero"` option. It would break the property the sum of output values of a given `SplineTransformer` encoding always sum to 1 while `handle_missing="indicator"` would preserve this property (in addition to make missingness more explicit to the downstream model in case missingness is informative one way or another).

If we want to preserve the sum to 1 property while not adding an explicit missingness indicator feature, maybe we could instead provide `handle_missing="constant"` (not sure about the name) that would encode missing values as `1 / n_outputs` to preserve the "sum to 1" property. Not entirely sure if this would result in a more interesting prior than the zero encoding.

COMMENT:
> Setting the whole row of spline basis functions to zero is what‘s not possible with > SimpleImputer. That’s why it has highest prio for me. (Let‘s discuss naming in the PR)

Ok.

>  Adding an indicator column is already possible with a SimpleImputer.

Yes but it's not possible to do both together: zero value for the spline feature in conjunction with a non-zero indicator of missingness.

I think both options have complementary inductive biases as I explained in https://github.com/scikit-learn/scikit-learn/issues/26793#issuecomment-1977149082. It does not really matter which we implement first as long as we agree that we want both in the long run.

Labels: Enhancement
Handle np.nan / missing values in SplineTransformer

I think it would be quite natural to add an option to `SplineTransformer` to accept inputs with missing values as follows:

- `handle_missing="error"` the default (keep current behavior)
- `handle_missing="zero"`/"constant": encode missing values by setting all output features for that input column to 0 (or some other constant, see discussion below),
- `handle_missing="indicator"`: append an extra binary feature as missingness indicator and encode missing values as 0 on the remaining output features.

Note that `handle_missing="indicator"` would be different and statistically more meaningful than  `SimpleImputer(strategy="mean", add_indicator=True)` with `SplineTransformer` and furthermore would make for leaner ML pipelines (better UX).

I am not sure if we need to add the `handle_missing="zero"` option. It would break the property the sum of output values of a given `SplineTransformer` encoding always sum to 1 while `handle_missing="indicator"` would preserve this property (in addition to make missingness more explicit to the downstream model in case missingness is informative one way or another).

If we want to preserve the sum to 1 property while not adding an explicit missingness indicator feature, maybe we could instead provide `handle_missing="constant"` (not sure about the name) that would encode missing values as `1 / n_outputs` to preserve the "sum to 1" property. Not entirely sure if this would result in a more interesting prior than the zero encoding.

COMMENT:
> Yes but it's not possible to do both together: zero value for the spline feature in conjunction with a non-zero indicator of missingness.

How about
```python
from sklearn.compose import ColumnTransformer
from sklearn.impute import MissingIndicator
from sklearn.preprocessing import SplineTransformer


ct = ColumnTransformer(
    [
        ("spline", SplineTransformer(handle_unknown="zero"), ["numerical_feature"]),
        ("missing_indicator", MissingIndicator(), ["numerical_feature"]),
    ],
)
```
While it might seem convenient, I would really like to start small and only add `handle_unknown="zero"` and `"error"`.

Labels: Enhancement
Handle np.nan / missing values in SplineTransformer

I think it would be quite natural to add an option to `SplineTransformer` to accept inputs with missing values as follows:

- `handle_missing="error"` the default (keep current behavior)
- `handle_missing="zero"`/"constant": encode missing values by setting all output features for that input column to 0 (or some other constant, see discussion below),
- `handle_missing="indicator"`: append an extra binary feature as missingness indicator and encode missing values as 0 on the remaining output features.

Note that `handle_missing="indicator"` would be different and statistically more meaningful than  `SimpleImputer(strategy="mean", add_indicator=True)` with `SplineTransformer` and furthermore would make for leaner ML pipelines (better UX).

I am not sure if we need to add the `handle_missing="zero"` option. It would break the property the sum of output values of a given `SplineTransformer` encoding always sum to 1 while `handle_missing="indicator"` would preserve this property (in addition to make missingness more explicit to the downstream model in case missingness is informative one way or another).

If we want to preserve the sum to 1 property while not adding an explicit missingness indicator feature, maybe we could instead provide `handle_missing="constant"` (not sure about the name) that would encode missing values as `1 / n_outputs` to preserve the "sum to 1" property. Not entirely sure if this would result in a more interesting prior than the zero encoding.

COMMENT:
Yes @lorentzenchr, I am working on this in the PR, where (to address your concerns) I now got rid of the code to concatenate the indicator column to the rest of X_transform. We can still add this later if needed.

Labels: Enhancement
⚠️ CI failed on Linux_Nightly.pylatest_pip_scipy_dev ⚠️

**CI is still failing on [Linux_Nightly.pylatest_pip_scipy_dev](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=58166&view=logs&j=dfe99b15-50db-5d7b-b1e9-4105c42527cf)** (Aug 22, 2023)
- test_imputation_mean_median_error_invalid_type[None-median]
- test_imputation_mean_median_error_invalid_type[str-median]
- test_imputation_mean_median_error_invalid_type[None-mean]
- test_imputation_mean_median_error_invalid_type[str-mean]
- test_metrics_pos_label_error_str[str-brier_score_loss-True]
- test_metrics_pos_label_error_str[str-precision_recall_curve-True]
- test_metrics_pos_label_error_str[str-roc_curve-True]
- test_ohe_infrequent_two_levels_drop_infrequent_errors[drop0]
- test_ohe_infrequent_two_levels_drop_infrequent_errors[drop1]
- test_ohe_infrequent_three_levels_drop_infrequent_errors[drop0]
- test_ohe_infrequent_three_levels_drop_infrequent_errors[drop1]
- test_binary_clf_curve_implicit_pos_label[det_curve]
- test_binary_clf_curve_implicit_pos_label[precision_recall_curve]
- test_binary_clf_curve_implicit_pos_label[roc_curve]
- test_calibration_curve_pos_label_error_str[str]
- test_constant_strategy_exceptions[single-output]
- test_constant_strategy_exceptions[multi-output]
- test_compute_class_weight_not_present[numeric-class_weight1-classes1-The classes, \\[0, 1, 2, 3\\], are not in class_weight]
- test_compute_class_weight_not_present[string-class_weight4-classes4-The classes, \\['dog'\\], are not in class_weight]

COMMENT:
## CI is no longer failing! ✅

[Successful run](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=58207&view=logs&j=dfe99b15-50db-5d7b-b1e9-4105c42527cf) on Aug 23, 2023

Labels: Needs Triage
⚠️ CI failed on Linux_Nightly.pylatest_pip_scipy_dev ⚠️

**CI is still failing on [Linux_Nightly.pylatest_pip_scipy_dev](https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=58166&view=logs&j=dfe99b15-50db-5d7b-b1e9-4105c42527cf)** (Aug 22, 2023)
- test_imputation_mean_median_error_invalid_type[None-median]
- test_imputation_mean_median_error_invalid_type[str-median]
- test_imputation_mean_median_error_invalid_type[None-mean]
- test_imputation_mean_median_error_invalid_type[str-mean]
- test_metrics_pos_label_error_str[str-brier_score_loss-True]
- test_metrics_pos_label_error_str[str-precision_recall_curve-True]
- test_metrics_pos_label_error_str[str-roc_curve-True]
- test_ohe_infrequent_two_levels_drop_infrequent_errors[drop0]
- test_ohe_infrequent_two_levels_drop_infrequent_errors[drop1]
- test_ohe_infrequent_three_levels_drop_infrequent_errors[drop0]
- test_ohe_infrequent_three_levels_drop_infrequent_errors[drop1]
- test_binary_clf_curve_implicit_pos_label[det_curve]
- test_binary_clf_curve_implicit_pos_label[precision_recall_curve]
- test_binary_clf_curve_implicit_pos_label[roc_curve]
- test_calibration_curve_pos_label_error_str[str]
- test_constant_strategy_exceptions[single-output]
- test_constant_strategy_exceptions[multi-output]
- test_compute_class_weight_not_present[numeric-class_weight1-classes1-The classes, \\[0, 1, 2, 3\\], are not in class_weight]
- test_compute_class_weight_not_present[string-class_weight4-classes4-The classes, \\['dog'\\], are not in class_weight]

COMMENT:
Closing as the build has started working again.

Labels: Needs Triage
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
I'd be +1 for that.

(Also a good issue for @StefanieSenger if we decide to do it)

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
> However, this paper is a preprint. In the final version of the paper, the SAMME.R algorithm is presented.

@glemaitre Did you mean that SAMME.R is **not** present in the published paper?

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
Yes, it is not published. I corrected my sentence.

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
Is @StefanieSenger working on this already? If not, I would be willing to take it up.:)

Labels: API, RFC, module:ensemble
RFC deprecate the SAMME.R algorithm in AdaBoostClassifier

While working on the following example (https://github.com/scikit-learn/scikit-learn/pull/26486), I found a couple of issues regarding the SAMME.R algorithm, which is the default algorithm in `AdaBoostClassifier`:

- The algorithm was implemented based on the [following paper](https://hastie.su.domains/Papers/samme.pdf). However, this paper is a preprint. In the [final version of the paper](https://www.intlpress.com/site/pub/files/_fulltext/journals/sii/2009/0002/0003/SII-2009-0002-0003-a008.pdf), the SAMME.R algorithm is not presented. So we implemented an unpublished algorithm.
- SAMME.R can show some diverging behaviour as shown in https://github.com/scikit-learn/scikit-learn/issues/20443#issuecomment-1574024873.

For these two reasons, I think that we should deprecate this algorithm and remove the parameter `algorithm` from the `AdaBoostClassifier`.

In addition, I think that we should monitor the latest work on multiclass AdaBoost, where additional theoretical founding is revealed, cf. https://proceedings.neurips.cc/paper/2021/hash/17f5e6db87929fb55cebeb7fd58c1d41-Abstract.html

COMMENT:
Hey @jpangas, I'm working on it already. :)

Labels: API, RFC, module:ensemble