Skip to content

MAINT Clean-up deprecated if_delegate_has_method for 1.3 #25879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 16, 2023
Merged

MAINT Clean-up deprecated if_delegate_has_method for 1.3 #25879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a0c14f9
cln deprecations
jeremiedbb Mar 16, 2023
547c09c
iter
jeremiedbb Mar 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions doc/modules/classes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1680,12 +1680,3 @@ Utilities from joblib:

Recently deprecated
===================

To be removed in 1.3
--------------------

.. autosummary::
:toctree: generated/
:template: function.rst

utils.metaestimators.if_delegate_has_method
2 changes: 1 addition & 1 deletion sklearn/utils/estimator_checks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1716,7 +1716,7 @@ def check_fit_score_takes_y(name, estimator_orig):
func(X, y)
args = [p.name for p in signature(func).parameters.values()]
if args[0] == "self":
# if_delegate_has_method makes methods into functions
# available_if makes methods into functions
# with an explicit "self", so need to shift arguments
args = args[1:]
assert args[1] in ["y", "Y"], (
Expand Down
82 changes: 2 additions & 80 deletions sklearn/utils/metaestimators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,17 @@
# Andreas Mueller
# License: BSD
from typing import List, Any
import warnings

from abc import ABCMeta, abstractmethod
from operator import attrgetter
import numpy as np
from contextlib import suppress

from ..utils import _safe_indexing
from ..utils._tags import _safe_tags
from ..base import BaseEstimator
from ._available_if import available_if, _AvailableIfDescriptor
from ._available_if import available_if

__all__ = ["available_if", "if_delegate_has_method"]
__all__ = ["available_if"]


class _BaseComposition(BaseEstimator, metaclass=ABCMeta):
Expand Down Expand Up @@ -96,82 +94,6 @@ def _validate_names(self, names):
)


# TODO(1.3) remove
class _IffHasAttrDescriptor(_AvailableIfDescriptor):
"""Implements a conditional property using the descriptor protocol.

Using this class to create a decorator will raise an ``AttributeError``
if none of the delegates (specified in ``delegate_names``) is an attribute
of the base object or the first found delegate does not have an attribute
``attribute_name``.

This allows ducktyping of the decorated method based on
``delegate.attribute_name``. Here ``delegate`` is the first item in
``delegate_names`` for which ``hasattr(object, delegate) is True``.

See https://docs.python.org/3/howto/descriptor.html for an explanation of
descriptors.
"""

def __init__(self, fn, delegate_names, attribute_name):
super().__init__(fn, self._check, attribute_name)
self.delegate_names = delegate_names

def _check(self, obj):
warnings.warn(
"if_delegate_has_method was deprecated in version 1.1 and will be "
"removed in version 1.3. Use available_if instead.",
FutureWarning,
)

delegate = None
for delegate_name in self.delegate_names:
try:
delegate = attrgetter(delegate_name)(obj)
break
except AttributeError:
continue

if delegate is None:
return False
# raise original AttributeError
getattr(delegate, self.attribute_name)

return True


# TODO(1.3) remove
def if_delegate_has_method(delegate):
"""Create a decorator for methods that are delegated to a sub-estimator.

.. deprecated:: 1.3
`if_delegate_has_method` is deprecated in version 1.1 and will be removed in
version 1.3. Use `available_if` instead.

This enables ducktyping by hasattr returning True according to the
sub-estimator.

Parameters
----------
delegate : str, list of str or tuple of str
Name of the sub-estimator that can be accessed as an attribute of the
base object. If a list or a tuple of names are provided, the first
sub-estimator that is an attribute of the base object will be used.

Returns
-------
callable
Callable makes the decorated method available if the delegate
has a method with the same name as the decorated method.
"""
if isinstance(delegate, list):
delegate = tuple(delegate)
if not isinstance(delegate, tuple):
delegate = (delegate,)

return lambda fn: _IffHasAttrDescriptor(fn, delegate, attribute_name=fn.__name__)


