Repository: scikit-learn/scikit-learn
Pull request: RFE and RFECV allow features_to_select to be larger than available
fixes #28725

Implemented a warning message for when the RFE or the RFECV objects are initialized with a n_features_to_select or min_features_to_select (respectively) attribute larger than the number of features present in the X variable that is passed to the fit method.

Additionally, a new test function has been added to ensure the correct warning message is raised in these scenarios. The test comprehensively covers both cases and validates that the warning is emitted as expected.


Problem statement:
RFE and RFECV allow features_to_select to be larger than available features
### Describe the bug

If the `RFE` or the `RFECV` objects are initialized with a `n_features_to_select` or a `min_features_to_select` (respectively) attribute larger than the number of features present in the `X` variable that is passed to the `fit` method, I would expect an error to be raised. However a result is returned where `n_features` is equal to the number of features in `X`.

### Steps/Code to Reproduce
For the `RFE` class:
```python
from sklearn.feature_selection import RFE
from sklearn.linear_model import LogisticRegression
from sklearn.datasets import make_classification
from pandas import DataFrame

X, y = make_classification(n_samples=1000, n_features=20, n_redundant=0, n_classes=2, random_state=0)

rfe = RFE(
    estimator=LogisticRegression(random_state=0),
    n_features_to_select=21,
    step=2,
)

rfe.fit(X=X, y=y)

print(rfe.n_features_)
print(rfe.ranking_)
```

For the `RFECV` class:
```python
from sklearn.feature_selection import RFECV
from sklearn.linear_model import LogisticRegression
from sklearn.datasets import make_classification
from pandas import DataFrame

X, y = make_classification(n_samples=1000, n_features=20, n_redundant=0, n_classes=2, random_state=0)

rfecv = RFECV(
    estimator=LogisticRegression(random_state=0),
    min_features_to_select=21,
    cv=2,
    step=2,
    scoring="precision"
)
rfecv.fit(X=X, y=y)

print(DataFrame(rfecv.cv_results_))
```


### Expected Results

Expected that an exception is raised stating that the `n_features_to_select` or `min_features_to_select` variables cannot be greater than the number of available features. The downside is that if we raise an error we can break someone's code that was unintentionally passing a number o features larger than the available ones. So would it be best to raise a warning instead?

### Actual Results

A result is computed where `n_features` is equal to the number of features in `X`.

### Versions

```shell
System:
    python: 3.9.18 (main, Mar 17 2024, 15:49:30)  [GCC 9.4.0]
executable: /home/miguel/repos/scikit-learn/sklearn-env/bin/python
   machine: Linux-5.15.146.1-microsoft-standard-WSL2-x86_64-with-glibc2.31

Python dependencies:
      sklearn: 1.5.dev0
          pip: 23.0.1
   setuptools: 58.1.0
        numpy: 1.26.4
        scipy: 1.12.0
       Cython: 3.0.9
       pandas: 2.2.1
   matplotlib: 3.8.3
       joblib: 1.3.2
threadpoolctl: 3.3.0

Built with OpenMP: True

threadpoolctl info:
       user_api: blas
   internal_api: openblas
    num_threads: 8
         prefix: libopenblas
       filepath: /home/miguel/repos/scikit-learn/sklearn-env/lib/python3.9/site-packages/numpy.libs/libopenblas64_p-r0-0cf96a72.3.23.dev.so
        version: 0.3.23.dev
threading_layer: pthreads
   architecture: Haswell

       user_api: blas
   internal_api: openblas
    num_threads: 8
         prefix: libopenblas
       filepath: /home/miguel/repos/scikit-learn/sklearn-env/lib/python3.9/site-packages/scipy.libs/libopenblasp-r0-23e5df77.3.21.dev.so
        version: 0.3.21.dev
threading_layer: pthreads
   architecture: Haswell

       user_api: openmp
   internal_api: openmp
    num_threads: 8
         prefix: libgomp
       filepath: /usr/lib/x86_64-linux-gnu/libgomp.so.1.0.0
        version: None
```

Hints from issue discussion:
Hi @miguelcsilva, I agree that the behavior is unclear. I'm not sure if we want an error or a warning. I sligthly tend towards a warning. We also need to explain the behavior in the parameter descriptions.

ping @glemaitre, wdyt ?
Can I work on this issue? @ogrisel @jeremiedbb 
> Can I work on this issue? @ogrisel @jeremiedbb 

Hi @SarthakNikhal, I haven't yet started to work on this, so feel free to take the issue. Also feel free to ping me if something in the description isn't clear, etc.
Hello @SarthakNikhal. Since you haven't said anything since yesterday is it ok if I take this issue?

