Repository: scikit-learn/scikit-learn
Pull request: Sequential feature selection - unsupervised learning
<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/main/CONTRIBUTING.md
-->

#### Reference Issues/PRs
Fixes #19538 .
<!--
Example: Fixes #1234. See also #3456.
Please use keywords (e.g., Fixes) to create link to the issues or pull requests
you resolved, so that they will automatically be closed when your pull request
is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests
-->


#### What does this implement/fix? Explain your changes.
Modified the `fit` signature to default to `y=None`, modified the `validate_data` function call accordingly, changed the docstring, & added SFS test cases to demonstrate usability. For cases such as KMeans clustering and other unsupervised learning methods that do not require `y` validation, `fit` will no longer produce an error. 

#### Any other comments?
Please let me know if there are any fixes or suggestions, I am fairly new at this and would be more than happy to go back and work on it :)

<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
Sequential forward selection - unsupervised fit_transform bug


#### Describe the bug
<!--
A clear and concise description of what the bug is.
-->
When trying to use Sequential Forward Selection in a unsupervised setting with no target labels y an error results when trying to follow the documentation

Example:
```python
from sklearn.feature_selection import SequentialFeatureSelector as SFS
from sklearn.cluster import KMeans

data = np.array([[1,1,1,1,1], [2,2,2,2,2], [3,3,3,3,3], [4,4,4,4,4], [5,5,5,5,5], [6,6,6,6,6]])
model = KMeans(3)
sfs = SFS(model, n_features_to_select=3)
data = sfs.fit_transform(data)
```

#### Expected Results
<!-- Example: No error is thrown. Please paste or describe the expected results.-->
data should be in new shape -> data.shape == (6,3)

#### Actual Results
TypeError: fit() missing 1 required positional argument: 'y'

#### Versions
<!--
Please run the following snippet and paste the output below.
For scikit-learn >= 0.20:
import sklearn; sklearn.show_versions()
For scikit-learn < 0.20:
import platform; print(platform.platform())
import sys; print("Python", sys.version)
import numpy; print("NumPy", numpy.__version__)
import scipy; print("SciPy", scipy.__version__)
import sklearn; print("Scikit-Learn", sklearn.__version__)
import imblearn; print("Imbalanced-Learn", imblearn.__version__)
-->
System:
    python: 3.8.3 (default, Jul  2 2020, 16:21:59)  [GCC 7.3.0]
executable: /home/USERNAME/anaconda3/bin/python3
   machine: Linux-5.8.0-43-generic-x86_64-with-glibc2.10

Python dependencies:
          pip: 21.0.1
   setuptools: 46.4.0
      sklearn: 0.24.1
        numpy: 1.19.2
        scipy: 1.5.4
       Cython: 0.29.21
       pandas: 1.1.4
   matplotlib: 3.3.4
       joblib: 0.17.0
threadpoolctl: 2.1.0


<!-- Thanks for contributing! -->

Hints from issue discussion:
I agree, `SequentialFeatureSelector` is validating `y` when it need not place any constraints or validation on `y`, including allowing it to be None.

A pull request fixing this is very welcome. It would require an update to the docstrings, tests, `fit` signature and call to `_validate_data`.
Can I take this issue? Could get working on a PR right now.
@ShyamDesai please do! :) You can comment "Take" in the issue and it will be automatically assigned to you.
Take
As far as I'm concerned it should not validate y in any case. Its only job
with regard to y is to pass it on to the underlying estimator (and to pass
on the absence of y if y=None).

Commit message: test file lint fix

Patch to review:
diff --git a/doc/whats_new/v1.0.rst b/doc/whats_new/v1.0.rst
index 3e36438dda095..8a8603c90b932 100644
--- a/doc/whats_new/v1.0.rst
+++ b/doc/whats_new/v1.0.rst
@@ -207,6 +207,14 @@ Changelog
   for non-English characters. :pr:`18959` by :user:`Zero <Zeroto521>`
   and :user:`wstates <wstates>`.
 
+:mod:`sklearn.feature_selection`
+.......................
+
+- |Fix| Change :func:`feature_selection.SequentialFeatureSelection` to
+  allow for unsupervised modelling so that the `fit` signature need not
+  do any `y` validation and allow for `y=None`. :pr:`19568` by
+  :user:`Shyam Desai <ShyamDesai>`.
+
 Code and Documentation Contributors
 -----------------------------------
 
