Repository: scikit-learn/scikit-learn
Pull request: ENH Allow `SelectFromModel`'s `max_features` to accept callables
#### Reference Issues/PRs
Fixes #21117

#### What does this implement/fix? Explain your changes.
This PR improves `SelectFromModel`'s `max_features` by allowing it to:
- Accept integral values to preserve prior behaviour
- Accept real values to prescribe a _proportion_ of features to allow
- Accepts a callable to return an integral value for the number of features to allow

To accomplish this, I introduced a new private attribute `max_features_` which is calculated in `fit` based on what type was received for `max_features`. The calculations for integral, and callable values, e.g.:
- `max_features_=max_features`
- `max_features_=round(max_features*X.shape[1])`
- `max_features_=max_features(X)`

I then augmented existing unit tests to account for these new cases
#### Any other comments?

Problem statement:
SelectFromModel: max_features can't be greater than number of features
### Describe the bug

When I define a `SelectFromModel` instance like here:

```python
SelectFromModel(RandomForestClassifier(), max_features=100)
```

and the number of total features is less than 100, then a `ValueError` is raised:

```pytb
ValueError: 'max_features' should be 0 and 10 features.Got 100 instead.
```

I consider this a bug as for example in my pipeline this feature selector is preceded by other feature selectors like low `VarianceThreshold`. Point being it is never known how many features will be left when this point is reached. If the value is bigger than available features it should just keep all of them and not throw an error.

### Steps/Code to Reproduce

```python
from sklearn.datasets import make_classification
from sklearn.feature_selection import SelectFromModel
from sklearn.ensemble import RandomForestClassifier

x,y = make_classification(n_features=10, n_informative=8)
sfm = SelectFromModel(RandomForestClassifier(), max_features=100)
sfm.fit(x,y)
```

### Expected Results

If the value is bigger than available features it should just keep all of them and not throw an error.

### Actual Results

```pytb
ValueError: 'max_features' should be 0 and 10 features.Got 100 instead.
```

### Versions

0.23.2
0.24.1

Hints from issue discussion:
I don't think this is a bug. the idea is that the developer knows exactly how many features come out of `SelectFromModel`. If you want, you can write your own `SelectMaxFeaturesFromModel` meta-estimator, which wraps around `SelectFromModel`, and selects max those number of features.
I agree with @kienerj that the current behavior is not very convenient. The number of features in the upstream of the pipeline is not always very stable, e.g. if you tweak the `min_df` / `max_df` parameters of a text vectorizer in a grid search for instance.

