Repository: scikit-learn/scikit-learn
Pull request: [MRG] Enable percentage for n_features_to_select in RFE(Fix PR #16228)
This brings PR #16228 up to date, adds tests and fixes some issues with it.

That PR was meant to Fix #14567, close #14627 and close #15529 by adding the option to select n features by percentage instead of by absolute number.

The changes PR #16228 made to `isotronic.py` have since been added already and are not included in this PR. Here I fix some bugs from PR #16228 and add tests for the new functionality.

Ping @noatamir 

Problem statement:
RFI: Setting the number of features to select 'n_features_to_select' to be percentage
While I was working on my thesis, I have used RFE and I needed to set it to select 25% of the available features. So, I calculated the target number of features before call init RFE.

I was wondering if sklearn support initiating RFE with this ability. for ex. if `n_features_to_select` is float between 0.0 and 1.0 it will be multiplied by the number of features in the training set, or using another parameter other than 'n_features_to_select'.

 Could it worth contributing to scikit-learn?

Hints from issue discussion:
I'd consider such a PR, thanks.

should it be renamed ``features_to_select``? No strong opinion though.
It's still about the number of features, just a different unit of
measurement

@jnothman I would love to work on this. Can you guide me on how I could go about this?
I was thinking to introduce a new attribute `percentage_features_to_select` ? Then I will have to tackle the issue of whether or not to allow a user to give both  `percentage_features_to_select` and `n_features_to_select`? I haven't thought it through fully, but can you tell me if I am on the right path.
I'm happy for n_features_to_select to have fraction semantics when less
than 1.

Commit message: BUG Fixes issues

Patch to review:
diff --git a/doc/whats_new/v0.24.rst b/doc/whats_new/v0.24.rst
index 7c2b6af86b815..cce62f12345a0 100644
--- a/doc/whats_new/v0.24.rst
+++ b/doc/whats_new/v0.24.rst
@@ -69,6 +69,11 @@ Changelog
   attribute name/path or a `callable` for extracting feature importance from
   the estimator.  :pr:`15361` by :user:`Venkatachalam N <venkyyuvy>`
 
+- |Enhancement| Added the option for the number of n_features_to_select to be
+  given as a float representing the percentage of features to select.
+  :pr:`17090` by :user:`Lisa Schwetlick <lschwetlick>` and
+  :user:`Marija Vlajic Wheeler <marijavlajic>`.
+
 :mod:`sklearn.impute`
 .....................
 
@@ -149,6 +154,7 @@ Changelog
   :pr:`17233` by :user:`Zolisa Bleki <zoj613>`.
 
 
+
 Code and Documentation Contributors
 -----------------------------------
 
diff --git a/sklearn/feature_selection/_rfe.py b/sklearn/feature_selection/_rfe.py
index 8af8d423b612f..ce6b415d6a525 100644
--- a/sklearn/feature_selection/_rfe.py
+++ b/sklearn/feature_selection/_rfe.py
@@ -7,6 +7,7 @@
 """Recursive feature elimination for feature ranking"""
 
 import numpy as np
+import numbers
 from joblib import Parallel, delayed, effective_n_jobs
 
 from ..utils.metaestimators import if_delegate_has_method
@@ -57,9 +58,13 @@ class RFE(SelectorMixin, MetaEstimatorMixin, BaseEstimator):
         information about feature importance
         (e.g. `coef_`, `feature_importances_`).
 
-    n_features_to_select : int or None, default=None
-        The number of features to select. If `None`, half of the features
-        are selected.
+    n_features_to_select : int, float, or None (default=None)
+        The number of features to select.
+        If `None`, half of the features are selected.
+        If integer, the parameter is the absolute number of features to select.
+        If float between 0 and 1, it is the fraction of features to select.
+        If float larger than 1, it is converted to integer and represents the
+        absolute number of features to select.
 
     step : int or float, default=1
         If greater than or equal to 1, then ``step`` corresponds to the
@@ -185,8 +190,12 @@ def _fit(self, X, y, step_score=None):
         n_features = X.shape[1]
         if self.n_features_to_select is None:
             n_features_to_select = n_features // 2
-        else:
+        elif self.n_features_to_select < 0:
+            raise ValueError("n_features_to_select must be positive or None")
+        elif isinstance(self.n_features_to_select, numbers.Integral):
             n_features_to_select = self.n_features_to_select
+        else:  # float
+            n_features_to_select = int(n_features * self.n_features_to_select)
 
         if 0.0 < self.step < 1.0:
             step = int(max(1, self.step * n_features))
diff --git a/sklearn/feature_selection/tests/test_rfe.py b/sklearn/feature_selection/tests/test_rfe.py
index 970ba7c0b71b8..d3770da1ccea8 100644
--- a/sklearn/feature_selection/tests/test_rfe.py
+++ b/sklearn/feature_selection/tests/test_rfe.py
@@ -109,6 +109,35 @@ def test_rfe():
     assert_array_almost_equal(X_r, X_r_sparse.toarray())
 
 
+def test_rfe_negative_n_features():
+    clf = SVC(kernel="linear")
+
+    iris = load_iris()
+    rfe = RFE(estimator=clf, n_features_to_select=-1, step=0.1)
+    msg = "n_features_to_select must be positive or None"
+    with pytest.raises(ValueError, match=msg):
+        rfe.fit(iris.data, iris.target)
+
+
+def test_rfe_percent_n_features():
+    # test that the results are the same
+    generator = check_random_state(0)
+    iris = load_iris()
+    X = np.c_[iris.data, generator.normal(size=(len(iris.data), 6))]
+    y = iris.target
+    # there are 10 features in the data. We select half.
+    clf = SVC(kernel="linear")
+    rfe_num = RFE(estimator=clf, n_features_to_select=5, step=0.1)
+    rfe_num.fit(X, y)
+
+    rfe_perc = RFE(estimator=clf, n_features_to_select=0.5, step=0.1)
+    rfe_perc.fit(X, y)
+
+    assert all(rfe_perc.ranking_ == rfe_num.ranking_)
+    assert (rfe_perc.n_features_ == rfe_num.n_features_)
+    assert all(rfe_perc.support_ == rfe_num.support_)
+
+
 def test_rfe_mockclassifier():
     generator = check_random_state(0)
     iris = load_iris()


Code review comments:
diff_hunk: @@ -57,9 +58,13 @@ class RFE(SelectorMixin, MetaEstimatorMixin, BaseEstimator):
         information about feature importance
         (e.g. `coef_`, `feature_importances_`).
 
-    n_features_to_select : int or None, default=None
-        The number of features to select. If `None`, half of the features
-        are selected.
+    n_features_to_select : int, float, or None (default=None)
+        The number of features to select.
+        If `None`, half of the features are selected.
+        If integer, the parameter is the absolute number of features to select.
+        If float between 0 and 1, it is the fraction of features to select.
+        If float larger than 1, it is converted to integer and represents the
+        absolute number of features to select.
path: sklearn/feature_selection/_rfe.py
text: @user1:
I think we should error in these cases. This is consistent with the behavior we have in trees and `min_samples_split`.

diff_hunk: @@ -185,8 +190,12 @@ def _fit(self, X, y, step_score=None):
         n_features = X.shape[1]
         if self.n_features_to_select is None:
             n_features_to_select = n_features // 2
-        else:
+        elif self.n_features_to_select < 0:
+            raise ValueError("n_features_to_select must be positive or None")
+        elif isinstance(self.n_features_to_select, numbers.Integral):
             n_features_to_select = self.n_features_to_select
+        else:  # float
+            n_features_to_select = int(n_features * self.n_features_to_select)
path: sklearn/feature_selection/_rfe.py
text: @user1:
At this point, it would be good to check for if the float is > 1.0 and error in those cases.