Repository: scikit-learn/scikit-learn
Pull request: FIX warn if an estimator does have a concrete __sklearn_tags__ implementation
closes #30479 

Catch the error in the case that there is no `__sklearn_tags__` implementation in the MRO. We return the default tags and request to inherit from `BaseEstimator` that implements it.

Problem statement:
Version 1.6.X: ClassifierMixIn failing with new __sklearn_tags__ function
### Describe the bug

Hi,

we are using Sklearn in our projects for different classification training methods on production level. In the dev stage we upgraded to the latest release and our Training failed due to changes in the ClassifierMixIn Class. We use it in combination with a sklearn Pipeline.

in 1.6.X the following function was introduced:

```
    def __sklearn_tags__(self):
        tags = super().__sklearn_tags__()
        tags.estimator_type = "classifier"
        tags.classifier_tags = ClassifierTags()
        tags.target_tags.required = True
        return tags
```

It is calling the sklearn_tags methods from it's parent class. But the ClassifierMixIn doesn't have a parent class. So it says function super().__sklearn_tags__() is not existing.


### Steps/Code to Reproduce

```python
from sklearn.base import ClassifierMixin,
from sklearn.pipeline import Pipeline
import numpy as np

class MyEstimator(ClassifierMixin):
    def __init__(self, *, param=1):
        self.param = param
    def fit(self, X, y=None):
        self.is_fitted_ = True
        return self
    def predict(self, X):
        return np.full(shape=X.shape[0], fill_value=self.param)

X = np.array([[1, 2], [2, 3], [3, 4]])
y = np.array([1, 0, 1])


my_pipeline = Pipeline([("estimator", MyEstimator(param=1))])
my_pipeline.fit(X, y)
my_pipeline.predict(X)
```

### Expected Results

A Prediction is returned.

### Actual Results

```shell
Traceback (most recent call last):
  File "c:\Users\xxxx\error_sklearn\redo_error.py", line 22, in <module>
    my_pipeline.predict(X)
  File "C:\Users\xxxx\error_sklearn\.venv\Lib\site-packages\sklearn\pipeline.py", line 780, in predict
    with _raise_or_warn_if_not_fitted(self):
  File "C:\Program Files\Wpy64-31230\python-3.12.3.amd64\Lib\contextlib.py", line 144, in __exit__
    next(self.gen)
  File "C:\Users\xxxx\error_sklearn\.venv\Lib\site-packages\sklearn\pipeline.py", line 60, in _raise_or_warn_if_not_fitted
    check_is_fitted(estimator)
  File "C:\Users\xxxx\git_projects\error_sklearn\.venv\Lib\site-packages\sklearn\utils\validation.py", line 1756, in check_is_fitted
    if not _is_fitted(estimator, attributes, all_or_any):
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\xxxx\error_sklearn\.venv\Lib\site-packages\sklearn\utils\validation.py", line 1665, in _is_fitted
    return estimator.__sklearn_is_fitted__()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\xxxx\error_sklearn\.venv\Lib\site-packages\sklearn\pipeline.py", line 1310, in __sklearn_is_fitted__
    check_is_fitted(last_step)
  File "C:\Users\xxxx\error_sklearn\.venv\Lib\site-packages\sklearn\utils\validation.py", line 1751, in check_is_fitted
    tags = get_tags(estimator)
           ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\xxxx\error_sklearn\.venv\Lib\site-packages\sklearn\utils\_tags.py", line 396, in get_tags
    tags = estimator.__sklearn_tags__()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\xxxx\error_sklearn\.venv\Lib\site-packages\sklearn\base.py", line 540, in __sklearn_tags__
    tags = super().__sklearn_tags__()
           ^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'super' object has no attribute '__sklearn_tags__'
```
### Versions

```shell
1.6.0 / 1.6.X
```

Hints from issue discussion:
@DaMuBo According to [the official documentation](https://scikit-learn.org/1.5/developers/develop.html#rolling-your-own-estimator), I think your `MyEstimator` should inherit from `BaseEstimator` as well.

```python
import numpy as np
from sklearn.base import BaseEstimator, ClassifierMixin

class MyEstimator(ClassifierMixin, BaseEstimator):
    def __init__(self, *, param=1):
        self.param = param

    def fit(self, X, y=None):
        self.is_fitted_ = True
        return self

    def predict(self, X):
        return np.full(shape=X.shape[0], fill_value=self.param)
```

The code snippet above runs fine with your `.fit()` and `.predict()` calls. `sklearn` also provides `sklearn.utils.estimator_checks.check_estimator` which you can validate your custom estimator as well. I myself find it very useful for production-grade implementation.
@gunsodo  
Yes we are not exactly following the documentation here. 
We will change that on our side in the future for more robust code.

Thanks for the hint to the check_estimator we'll look into it.

But i also think the ClassifierMixIn shouldn't use a method which it can't execute.

I would still consider this breakage as a regression. We made effort to raise a proper deprecation warning such that user that defined compatible estimators have one version to change their code.

While we strongly advocate for using `BaseEstimator` and would request it in 1.7, I think that the `Mixin` should indeed not break but instead raise the same deprecation warnings than when one inherit from `BaseEstimator`.

We should probably have a try/except around the `super` call and if failing, get the `default_tags` and populate it.
@adrinjalali do you think that it is a reasonable way forward?
I'd rather make `ClassifierMixin` inherit from `BaseEstimator` instead (it's not a mixin per say then anymore though).