diff --git a/sklearn/feature_selection/_sequential.py b/sklearn/feature_selection/_sequential.py
index 7ee6b043a0df1..6c9a857626f4e 100644
--- a/sklearn/feature_selection/_sequential.py
+++ b/sklearn/feature_selection/_sequential.py
@@ -19,7 +19,9 @@ class SequentialFeatureSelector(SelectorMixin, MetaEstimatorMixin,
     This Sequential Feature Selector adds (forward selection) or
     removes (backward selection) features to form a feature subset in a
     greedy fashion. At each stage, this estimator chooses the best feature to
-    add or remove based on the cross-validation score of an estimator.
+    add or remove based on the cross-validation score of an estimator. In
+    the case of unsupervised learning, this Sequential Feature Selector
+    looks only at the features (X), not the desired outputs (y).
 
     Read more in the :ref:`User Guide <sequential_feature_selection>`.
 
@@ -115,26 +117,26 @@ def __init__(self, estimator, *, n_features_to_select=None,
         self.cv = cv
         self.n_jobs = n_jobs
 
-    def fit(self, X, y):
+    def fit(self, X, y=None):
         """Learn the features to select.
 
         Parameters
         ----------
         X : array-like of shape (n_samples, n_features)
             Training vectors.
-        y : array-like of shape (n_samples,)
-            Target values.
+        y : array-like of shape (n_samples,), default=None
+            Target values. This parameter may be ignored for
+            unsupervised learning.
 
         Returns
         -------
         self : object
         """
         tags = self._get_tags()
-        X, y = self._validate_data(
-            X, y, accept_sparse="csc",
+        X = self._validate_data(
+            X, accept_sparse="csc",
             ensure_min_features=2,
             force_all_finite=not tags.get("allow_nan", True),
-            multi_output=True
         )
         n_features = X.shape[1]
 
diff --git a/sklearn/feature_selection/tests/test_sequential.py b/sklearn/feature_selection/tests/test_sequential.py
index 2ca22517ef956..e80294e38dc7b 100644
--- a/sklearn/feature_selection/tests/test_sequential.py
+++ b/sklearn/feature_selection/tests/test_sequential.py
@@ -6,10 +6,11 @@
 from sklearn.preprocessing import StandardScaler
 from sklearn.pipeline import make_pipeline
 from sklearn.feature_selection import SequentialFeatureSelector
-from sklearn.datasets import make_regression
+from sklearn.datasets import make_regression, make_blobs
 from sklearn.linear_model import LinearRegression
 from sklearn.experimental import enable_hist_gradient_boosting  # noqa
 from sklearn.ensemble import HistGradientBoostingRegressor
+from sklearn.cluster import KMeans
 
 
 @pytest.mark.parametrize('n_features_to_select', (0, 5, 0., -1, 1.1))
@@ -132,3 +133,31 @@ def test_pipeline_support():
     pipe = make_pipeline(StandardScaler(), sfs)
     pipe.fit(X, y)
     pipe.transform(X)
+
+
+@pytest.mark.parametrize('n_features_to_select', (2, 3, 4))
+def test_unsupervised_model_fit(n_features_to_select):
+    # Make sure that models without classification labels are not being
+    # validated
+
+    X, y = make_blobs(n_features=6)
+    sfs = SequentialFeatureSelector(
+        KMeans(),
+        n_features_to_select=n_features_to_select,
+    )
+    sfs.fit(X)
+    assert(sfs.transform(X).shape[1] == n_features_to_select)
+
+
+@pytest.mark.parametrize('y', ('no_validation', 1j, 99.9, np.nan, 3))
+def test_no_y_validation_model_fit(y):
+    # Make sure that other non-conventional y labels are not accepted
+
+    X, clusters = make_blobs(n_features=6)
+    sfs = SequentialFeatureSelector(
+        KMeans(),
+        n_features_to_select=3,
+    )
+
+    with pytest.raises((TypeError, ValueError)):
+        sfs.fit(X, y)


Code review comments:
diff_hunk: @@ -207,6 +207,14 @@ Changelog
   for non-English characters. :pr:`18959` by :user:`Zero <Zeroto521>`
   and :user:`wstates <wstates>`.
 
+:mod:`sklearn.feature_selection`
+.......................
path: doc/whats_new/v1.0.rst
text: @user1:
```suggestion
:mod:`sklearn.feature_selection`
................................
```
This will fix the sphinx warnings.