Commit message: RFE and RFECV allow features_to_select to be larger than available features (scikit-learn#28725)

Implemented a warning message for when the RFE or the RFECV objects are initialized with a n_features_to_select or min_features_to_select (respectively) attribute larger than the number of features present in the X variable that is passed to the fit method.

Additionally, a new test function has been added to ensure the correct warning message is raised in these scenarios. The test comprehensively covers both cases and validates that the warning is emitted as expected.

Patch to review:
diff --git a/sklearn/feature_selection/_rfe.py b/sklearn/feature_selection/_rfe.py
index 44764655e988d..85dbb686ac538 100644
--- a/sklearn/feature_selection/_rfe.py
+++ b/sklearn/feature_selection/_rfe.py
@@ -11,6 +11,7 @@
 import numpy as np
 from joblib import effective_n_jobs
 
+import warnings
 from ..base import BaseEstimator, MetaEstimatorMixin, _fit_context, clone, is_classifier
 from ..metrics import check_scoring
 from ..model_selection import check_cv
@@ -289,6 +290,15 @@ def _fit(self, X, y, step_score=None, **fit_params):
         else:  # float
             n_features_to_select = int(n_features * self.n_features_to_select)
 
+        if n_features_to_select > n_features:
+            warnings.warn(
+                (
+                    "features_to_select shouldn't be greater than the number of"
+                    " available features"
+                ),
+                UserWarning,
+            )
+
         if 0.0 < self.step < 1.0:
             step = int(max(1, self.step * n_features))
         else:
diff --git a/sklearn/feature_selection/tests/test_rfe.py b/sklearn/feature_selection/tests/test_rfe.py
index 01c6194493ab6..e0137a7e530f1 100644
--- a/sklearn/feature_selection/tests/test_rfe.py
+++ b/sklearn/feature_selection/tests/test_rfe.py
@@ -24,6 +24,7 @@
 from sklearn.utils import check_random_state
 from sklearn.utils._testing import ignore_warnings
 from sklearn.utils.fixes import CSR_CONTAINERS
+import warnings
 
 
 class MockClassifier:
@@ -649,3 +650,46 @@ def test_rfe_estimator_attribute_error():
         rfe.fit(iris.data, iris.target).decision_function(iris.data)
     assert isinstance(exec_info.value.__cause__, AttributeError)
     assert inner_msg in str(exec_info.value.__cause__)
+
+
+@pytest.mark.parametrize("ClsRFE", [RFE, RFECV])
+def test_rfe_n_features_to_select_warning(ClsRFE):
+    """Check if the correct warning is raised when trying to initialize a RFE
+    object with a n_features_to_select attribute larger than the number of
+    features present in the X variable that is passed to the fit method
+
+    Non-regression test for:
+    https://github.com/scikit-learn/scikit-learn/issues/28725
+    """
+    X, y = make_classification(
+        n_samples=1000, n_features=20, n_redundant=0, n_classes=2, random_state=0
+    )
+
+    # Catch the warning
+    with warnings.catch_warnings(record=True) as w:
+        # Determine the correct parameter name based on the estimator
+        if issubclass(ClsRFE, RFECV):
+            parameter_name = "min_features_to_select"
+        elif issubclass(ClsRFE, RFE):
+            parameter_name = "n_features_to_select"
+        else:
+            raise ValueError("Unsupported estimator")
+
+        # Create RFE/RFECV with n_features_to_select/min_features_to_select
+        # larger than the number of features present in the X variable
+        rfe = ClsRFE(
+            estimator=LogisticRegression(random_state=0),
+            **{parameter_name: 21},
+            step=2,
+        )
+        rfe.fit(X=X, y=y)
+
+        # Check if any UserWarning was raised
+        assert any(issubclass(warning.category, UserWarning) for warning in w)
+        # Check if the warning message is correct
+        assert any(
+            "features_to_select shouldn't be greater than the number of available"
+            " features"
+            in str(warning.message)
+            for warning in w
+        )


Code review comments:
diff_hunk: @@ -289,6 +290,15 @@ def _fit(self, X, y, step_score=None, **fit_params):
         else:  # float
             n_features_to_select = int(n_features * self.n_features_to_select)
 
+        if n_features_to_select > n_features:
+            warnings.warn(
+                (
+                    "features_to_select shouldn't be greater than the number of"
+                    " available features"
path: sklearn/feature_selection/_rfe.py
text: @user1:
We need to mention the actual behavior, what about something similar to

`n_features_to_select must not exceed available features. Falling back to select all n_features.`

diff_hunk: @@ -289,6 +290,15 @@ def _fit(self, X, y, step_score=None, **fit_params):
         else:  # float
             n_features_to_select = int(n_features * self.n_features_to_select)
 
+        if n_features_to_select > n_features:
path: sklearn/feature_selection/_rfe.py
text: @user1:
I would also put this inside the scenario where `n_features_to_select` is already an integer.

@author:
I refrained from doing this because the fit method of this class already invokes the fit method of the RFE class. Do you want me to do it anyways?

@user1:
> Do you want me to do it anyways?

Yes, please. You can use a context manager to silence the warning when calling the RFE object inside the RFECV. Else the user would get the message repeated as many times as the number of cv splits, which can become very verbose really fast.

The other advantage is related to @user2's  https://github.com/scikit-learn/scikit-learn/pull/28764#discussion_r1553267276, as I would rather have the warning message to refer the correct hyperparameter `min_features_to_select` in `RFECV`. 

@user2:
> Else the user would get the message repeated as many times as the number of cv splits, which can become very verbose really fast.

Indeed, I missed that. Then we should definitely raise the warning directly in RFECV and pass the curated value to RFE to not trigger the RFE warning.