We certainly shouldn't "fix" the issue as proposed in #30480 since it breaks code if inheritance order is wrong.

The code written here in the OP is simply "not supported", however, it's the kinda thing that was supported before, albeit dysfunctional and wrong.

I'd suggest two things:

- improve the inheritance order check test, to make sure if there's any of the mixins present, `BaseEstimator` is also present, and call the test before trying to get tags
- The issue here is coming from `get_tags` called in `check_is_fitted`, which was a fix in this release too. We can make `get_tags` catch that particular exception, raise a deprecation warning, and return default tags, and the warning says that the code "runs", but it's probably wrong.

Commit message: Update doc/whats_new/upcoming_changes/sklearn.utils/30516.fix.rst

Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>

Patch to review:
diff --git a/doc/whats_new/upcoming_changes/sklearn.utils/30516.fix.rst b/doc/whats_new/upcoming_changes/sklearn.utils/30516.fix.rst
new file mode 100644
index 0000000000000..9c8acaa10f220
--- /dev/null
+++ b/doc/whats_new/upcoming_changes/sklearn.utils/30516.fix.rst
@@ -0,0 +1,4 @@
+- Raise a `FutureWarning` when there is no concrete implementation of `__sklearn_tags__`
+  in the MRO of the estimator. We request to inherit from `BaseEstimator` that
+  implements `__sklearn_tags__`.
+  By :user:`Guillaume Lemaitre <glemaitre>`
\ No newline at end of file
diff --git a/sklearn/utils/_tags.py b/sklearn/utils/_tags.py
index d4f211eb52152..1a3f707976042 100644
--- a/sklearn/utils/_tags.py
+++ b/sklearn/utils/_tags.py
@@ -393,7 +393,29 @@ def get_tags(estimator) -> Tags:
     tag_provider = _find_tags_provider(estimator)
 
     if tag_provider == "__sklearn_tags__":
-        tags = estimator.__sklearn_tags__()
+        try:
+            tags = estimator.__sklearn_tags__()
+        except AttributeError as exc:
+            if hasattr(estimator, "__sklearn_tags__"):
+                # workaround the regression reported in
+                # https://github.com/scikit-learn/scikit-learn/issues/30479
+                # `__sklearn_tags__` is implemented by calling
+                # `super().__sklearn_tags__()` but there is no `__sklearn_tags__`
+                # method in the base class.
+                warnings.warn(
+                    f"The following error was raised: {str(exc)}. It seems that "
+                    "there are no classes that implement `__sklearn_tags__` "
+                    "in the MRO and/or all classes in the MRO call "
+                    "`super().__sklearn_tags__()`. Make sure to inherit from "
+                    "`BaseEstimator` which implements `__sklearn_tags__`. Note that "
+                    "`BaseEstimator` needs to be on the right side of other Mixins in "
+                    "the inheritance order. The default are now used instead since "
+                    "retrieving tags failed.",
+                    category=DeprecationWarning,
+                )
+                tags = default_tags(estimator)
+            else:
+                raise exc
     else:
         # TODO(1.7): Remove this branch of the code
         # Let's go through the MRO and patch each class implementing _more_tags
diff --git a/sklearn/utils/tests/test_tags.py b/sklearn/utils/tests/test_tags.py
index 2ff6878d974fb..12ccd09e95e39 100644
--- a/sklearn/utils/tests/test_tags.py
+++ b/sklearn/utils/tests/test_tags.py
@@ -1,12 +1,15 @@
 from dataclasses import dataclass, fields
 
+import numpy as np
 import pytest
 
 from sklearn.base import (
     BaseEstimator,
+    ClassifierMixin,
     RegressorMixin,
     TransformerMixin,
 )
+from sklearn.pipeline import Pipeline
 from sklearn.utils import (
     ClassifierTags,
     InputTags,
@@ -629,3 +632,31 @@ def __sklearn_tags__(self):
     }
     assert old_tags == expected_tags
     assert _to_new_tags(_to_old_tags(new_tags), estimator=estimator) == new_tags
+
+
+def test_tags_no_sklearn_tags_concrete_implementation():
+    """Non-regression test for:
+    https://github.com/scikit-learn/scikit-learn/issues/30479
+
+    There is no class implementing `__sklearn_tags__` without calling
+    `super().__sklearn_tags__()`. Thus, we raise a warning and request to inherit from
+    `BaseEstimator` that implements `__sklearn_tags__`.
+    """
+
+    class MyEstimator(ClassifierMixin):
+        def __init__(self, *, param=1):
+            self.param = param
+
+        def fit(self, X, y=None):
+            self.is_fitted_ = True
+            return self
+
+        def predict(self, X):
+            return np.full(shape=X.shape[0], fill_value=self.param)
+
+    X = np.array([[1, 2], [2, 3], [3, 4]])
+    y = np.array([1, 0, 1])
+
+    my_pipeline = Pipeline([("estimator", MyEstimator(param=1))])
+    with pytest.warns(DeprecationWarning, match="The following error was raised"):
+        my_pipeline.fit(X, y).predict(X)


Code review comments:
diff_hunk: @@ -0,0 +1,4 @@
+- Raise a `FutureWarning` when there is no concrete implementation of `__sklearn_tags__`
path: doc/whats_new/upcoming_changes/sklearn.utils/30516.fix.rst
text: @user1:
```suggestion
- Raise a `DeprecationWarning` when there is no concrete implementation of `__sklearn_tags__`
```