I would be in favor of accepting a PR to avoid raising an error when `max_features > X.shape[1]` an select all features instead.
If we're adding this, we can add this via an extra parameter which says "feature_count_validation" or something, and only accept if the user explicitly says they are happy to not raise.
What do you think about supporting floats to specify a fraction of features and keeping the validation for ints? (This would be a similar API to random forest's `max_features`.)
> 
> 
> What do you think about supporting floats to specify a fraction of features and keeping the validation for ints? (This would be a similar API to random forest's `max_features`.)

It would be a nice addition but right now for my use-case not needed.

But I still think my initial point is valid because I absolutely fail to see why an error must be raised if max_features is > than available features. It's a good way to limit model complexity and limit amount of features to an upper bound like when you don't have that much data compared to number of features. Then I set it to say 1/5 of your number of rows (= an absolute value).

To expand further there is extra code to make this check and raise the error. Why is this extra code needed? Removing it wouldn't break any expectations or existing workflows and make the code simpler. For me this code just add a hassle to me as end-user with no apparent advanatge, Maybe seeing why this need to be checked would make me reconsider my point of view.
> To expand further there is extra code to make this check and raise the error. Why is this extra code needed? Removing it wouldn't break any expectations or existing workflows and make the code simpler. For me this code just add a hassle to me as end-user with no apparent advanatge, Maybe seeing why this need to be checked would make me reconsider my point of view.

What it brings is consistency in the pipeline. I understand that's not what you want in your pipeline, but it is something which people rely on.

> What do you think about supporting floats to specify a fraction of features and keeping the validation for ints? (This would be a similar API to random forest's `max_features`.)

I'm not sure if that'd solve the problem. `min(n_cols(X), max_features)` is different than `n_cols(X) * fraction`.

On the other hand, `max_features` is called `max_features`, and not `n_features`. So we could argue that it's supposed to behave as it's called.

I'm wondering what @jnothman thinks here.
> It's a good way to limit model complexity and limit amount of features to an upper bound like when you don't have that much data compared to number of features. 

I agree this is a good use case for allowing `max_features` to be greater than the number of features.

> To expand further there is extra code to make this check and raise the error. Why is this extra code needed? Removing it wouldn't break any expectations or existing workflows and make the code simpler. 

Here is a workflow that may take longer if we do not error: When searching through `max_features` in a `RandomSearchCV`, one may depend on the error raised for "`max_features` > number of features" so the search does not spend time looking through values of `max_features` that would produce the same result. 
What about accepting a callable that would take the X and should return an integer.

```python
SelectFromModel(RandomForestClassifier(), max_features=lambda X: min(X.shape[0], 100))
```

I still think that adding support to a float could be nice because this is a common API in other estimators and one would expect to have it there as well.

Gave an initial implementation in PR #22356 if anyone is interested in taking a look!

Commit message: Added a sample callable in `max_features` description

Patch to review:
diff --git a/doc/whats_new/v1.1.rst b/doc/whats_new/v1.1.rst
index 4c307d9e54250..c71cf9b545e24 100644
--- a/doc/whats_new/v1.1.rst
+++ b/doc/whats_new/v1.1.rst
@@ -264,6 +264,14 @@ Changelog
 - |Efficiency| Improve runtime performance of :func:`feature_selection.chi2`
   with boolean arrays. :pr:`22235` by `Thomas Fan`_.
 
+- |Feature| Added the ability to pass floats and callables to the `max_features`
+  parameter of :class:`feature_selection.SelectFromModel`. Also introduced new
+  attribute `max_features_` which is inferred from `max_features` and the data
+  during `fit`. If `max_features` is integral, then `max_features_=max_features`.
+  If `max_features` is a float, then `max_features_=round(max_features*X.shape[1])`.
+  If `max_features` is a callable, then `max_features_=max_features(X)`.
+  :pr:`22356` by :user:`Meekail Zain <micky774>`
+
 :mod:`sklearn.feature_selection`
 ................................
 - |Efficiency| Reduced memory usage of :func:`feature_selection.chi2`.
diff --git a/sklearn/feature_selection/_from_model.py b/sklearn/feature_selection/_from_model.py
index 4ebc351eca085..f211b49e82a5f 100644
--- a/sklearn/feature_selection/_from_model.py
+++ b/sklearn/feature_selection/_from_model.py
@@ -1,6 +1,7 @@
 # Authors: Gilles Louppe, Mathieu Blondel, Maheshakya Wijewardena
 # License: BSD 3 clause
 
+import inspect
 import numpy as np
 import numbers
 
@@ -99,8 +100,15 @@ class SelectFromModel(MetaEstimatorMixin, SelectorMixin, BaseEstimator):
         ``threshold`` in the case where the ``coef_`` attribute of the
         estimator is of dimension 2.
 
-    max_features : int, default=None
-        The maximum number of features to select.
+    max_features : int, callable, default=None
+        The maximum number of features to select. ``max_features`` may be a
+        whole number representing the number of maximum features to allow,
+        or a callable that takes only ``X`` as an argument and returns the
+        number of maximum features to allow. When ``max_features`` is a
+        callable, the maximum number of features is calculated as
+        ``max_features_=max_features(X)``. For example, if one wanted to
+        select only up to 50% of the incoming features, they may set
+        ``max_features=lambda X: round(X.shape[1]/2)``.
         To only select based on ``max_features``, set ``threshold=-np.inf``.
 
         .. versionadded:: 0.20
@@ -135,6 +143,13 @@ class SelectFromModel(MetaEstimatorMixin, SelectorMixin, BaseEstimator):
 
         .. versionadded:: 0.24
 
+    max_features_ : int
+        Maximum number of features calculated during :term:`fit`. Only defined
+        if the ``max_features`` is not none. For details on how
+        ``max_features_`` is calculated, see ``max_features``.
+
+        .. versionadded:: 1.1
+
     feature_names_in_ : ndarray of shape (`n_features_in_`,)
         Names of features seen during :term:`fit`. Defined only when `X`
         has feature names that are all strings.
@@ -218,7 +233,7 @@ def _get_support_mask(self):
         if self.max_features is not None:
             mask = np.zeros_like(scores, dtype=bool)
             candidate_indices = np.argsort(-scores, kind="mergesort")[
-                : self.max_features
+                : self.max_features_
             ]
             mask[candidate_indices] = True
         else:
@@ -247,18 +262,32 @@ def fit(self, X, y=None, **fit_params):
             Fitted estimator.
         """
         if self.max_features is not None:
-            if not isinstance(self.max_features, numbers.Integral):
-                raise TypeError(
-                    "'max_features' should be an integer between"
-                    " 0 and {} features. Got {!r} instead.".format(
-                        X.shape[1], self.max_features
+            if isinstance(self.max_features, numbers.Integral):
+                if self.max_features < 0 or self.max_features > X.shape[1]:
+                    raise ValueError(
+                        "When using an integral value, 'max_features' should be"
+                        f" between 0 and {X.shape[1]}. Got {self.max_features} instead."
                     )
-                )
-            elif self.max_features < 0 or self.max_features > X.shape[1]:
-                raise ValueError(
-                    "'max_features' should be 0 and {} features.Got {} instead.".format(
-                        X.shape[1], self.max_features
+                self.max_features_ = self.max_features
+            elif callable(self.max_features):
+                try:
+                    self.max_features_ = self.max_features(X)
+                    if not isinstance(self.max_features_, numbers.Integral):
+                        raise ValueError(
+                            "When `max_features` is a callable, it must return an"
+                            " integral value (e.g. `int`)"
+                        )
+                except TypeError:
+                    raise TypeError(
+                        "When 'max_features' is a callable, it must take only 'X' as"
+                        " input -- you passed a function with signature"
+                        f" {inspect.signature(self.max_features)}"
                     )
+            else:
+                raise TypeError(
+                    "'max_features' must be either an integral value or a"
+                    " callable that takes 'X' as input. Got"
+                    f" {self.max_features} instead."
                 )
 
         if self.prefit:
diff --git a/sklearn/feature_selection/tests/test_from_model.py b/sklearn/feature_selection/tests/test_from_model.py
index 854f2c3cac615..4702131f74503 100644
--- a/sklearn/feature_selection/tests/test_from_model.py
+++ b/sklearn/feature_selection/tests/test_from_model.py
@@ -1,5 +1,6 @@
 import pytest
 import numpy as np
+from unittest.mock import Mock
 
 from sklearn.utils._testing import assert_array_almost_equal
 from sklearn.utils._testing import assert_array_equal
@@ -60,10 +61,37 @@ def test_input_estimator_unchanged():
 @pytest.mark.parametrize(
     "max_features, err_type, err_msg",
     [
-        (-1, ValueError, "'max_features' should be 0 and"),
-        (data.shape[1] + 1, ValueError, "'max_features' should be 0 and"),
-        ("gobbledigook", TypeError, "should be an integer"),
-        ("all", TypeError, "should be an integer"),
+        (-1, ValueError, "When using an integral value, 'max_features' should be"),
+        (
+            lambda: 10,
+            TypeError,
+            "When 'max_features' is a callable, it must take only 'X' as",
+        ),
+        (
+            lambda X: 0.9,
+            ValueError,
+            "When `max_features` is a callable, it must return an integral",
+        ),
+        (
+            lambda X, y: 10,
+            TypeError,
+            "When 'max_features' is a callable, it must take only 'X' as",
+        ),
+        (
+            data.shape[1] + 1,
+            ValueError,
+            "When using an integral value, 'max_features' should be",
+        ),
+        (
+            "gobbledigook",
+            TypeError,
+            "'max_features' must be either an integral value or a",
+        ),
+        (
+            "all",
+            TypeError,
+            "'max_features' must be either an integral value or a",
+        ),
     ],
 )
 def test_max_features_error(max_features, err_type, err_msg):
@@ -77,13 +105,62 @@ def test_max_features_error(max_features, err_type, err_msg):
 
 
 @pytest.mark.parametrize("max_features", [0, 2, data.shape[1]])
-def test_max_features_dim(max_features):
+def test_inferred_max_features_integer(max_features):
+    clf = RandomForestClassifier(n_estimators=50, random_state=0)
+    transformer = SelectFromModel(
+        estimator=clf, max_features=max_features, threshold=-np.inf
+    )
+    transformer.fit_transform(data, y)
+    assert transformer.max_features_ == max_features
+
+
+@pytest.mark.parametrize(
+    "max_features",
+    [lambda X: 1, lambda X: X.shape[1], lambda X: min(X.shape[1], 10000)],
+)
+def test_inferred_max_features_callable(max_features):
+    clf = RandomForestClassifier(n_estimators=50, random_state=0)
+    transformer = SelectFromModel(
+        estimator=clf, max_features=max_features, threshold=-np.inf
+    )
+    transformer.fit_transform(data, y)
+    assert transformer.max_features_ == max_features(data)
+
+
+@pytest.mark.parametrize("max_features", [0, 2, data.shape[1]])
+def test_output_shape_max_features_integers(max_features):
+    clf = RandomForestClassifier(n_estimators=50, random_state=0)
+    transformer = SelectFromModel(
+        estimator=clf, max_features=max_features, threshold=-np.inf
+    )
+    X_trans = transformer.fit_transform(data, y)
+    assert X_trans.shape[1] == transformer.max_features_
+
+
+@pytest.mark.parametrize(
+    "max_features",
+    [lambda X: 1, lambda X: X.shape[1], lambda X: min(X.shape[1], 10000)],
+)
+def test_output_shape_max_features_callable(max_features):
     clf = RandomForestClassifier(n_estimators=50, random_state=0)
     transformer = SelectFromModel(
         estimator=clf, max_features=max_features, threshold=-np.inf
     )
     X_trans = transformer.fit_transform(data, y)
-    assert X_trans.shape[1] == max_features
+    assert X_trans.shape[1] == transformer.max_features_
+
+
+@pytest.mark.parametrize(
+    "max_features",
+    [lambda X: min(X.shape[1], 10000), lambda X: X.shape[1], lambda X: 1],
+)
+def test_max_features_callable_data(max_features):
+    """Tests that the callable passed to `fit` is called on X"""
+    clf = RandomForestClassifier(n_estimators=50, random_state=0)
+    m = Mock(side_effect=max_features)
+    transformer = SelectFromModel(estimator=clf, max_features=m, threshold=-np.inf)
+    transformer.fit_transform(data, y)
+    m.assert_called_with(data)
 
 
 class FixedImportanceEstimator(BaseEstimator):


Code review comments:
diff_hunk: @@ -247,18 +262,32 @@ def fit(self, X, y=None, **fit_params):
             Fitted estimator.
         """
         if self.max_features is not None:
-            if not isinstance(self.max_features, numbers.Integral):
-                raise TypeError(
-                    "'max_features' should be an integer between"
-                    " 0 and {} features. Got {!r} instead.".format(
-                        X.shape[1], self.max_features
+            if isinstance(self.max_features, numbers.Integral):
+                if self.max_features < 0 or self.max_features > X.shape[1]:
+                    raise ValueError(
+                        "When using an integral value, 'max_features' should be"
+                        f" between 0 and {X.shape[1]}. Got {self.max_features} instead."
                     )
-                )
-            elif self.max_features < 0 or self.max_features > X.shape[1]:
-                raise ValueError(
-                    "'max_features' should be 0 and {} features.Got {} instead.".format(
-                        X.shape[1], self.max_features
+                self.max_features_ = self.max_features
+            elif callable(self.max_features):
+                try:
+                    self.max_features_ = self.max_features(X)
+                    if not isinstance(self.max_features_, numbers.Integral):
+                        raise ValueError(
+                            "When `max_features` is a callable, it must return an"
+                            " integral value (e.g. `int`)"
path: sklearn/feature_selection/_from_model.py
text: @user1:
We can use "int". Most do not know what an `numbers.Integral` is:

```suggestion
                            f" int. Got {self.max_features_}"
```

diff_hunk: @@ -135,6 +143,13 @@ class SelectFromModel(MetaEstimatorMixin, SelectorMixin, BaseEstimator):
 
         .. versionadded:: 0.24
 
+    max_features_ : int
+        Maximum number of features calculated during :term:`fit`. Only defined
+        if the ``max_features`` is not none. For details on how
path: sklearn/feature_selection/_from_model.py
text: @user1:
```suggestion
        if the ``max_features`` is not `None`. For details on how
```

diff_hunk: @@ -264,6 +264,14 @@ Changelog
 - |Efficiency| Improve runtime performance of :func:`feature_selection.chi2`
   with boolean arrays. :pr:`22235` by `Thomas Fan`_.
 
+- |Feature| Added the ability to pass floats and callables to the `max_features`
+  parameter of :class:`feature_selection.SelectFromModel`. Also introduced new
+  attribute `max_features_` which is inferred from `max_features` and the data
+  during `fit`. If `max_features` is integral, then `max_features_=max_features`.
+  If `max_features` is a float, then `max_features_=round(max_features*X.shape[1])`.
+  If `max_features` is a callable, then `max_features_=max_features(X)`.
path: doc/whats_new/v1.1.rst
text: @user1:
```suggestion
  If `max_features` is a callable, then `max_features_ = max_features(X)`.
```

diff_hunk: @@ -99,8 +100,15 @@ class SelectFromModel(MetaEstimatorMixin, SelectorMixin, BaseEstimator):
         ``threshold`` in the case where the ``coef_`` attribute of the
         estimator is of dimension 2.
 
-    max_features : int, default=None
-        The maximum number of features to select.
+    max_features : int, callable, default=None
+        The maximum number of features to select. ``max_features`` may be a
+        whole number representing the number of maximum features to allow,
+        or a callable that takes only ``X`` as an argument and returns the
+        number of maximum features to allow. When ``max_features`` is a
+        callable, the maximum number of features is calculated as
+        ``max_features_=max_features(X)``. For example, if one wanted to
path: sklearn/feature_selection/_from_model.py
text: @user1:
We do not need to describe `max_features_` here. We need to describe what the input of the callable is.

diff_hunk: @@ -264,6 +264,14 @@ Changelog
 - |Efficiency| Improve runtime performance of :func:`feature_selection.chi2`
   with boolean arrays. :pr:`22235` by `Thomas Fan`_.
 
+- |Feature| Added the ability to pass floats and callables to the `max_features`
+  parameter of :class:`feature_selection.SelectFromModel`. Also introduced new
+  attribute `max_features_` which is inferred from `max_features` and the data
+  during `fit`. If `max_features` is integral, then `max_features_=max_features`.
path: doc/whats_new/v1.1.rst
text: @user1:
```suggestion
  during `fit`. If `max_features` is a int, then `max_features_ = max_features`.
```