diff --git a/examples/release_highlights/plot_release_highlights_0_22_0.py b/examples/release_highlights/plot_release_highlights_0_22_0.py index 450700d143ca2..0d53dea9f7640 100644 --- a/examples/release_highlights/plot_release_highlights_0_22_0.py +++ b/examples/release_highlights/plot_release_highlights_0_22_0.py @@ -225,17 +225,21 @@ # --------------------------------------------------- # Developers can check the compatibility of their scikit-learn compatible # estimators using :func:`~utils.estimator_checks.check_estimator`. For -# instance, the ``check_estimator(LinearSVC)`` passes. +# instance, the ``check_estimator(LinearSVC())`` passes. # # We now provide a ``pytest`` specific decorator which allows ``pytest`` # to run all checks independently and report the checks that are failing. +# +# ..note:: +# This entry was slightly updated in version 0.24, where passing classes +# isn't supported anymore: pass instances instead. from sklearn.linear_model import LogisticRegression from sklearn.tree import DecisionTreeRegressor from sklearn.utils.estimator_checks import parametrize_with_checks -@parametrize_with_checks([LogisticRegression, DecisionTreeRegressor]) +@parametrize_with_checks([LogisticRegression(), DecisionTreeRegressor()]) def test_sklearn_compatible_estimator(estimator, check): check(estimator) diff --git a/sklearn/tests/test_common.py b/sklearn/tests/test_common.py index 73c99b0483de8..b9f50a76f7b30 100644 --- a/sklearn/tests/test_common.py +++ b/sklearn/tests/test_common.py @@ -27,15 +27,12 @@ from sklearn.linear_model._base import LinearClassifierMixin from sklearn.linear_model import LogisticRegression -from sklearn.model_selection import GridSearchCV from sklearn.utils import IS_PYPY from sklearn.utils._testing import SkipTest from sklearn.utils.estimator_checks import ( - _mark_xfail_checks, _construct_instance, _set_checking_parameters, _set_check_estimator_ids, - check_parameters_default_constructible, check_class_weight_balanced_linear_classifier, parametrize_with_checks) @@ -48,35 +45,6 @@ def test_all_estimator_no_base_class(): assert not name.lower().startswith('base'), msg -@ignore_warnings("Passing a class is depr", category=FutureWarning) # 0.24 -def test_estimator_cls_parameterize_with_checks(): - # TODO: remove test in 0.24 - # Non-regression test for #16707 to ensure that parametrize_with_checks - # works with estimator classes - param_checks = parametrize_with_checks([LogisticRegression]) - # Using the generator does not raise - list(param_checks.args[1]) - - -def test_mark_xfail_checks_with_unconsructable_estimator(): - class MyEstimator: - def __init__(self): - raise ValueError("This is bad") - - estimator, check = _mark_xfail_checks(MyEstimator, 42, None) - assert estimator == MyEstimator - assert check == 42 - - -@pytest.mark.parametrize( - 'name, Estimator', - all_estimators() -) -def test_parameters_default_constructible(name, Estimator): - # Test that estimators are default-constructible - check_parameters_default_constructible(name, Estimator) - - def _sample_func(x, y=1): pass @@ -117,29 +85,11 @@ def test_estimators(estimator, check, request): check(estimator) -@ignore_warnings("Passing a class is depr", category=FutureWarning) # 0.24 def test_check_estimator_generate_only(): - # TODO in 0.24: remove checks on passing a class - estimator_cls_gen_checks = check_estimator(LogisticRegression, - generate_only=True) all_instance_gen_checks = check_estimator(LogisticRegression(), generate_only=True) - assert isgenerator(estimator_cls_gen_checks) assert isgenerator(all_instance_gen_checks) - estimator_cls_checks = list(estimator_cls_gen_checks) - all_instance_checks = list(all_instance_gen_checks) - - # all classes checks include check_parameters_default_constructible - assert len(estimator_cls_checks) == len(all_instance_checks) + 1 - - # TODO: meta-estimators like GridSearchCV has required parameters - # that do not have default values. This is expected to change in the future - with pytest.raises(SkipTest): - for estimator, check in check_estimator(GridSearchCV, - generate_only=True): - check(estimator) - @ignore_warnings(category=(DeprecationWarning, FutureWarning)) # ignore deprecated open(.., 'U') in numpy distutils @@ -244,17 +194,13 @@ def test_all_tests_are_importable(): 'setup.py'.format(missing_tests)) -# TODO: remove in 0.24 -def test_class_support_deprecated(): +def test_class_support_removed(): # Make sure passing classes to check_estimator or parametrize_with_checks - # is deprecated + # raises an error - msg = "Passing a class is deprecated" - with pytest.warns(FutureWarning, match=msg): + msg = "Passing a class was deprecated.* isn't supported anymore" + with pytest.raises(TypeError, match=msg): check_estimator(LogisticRegression) - with pytest.warns(FutureWarning, match=msg): + with pytest.raises(TypeError, match=msg): parametrize_with_checks([LogisticRegression]) - - # Make sure check_parameters_default_constructible accepts instances now - check_parameters_default_constructible('name', LogisticRegression()) diff --git a/sklearn/utils/estimator_checks.py b/sklearn/utils/estimator_checks.py index 5fe5fa1458ecc..1d5bece4fa60d 100644 --- a/sklearn/utils/estimator_checks.py +++ b/sklearn/utils/estimator_checks.py @@ -33,7 +33,7 @@ from ..linear_model import Ridge from ..base import (clone, ClusterMixin, is_classifier, is_regressor, - RegressorMixin, is_outlier_detector, BaseEstimator) + RegressorMixin, is_outlier_detector) from ..metrics import accuracy_score, adjusted_rand_score, f1_score from ..random_projection import BaseRandomProjection @@ -258,6 +258,7 @@ def _yield_all_checks(name, estimator): if is_outlier_detector(estimator): for check in _yield_outliers_checks(name, estimator): yield check + yield check_parameters_default_constructible yield check_fit2d_predict1d yield check_methods_subset_invariance yield check_fit2d_1sample @@ -333,36 +334,9 @@ def _construct_instance(Estimator): return estimator -# TODO: probably not needed anymore in 0.24 since _generate_class_checks should -# be removed too. Just put this in check_estimator() -def _generate_instance_checks(name, estimator): - """Generate instance checks.""" - yield from ((estimator, partial(check, name)) - for check in _yield_all_checks(name, estimator)) - - -# TODO: remove this in 0.24 -def _generate_class_checks(Estimator): - """Generate class checks.""" - name = Estimator.__name__ - yield (Estimator, partial(check_parameters_default_constructible, name)) - estimator = _construct_instance(Estimator) - yield from _generate_instance_checks(name, estimator) - - def _mark_xfail_checks(estimator, check, pytest): """Mark (estimator, check) pairs with xfail according to the _xfail_checks_ tag""" - if isinstance(estimator, type): - # try to construct estimator instance, if it is unable to then - # return the estimator class, ignoring the tag - # TODO: remove this if block in 0.24 since passing instances isn't - # supported anymore - try: - estimator = _construct_instance(estimator) - except Exception: - return estimator, check - xfail_checks = estimator._get_tags()['_xfail_checks'] or {} check_name = _set_check_estimator_ids(check) @@ -387,12 +361,12 @@ def parametrize_with_checks(estimators): Parameters ---------- - estimators : list of estimators objects or classes + estimators : list of estimators instances Estimators to generated checks for. - .. deprecated:: 0.23 - Passing a class is deprecated from version 0.23, and won't be - supported in 0.24. Pass an instance instead. + .. versionchanged:: 0.24 + Passing a class was deprecated in version 0.23, and support for + classes was removed in 0.24. Pass an instance instead. Returns ------- @@ -413,11 +387,10 @@ def parametrize_with_checks(estimators): import pytest if any(isinstance(est, type) for est in estimators): - # TODO: remove class support in 0.24 and update docstrings - msg = ("Passing a class is deprecated since version 0.23 " - "and won't be supported in 0.24." + msg = ("Passing a class was deprecated in version 0.23 " + "and isn't supported anymore from 0.24." "Please pass an instance instead.") - warnings.warn(msg, FutureWarning) + raise TypeError(msg) checks_generator = chain.from_iterable( check_estimator(estimator, generate_only=True) @@ -441,12 +414,6 @@ def check_estimator(Estimator, generate_only=False): will be run if the Estimator class inherits from the corresponding mixin from sklearn.base. - This test can be applied to classes or instances. - Classes currently have some additional tests that related to construction, - while passing instances allows the testing of multiple options. However, - support for classes is deprecated since version 0.23 and will be removed - in version 0.24 (class checks will still be run on the instances). - Setting `generate_only=True` returns a generator that yields (estimator, check) tuples where the check can be called independently from each other, i.e. `check(estimator)`. This allows all checks to be run @@ -459,11 +426,11 @@ def check_estimator(Estimator, generate_only=False): Parameters ---------- estimator : estimator object - Estimator to check. Estimator is a class object or instance. + Estimator instance to check. - .. deprecated:: 0.23 - Passing a class is deprecated from version 0.23, and won't be - supported in 0.24. Pass an instance instead. + .. versionchanged:: 0.24 + Passing a class was deprecated in version 0.23, and support for + classes was removed in 0.24. generate_only : bool, optional (default=False) When `False`, checks are evaluated when `check_estimator` is called. @@ -479,20 +446,17 @@ def check_estimator(Estimator, generate_only=False): Generator that yields (estimator, check) tuples. Returned when `generate_only=True`. """ - # TODO: remove class support in 0.24 and update docstrings if isinstance(Estimator, type): - # got a class - msg = ("Passing a class is deprecated since version 0.23 " - "and won't be supported in 0.24." + msg = ("Passing a class was deprecated in version 0.23 " + "and isn't supported anymore from 0.24." "Please pass an instance instead.") - warnings.warn(msg, FutureWarning) + raise TypeError(msg) - checks_generator = _generate_class_checks(Estimator) - else: - # got an instance - estimator = Estimator - name = type(estimator).__name__ - checks_generator = _generate_instance_checks(name, estimator) + estimator = Estimator + name = type(estimator).__name__ + + checks_generator = ((estimator, partial(check, name)) + for check in _yield_all_checks(name, estimator)) if generate_only: return checks_generator @@ -2591,14 +2555,10 @@ def check_estimators_data_not_an_array(name, estimator_orig, X, y, obj_type): def check_parameters_default_constructible(name, Estimator): - # this check works on classes, not instances # test default-constructibility # get rid of deprecation warnings - if isinstance(Estimator, BaseEstimator): - # Convert estimator instance to its class - # TODO: Always convert to class in 0.24, because check_estimator() will - # only accept instances, not classes - Estimator = Estimator.__class__ + + Estimator = Estimator.__class__ with ignore_warnings(category=FutureWarning): estimator = _construct_instance(Estimator) diff --git a/sklearn/utils/tests/test_estimator_checks.py b/sklearn/utils/tests/test_estimator_checks.py index 594ff65f9e889..ad66021f3ba03 100644 --- a/sklearn/utils/tests/test_estimator_checks.py +++ b/sklearn/utils/tests/test_estimator_checks.py @@ -356,13 +356,12 @@ def fit(self, X, y): check_fit_score_takes_y("test", TestEstimatorWithDeprecatedFitMethod()) -@ignore_warnings("Passing a class is depr", category=FutureWarning) # 0.24 def test_check_estimator(): # tests that the estimator actually fails on "bad" estimators. # not a complete test of all checks, which are very extensive. # check that we have a set_params and can clone - msg = "it does not implement a 'get_params' method" + msg = "Passing a class was deprecated" assert_raises_regex(TypeError, msg, check_estimator, object) msg = "object has no attribute '_get_tags'" assert_raises_regex(AttributeError, msg, check_estimator, object()) @@ -375,12 +374,9 @@ def test_check_estimator(): ModifiesAnotherValue()) # check that we have a fit method msg = "object has no attribute 'fit'" - assert_raises_regex(AttributeError, msg, check_estimator, BaseEstimator) assert_raises_regex(AttributeError, msg, check_estimator, BaseEstimator()) # check that fit does input validation msg = "ValueError not raised" - assert_raises_regex(AssertionError, msg, check_estimator, - BaseBadClassifier) assert_raises_regex(AssertionError, msg, check_estimator, BaseBadClassifier()) # check that sample_weights in fit accepts pandas.Series type @@ -389,39 +385,38 @@ def test_check_estimator(): msg = ("Estimator NoSampleWeightPandasSeriesType raises error if " "'sample_weight' parameter is of type pandas.Series") assert_raises_regex( - ValueError, msg, check_estimator, NoSampleWeightPandasSeriesType) + ValueError, msg, check_estimator, NoSampleWeightPandasSeriesType()) except ImportError: pass # check that predict does input validation (doesn't accept dicts in input) msg = "Estimator doesn't check for NaN and inf in predict" - assert_raises_regex(AssertionError, msg, check_estimator, NoCheckinPredict) assert_raises_regex(AssertionError, msg, check_estimator, NoCheckinPredict()) # check that estimator state does not change # at transform/predict/predict_proba time msg = 'Estimator changes __dict__ during predict' - assert_raises_regex(AssertionError, msg, check_estimator, ChangesDict) + assert_raises_regex(AssertionError, msg, check_estimator, ChangesDict()) # check that `fit` only changes attribures that # are private (start with an _ or end with a _). msg = ('Estimator ChangesWrongAttribute should not change or mutate ' 'the parameter wrong_attribute from 0 to 1 during fit.') assert_raises_regex(AssertionError, msg, - check_estimator, ChangesWrongAttribute) - check_estimator(ChangesUnderscoreAttribute) + check_estimator, ChangesWrongAttribute()) + check_estimator(ChangesUnderscoreAttribute()) # check that `fit` doesn't add any public attribute msg = (r'Estimator adds public attribute\(s\) during the fit method.' ' Estimators are only allowed to add private attributes' ' either started with _ or ended' ' with _ but wrong_attribute added') assert_raises_regex(AssertionError, msg, - check_estimator, SetsWrongAttribute) + check_estimator, SetsWrongAttribute()) # check for invariant method name = NotInvariantPredict.__name__ method = 'predict' msg = ("{method} of {name} is not invariant when applied " "to a subset.").format(method=method, name=name) assert_raises_regex(AssertionError, msg, - check_estimator, NotInvariantPredict) + check_estimator, NotInvariantPredict()) # check for sparse matrix input handling name = NoSparseClassifier.__name__ msg = "Estimator %s doesn't seem to fail gracefully on sparse data" % name @@ -432,8 +427,8 @@ def test_check_estimator(): string_buffer = StringIO() sys.stdout = string_buffer try: - check_estimator(NoSparseClassifier) - except: + check_estimator(NoSparseClassifier()) + except Exception: pass finally: sys.stdout = old_stdout @@ -443,29 +438,28 @@ def test_check_estimator(): msg = ('Estimator LargeSparseNotSupportedClassifier doesn\'t seem to ' r'support \S{3}_64 matrix, and is not failing gracefully.*') assert_raises_regex(AssertionError, msg, check_estimator, - LargeSparseNotSupportedClassifier) + LargeSparseNotSupportedClassifier()) # does error on binary_only untagged estimator msg = 'Only 2 classes are supported' assert_raises_regex(ValueError, msg, check_estimator, - UntaggedBinaryClassifier) + UntaggedBinaryClassifier()) # non-regression test for estimators transforming to sparse data check_estimator(SparseTransformer()) # doesn't error on actual estimator - check_estimator(LogisticRegression) + check_estimator(LogisticRegression()) check_estimator(LogisticRegression(C=0.01)) - check_estimator(MultiTaskElasticNet) check_estimator(MultiTaskElasticNet()) # doesn't error on binary_only tagged estimator - check_estimator(TaggedBinaryClassifier) + check_estimator(TaggedBinaryClassifier()) # Check regressor with requires_positive_y estimator tag msg = 'negative y values not supported!' assert_raises_regex(ValueError, msg, check_estimator, - RequiresPositiveYRegressor) + RequiresPositiveYRegressor()) def test_check_outlier_corruption(): @@ -580,22 +574,6 @@ def test_check_regressor_data_not_an_array(): EstimatorInconsistentForPandas()) -@ignore_warnings("Passing a class is depr", category=FutureWarning) # 0.24 -def test_check_estimator_required_parameters_skip(): - # TODO: remove whole test in 0.24 since passes classes to check_estimator() - # isn't supported anymore - class MyEstimator(BaseEstimator): - _required_parameters = ["special_parameter"] - - def __init__(self, special_parameter): - self.special_parameter = special_parameter - - assert_raises_regex(SkipTest, r"Can't instantiate estimator MyEstimator " - r"which requires parameters " - r"\['special_parameter'\]", - check_estimator, MyEstimator) - - def run_tests_without_pytest(): """Runs the tests in this file without using pytest. """