Repository: scikit-learn/scikit-learn
Pull request: FIX Non-fit methods no long raises UserWarning for valid dataframes
<!--
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
<!--
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
-->
Fixes #21188


#### What does this implement/fix? Explain your changes.
This PR removes the double validation warnings from the estimators and updates the common test to check for the warning.


<!--
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:
False UserWarning for features names
### Describe the bug

Since the last stable update **(1.0)**, I get the following warning: 
**C:\Users\...\venv\lib\site-packages\sklearn\base.py:441: UserWarning: X does not have valid feature names, but KNeighborsClassifier was fitted with feature names
  warnings.warn(**
Changing features names to every possible way didn't change anything, and I didn't find anything related to valid feature names in the documentation.

### Steps/Code to Reproduce

``` python
import pandas as pd
import numpy as np
from sklearn.neighbors import KNeighborsClassifier

X_train = pd.DataFrame(np.random.randint(0,100,size=(100, 4)), columns=list('ABCD'))
y_train = np.random.default_rng().integers(2, size=100)
X_test = pd.DataFrame(np.random.randint(0,25,size=(25, 4)), columns=list('ABCD'))

k = 5
knn = KNeighborsClassifier(n_neighbors=k)
knn.fit(X_train, y_train)
y_pred = knn.predict(X_test)
print(y_pred)
```

### Expected Results

A list of 25 elements, possible values are {0,1}.
Example : [1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1]

### Actual Results

C:\Users\...\venv\lib\site-packages\sklearn\base.py:441:  UserWarning: X does not have valid feature names, but KNeighborsClassifier was fitted with feature names
  warnings.warn(
[1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1 1]

### Versions

System:
    python: 3.8.5 (tags/v3.8.5:580fbb0, Jul 20 2020, 15:57:54) [MSC v.1924 64 bit (AMD64)]
executable: c:\Users\...\venv\Scripts\python.exe
   machine: Windows-10-10.0.19041-SP0

Python dependencies:
          pip: 20.1.1
   setuptools: 47.1.0
      sklearn: 1.0
        numpy: 1.21.2
        scipy: 1.7.1
       Cython: None
       pandas: 1.3.3
   matplotlib: None
       joblib: 1.0.1
threadpoolctl: 2.2.0

Built with OpenMP: True

Hints from issue discussion:
The issue comes from the fact that we validate twice in the predict method. First at the beginning of this method and then in the kneighbors method. After the first validation, X is a ndarray and no longer has feature names. https://github.com/scikit-learn/scikit-learn/blob/9cb4e761f530e7a0708059b1312921753d056cbe/sklearn/neighbors/_classification.py#L214 https://github.com/scikit-learn/scikit-learn/blob/9cb4e761f530e7a0708059b1312921753d056cbe/sklearn/neighbors/_classification.py#L216

We should avoid the first validation (as well as in the predict_proba method). I'm surprised that we don't have a common test for that
If the type of X isn't important we could simply delete the first validation, but if a numpy.ndarray is more convenient for the rest of the function switching the validations will be enough to avoid the warnings.

Commit message: FIX Adds support for bsr and dok in precomputed

Patch to review:
diff --git a/doc/whats_new/v1.0.rst b/doc/whats_new/v1.0.rst
index d8776653cd9e8..0a3c5462d7693 100644
--- a/doc/whats_new/v1.0.rst
+++ b/doc/whats_new/v1.0.rst
@@ -2,6 +2,27 @@
 
 .. currentmodule:: sklearn
 
+.. _changes_1_0_1:
+
+Version 1.0.1
+==============
+
+**In Development**
+
+Changelog
+---------
+
+Fixed models
+------------
+
+- |Fix| Non-fit methods in the following classes do not raise a UserWarning
+  when fitted on DataFrames with valid feature names:
+  :class:`covariance.EllipticEnvelope`, :class:`ensemble.IsolationForest`,
+  :class:`ensemble.AdaBoostClassifier`, :class:`neighbors.KNeighborsClassifier`,
+  :class:`ensemble.RadiusNeighborsClassifier`,
+  :class:`ensemble.KNeighborsRegressor`,
+  :class:`ensemble.RadiusNeighborsRegressor`. :pr:`21199` by `Thomas Fan`_.
+
 .. _changes_1_0:
 
 Version 1.0.0
diff --git a/sklearn/covariance/_elliptic_envelope.py b/sklearn/covariance/_elliptic_envelope.py
index b63a4a67a9cfd..569e478e38ff4 100644
--- a/sklearn/covariance/_elliptic_envelope.py
+++ b/sklearn/covariance/_elliptic_envelope.py
@@ -215,7 +215,6 @@ def score_samples(self, X):
             Opposite of the Mahalanobis distances.
         """
         check_is_fitted(self)
-        X = self._validate_data(X, reset=False)
         return -self.mahalanobis(X)
 
     def predict(self, X):
diff --git a/sklearn/ensemble/_iforest.py b/sklearn/ensemble/_iforest.py
index e7562616b0cc5..2dc8d9d058d88 100644
--- a/sklearn/ensemble/_iforest.py
+++ b/sklearn/ensemble/_iforest.py
@@ -337,9 +337,9 @@ def predict(self, X):
             be considered as an inlier according to the fitted model.
         """
         check_is_fitted(self)
-        X = self._validate_data(X, accept_sparse="csr", reset=False)
-        is_inlier = np.ones(X.shape[0], dtype=int)
-        is_inlier[self.decision_function(X) < 0] = -1
+        decision_func = self.decision_function(X)
+        is_inlier = np.ones_like(decision_func, dtype=int)
+        is_inlier[decision_func < 0] = -1
         return is_inlier
 
     def decision_function(self, X):
diff --git a/sklearn/ensemble/_weight_boosting.py b/sklearn/ensemble/_weight_boosting.py
index a47937880d91c..8fd6cc0228137 100644
--- a/sklearn/ensemble/_weight_boosting.py
+++ b/sklearn/ensemble/_weight_boosting.py
@@ -676,8 +676,6 @@ def predict(self, X):
         y : ndarray of shape (n_samples,)
             The predicted classes.
         """
-        X = self._check_X(X)
-
         pred = self.decision_function(X)
 
         if self.n_classes_ == 2:
@@ -852,8 +850,6 @@ def predict_proba(self, X):
             outputs is the same of that of the :term:`classes_` attribute.
         """
         check_is_fitted(self)
-        X = self._check_X(X)
-
         n_classes = self.n_classes_
 
         if n_classes == 1:
@@ -886,7 +882,6 @@ def staged_predict_proba(self, X):
             The class probabilities of the input samples. The order of
             outputs is the same of that of the :term:`classes_` attribute.
         """
-        X = self._check_X(X)
 
         n_classes = self.n_classes_
 
@@ -912,7 +907,6 @@ def predict_log_proba(self, X):
             The class probabilities of the input samples. The order of
             outputs is the same of that of the :term:`classes_` attribute.
         """
-        X = self._check_X(X)
         return np.log(self.predict_proba(X))
 
 
diff --git a/sklearn/neighbors/_base.py b/sklearn/neighbors/_base.py
index 4e01cf2cd1076..8a626fe9e3c77 100644
--- a/sklearn/neighbors/_base.py
+++ b/sklearn/neighbors/_base.py
@@ -186,7 +186,7 @@ def _check_precomputed(X):
     else:
         graph = X
 
-    if graph.format not in ("csr", "csc", "coo", "lil"):
+    if graph.format not in ("csr", "csc", "coo", "lil", "bsr", "dok"):
         raise TypeError(
             "Sparse matrix in {!r} format is not supported due to "
             "its handling of explicit zeros".format(graph.format)
diff --git a/sklearn/neighbors/_classification.py b/sklearn/neighbors/_classification.py
index ced21c7885962..d616eaa2f32a8 100644
--- a/sklearn/neighbors/_classification.py
+++ b/sklearn/neighbors/_classification.py
@@ -211,8 +211,6 @@ def predict(self, X):
         y : ndarray of shape (n_queries,) or (n_queries, n_outputs)
             Class labels for each data sample.
         """
-        X = self._validate_data(X, accept_sparse="csr", reset=False)
-
         neigh_dist, neigh_ind = self.kneighbors(X)
         classes_ = self.classes_
         _y = self._y
@@ -255,8 +253,6 @@ def predict_proba(self, X):
             The class probabilities of the input samples. Classes are ordered
             by lexicographic order.
         """
-        X = self._validate_data(X, accept_sparse="csr", reset=False)
-
         neigh_dist, neigh_ind = self.kneighbors(X)
 
         classes_ = self.classes_
@@ -271,7 +267,7 @@ def predict_proba(self, X):
         if weights is None:
             weights = np.ones_like(neigh_ind)
 
-        all_rows = np.arange(X.shape[0])
+        all_rows = np.arange(n_queries)
         probabilities = []
         for k, classes_k in enumerate(classes_):
             pred_labels = _y[:, k][neigh_ind]
@@ -614,7 +610,6 @@ def predict_proba(self, X):
             by lexicographic order.
         """
 
-        X = self._validate_data(X, accept_sparse="csr", reset=False)
         n_queries = _num_samples(X)
 
         neigh_dist, neigh_ind = self.radius_neighbors(X)
diff --git a/sklearn/neighbors/_regression.py b/sklearn/neighbors/_regression.py
index 620a7af1d6708..75ef124ad1711 100644
--- a/sklearn/neighbors/_regression.py
+++ b/sklearn/neighbors/_regression.py
@@ -226,8 +226,6 @@ def predict(self, X):
         y : ndarray of shape (n_queries,) or (n_queries, n_outputs), dtype=int
             Target values.
         """
-        X = self._validate_data(X, accept_sparse="csr", reset=False)
-
         neigh_dist, neigh_ind = self.kneighbors(X)
 
         weights = _get_weights(neigh_dist, self.weights)
@@ -436,8 +434,6 @@ def predict(self, X):
                 dtype=double
             Target values.
         """
-        X = self._validate_data(X, accept_sparse="csr", reset=False)
-
         neigh_dist, neigh_ind = self.radius_neighbors(X)
 
         weights = _get_weights(neigh_dist, self.weights)
diff --git a/sklearn/utils/estimator_checks.py b/sklearn/utils/estimator_checks.py
index 914b4e6168247..37537bc1b0498 100644
--- a/sklearn/utils/estimator_checks.py
+++ b/sklearn/utils/estimator_checks.py
@@ -3779,7 +3779,14 @@ def check_dataframe_column_names_consistency(name, estimator_orig):
         check_methods.append((method, callable_method))
 
     for _, method in check_methods:
-        method(X)  # works
+        with warnings.catch_warnings():
+            warnings.filterwarnings(
+                "error",
+                message="X does not have valid feature names",
+                category=UserWarning,
+                module="sklearn",
+            )
+            method(X)  # works without UserWarning for valid features
 
     invalid_names = [
         (names[::-1], "Feature names must be in the same order as they were in fit."),


Code review comments:
diff_hunk: @@ -186,7 +186,7 @@ def _check_precomputed(X):
     else:
         graph = X
 
-    if graph.format not in ("csr", "csc", "coo", "lil"):
+    if graph.format not in ("csr", "csc", "coo", "lil", "bsr", "dok"):
path: sklearn/neighbors/_base.py
text: @author:
The implementation in `main` converts "bsr" to "csr" before calling `_check_precomputed`. I see two paths forward:

1. `_check_precomputed` should allow `bsr` and `dok`
2. `test_kneighbors_regressor_sparse` is incorrect in testing `bsr` and `dok`.

@user1 What was the issue with `bsr` and `dok` with explicit zeros?

@user1:
The neighbors module uses scipy sparse matrices using [graph semantics (glossary)](https://scikit-learn.org/stable/glossary.html#term-sparse-graph):
> As with `scipy.sparse.csgraph`, explicit zeros are interpreted as the number 0, but implicit zeros indicate a masked or absent value, such as the absence of an edge between two vertices of a graph, where an explicit value indicates an edge’s weight. This interpretation is adopted to represent connectivity in clustering, in representations of nearest neighborhoods (e.g. `neighbors.kneighbors_graph`), and for precomputed distance representation where only distances in the neighborhood of each point are required.

However, the handling of explicit zeros is inconsistent over scipy sparse formats.
- This first example shows that in the BSR format, the block structure often changes implicit zeros into explicit zeros.
```python
import numpy as np
from scipy.sparse import (bsr_matrix, coo_matrix, csc_matrix, csr_matrix,
                          dok_matrix, lil_matrix)

functions = [
    csr_matrix, csc_matrix, coo_matrix, lil_matrix, dok_matrix, bsr_matrix
]

# BSR format is forced to store some zeros explicitly
X = np.ones((3, 3))
X[0, 0] = 0  # future implicit zero
for function in functions:
    print(function.__name__, function(X).nnz == 8)

# csr_matrix True
# csc_matrix True
# coo_matrix True
# lil_matrix True
# dok_matrix True
# bsr_matrix False
```
- This second example shows that DOK (and actually LIL as well) removes some explicit zeros, for instance zeros set through item assignment.
```py
# LIL/DOK remove explicitly set zeros
for function in functions:
    X = function(np.ones((3, 3)))
    try:
        X[0, 0] = 0  # explicit zero
        print(function.__name__, X.nnz == 9)
    except Exception:  # COO/BSR do not support item assignment
        print(function.__name__, "Skip")

# csr_matrix True
# csc_matrix True
# coo_matrix Skip
# lil_matrix False
# dok_matrix False
# bsr_matrix Skip
```
Therefore, BSR/DOK/LIL formats might lead to unexpected results when used as precomputed sparse graphs.

As a side note, explicitly set zeros in CSR format are preserved when converting to other formats. So depending on how the sparse graph is generated, all formats might work as expected. Moreover, mistakes can be made with any format, by simply calling `X.eliminate_zeros()` to remove all explicitly stored zeros. So we could also accept all formats.
```py
# All formats preserve explicit zeros when converted from CSR
for function in functions:
    X = csr_matrix(np.ones((3, 3)))
    X[0, 0] = 0  # explicit zero
    print(function.__name__, function(X).nnz == 9)

# csr_matrix True
# csc_matrix True
# coo_matrix True
# lil_matrix True
# dok_matrix True
# bsr_matrix True

```
___
However, I think raising an error highlights the danger, and users can easily cast to CSR format if they want to force their usecase. So I would suggest:
- raising for BSR
- raising for DOK and LIL, or at least be consistent between the two
- adding some tests

@user1:
(I added this error based on this suggestion https://github.com/scikit-learn/scikit-learn/pull/10482#issuecomment-359959580, but I did not double check it, and I did not implement tests concerning this decision)

@user2:
So we should rather update the previously failing tests to expect a failure for those cases. I think we can treat it like a bug / no backward compat and it won't impact many users any way.

Ideally we would add a new test based on https://github.com/scikit-learn/scikit-learn/pull/10482#issuecomment-359959580 but I would not block this PR for this.

@author:
I updated the tests and whats new treating it as a bug fix.