def _safe_split(estimator, X, y, indices, train_indices=None):
"""Create subset of dataset and properly handle kernels.

Expand Down
109 changes: 0 additions & 109 deletions sklearn/utils/tests/test_metaestimators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,96 +1,10 @@
import numpy as np
import pytest
import warnings

import pickle

from sklearn.utils.metaestimators import if_delegate_has_method
from sklearn.utils.metaestimators import available_if


class Prefix:
def func(self):
pass


class MockMetaEstimator:
"""This is a mock meta estimator"""

a_prefix = Prefix()

@if_delegate_has_method(delegate="a_prefix")
def func(self):
"""This is a mock delegated function"""
pass


@pytest.mark.filterwarnings("ignore:if_delegate_has_method was deprecated")
def test_delegated_docstring():
assert "This is a mock delegated function" in str(
MockMetaEstimator.__dict__["func"].__doc__
)
assert "This is a mock delegated function" in str(MockMetaEstimator.func.__doc__)
assert "This is a mock delegated function" in str(MockMetaEstimator().func.__doc__)


class MetaEst:
"""A mock meta estimator"""

def __init__(self, sub_est, better_sub_est=None):
self.sub_est = sub_est
self.better_sub_est = better_sub_est

@if_delegate_has_method(delegate="sub_est")
def predict(self):
pass


class MetaEstTestTuple(MetaEst):
"""A mock meta estimator to test passing a tuple of delegates"""

@if_delegate_has_method(delegate=("sub_est", "better_sub_est"))
def predict(self):
pass


class MetaEstTestList(MetaEst):
"""A mock meta estimator to test passing a list of delegates"""

@if_delegate_has_method(delegate=["sub_est", "better_sub_est"])
def predict(self):
pass


class HasPredict:
"""A mock sub-estimator with predict method"""

def predict(self):
pass


class HasNoPredict:
"""A mock sub-estimator with no predict method"""

pass


class HasPredictAsNDArray:
"""A mock sub-estimator where predict is a NumPy array"""

predict = np.ones((10, 2), dtype=np.int64)


@pytest.mark.filterwarnings("ignore:if_delegate_has_method was deprecated")
def test_if_delegate_has_method():
assert hasattr(MetaEst(HasPredict()), "predict")
assert not hasattr(MetaEst(HasNoPredict()), "predict")
assert not hasattr(MetaEstTestTuple(HasNoPredict(), HasNoPredict()), "predict")
assert hasattr(MetaEstTestTuple(HasPredict(), HasNoPredict()), "predict")
assert not hasattr(MetaEstTestTuple(HasNoPredict(), HasPredict()), "predict")
assert not hasattr(MetaEstTestList(HasNoPredict(), HasPredict()), "predict")
assert hasattr(MetaEstTestList(HasPredict(), HasPredict()), "predict")


class AvailableParameterEstimator:
"""This estimator's `available` parameter toggles the presence of a method"""

Expand Down Expand Up @@ -137,29 +51,6 @@ def test_available_if_unbound_method():
AvailableParameterEstimator.available_func(est)


@pytest.mark.filterwarnings("ignore:if_delegate_has_method was deprecated")
def test_if_delegate_has_method_numpy_array():
"""Check that we can check for an attribute that is a NumPy array.

This is a non-regression test for:
https://github.com/scikit-learn/scikit-learn/issues/21144
"""
estimator = MetaEst(HasPredictAsNDArray())
assert hasattr(estimator, "predict")


def test_if_delegate_has_method_deprecated():
"""Check the deprecation warning of if_delegate_has_method"""
# don't warn when creating the decorator
with warnings.catch_warnings():
warnings.simplefilter("error", FutureWarning)
_ = if_delegate_has_method(delegate="predict")

# Only when calling it
with pytest.warns(FutureWarning, match="if_delegate_has_method was deprecated"):
hasattr(MetaEst(HasPredict()), "predict")


def test_available_if_methods_can_be_pickled():
"""Check that available_if methods can be pickled.

Expand Down
62 changes: 3 additions & 59 deletions sklearn/utils/tests/test_testing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import pytest

from sklearn.utils.deprecation import deprecated
from sklearn.utils.metaestimators import available_if, if_delegate_has_method
from sklearn.utils.metaestimators import available_if
from sklearn.utils._testing import (
assert_raises,
assert_no_warnings,
Expand Down Expand Up @@ -430,64 +430,7 @@ def fit(self, X, y):
"""Incorrect docstring but should not be tested"""


class MockMetaEstimatorDeprecatedDelegation:
def __init__(self, delegate):
"""MetaEstimator to check if doctest on delegated methods work.

Parameters
---------
delegate : estimator
Delegated estimator.
"""
self.delegate = delegate

@if_delegate_has_method(delegate="delegate")
def predict(self, X):
"""This is available only if delegate has predict.

Parameters
----------
y : ndarray
Parameter y
"""
return self.delegate.predict(X)

@if_delegate_has_method(delegate="delegate")
@deprecated("Testing a deprecated delegated method")
def score(self, X):
"""This is available only if delegate has score.

Parameters
---------
y : ndarray
Parameter y
"""

@if_delegate_has_method(delegate="delegate")
def predict_proba(self, X):
"""This is available only if delegate has predict_proba.

Parameters
---------
X : ndarray
Parameter X
"""
return X

@deprecated("Testing deprecated function with wrong params")
def fit(self, X, y):
"""Incorrect docstring but should not be tested"""


@pytest.mark.filterwarnings("ignore:if_delegate_has_method was deprecated")
@pytest.mark.parametrize(
"mock_meta",
[
MockMetaEstimator(delegate=MockEst()),
MockMetaEstimatorDeprecatedDelegation(delegate=MockEst()),
],
)
def test_check_docstring_parameters(mock_meta):
def test_check_docstring_parameters():
pytest.importorskip(
"numpydoc",
reason="numpydoc is required to test the docstrings",
Expand All @@ -506,6 +449,7 @@ def test_check_docstring_parameters(mock_meta):
check_docstring_parameters(Klass.f_bad_sections)

incorrect = check_docstring_parameters(f_check_param_definition)
mock_meta = MockMetaEstimator(delegate=MockEst())
mock_meta_name = mock_meta.__class__.__name__
assert incorrect == [
"sklearn.utils.tests.test_testing.f_check_param_definition There "
Expand Down