Skip to content

ENH XFAIL in common tests with estimator tags (v3) #16502

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.

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 18 commits into from
Feb 20, 2020
Merged
5 changes: 5 additions & 0 deletions doc/developers/develop.rst
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,11 @@ _skip_test (default=``False``)
whether to skip common tests entirely. Don't use this unless you have a
*very good* reason.

_xfail_test (default=``False``)
dictionary ``{check_name : reason}`` of common checks to mark as a
known failure, with the associated reason. Don't use this unless you have a
*very good* reason.

stateless (default=``False``)
whether the estimator needs access to data for fitting. Even though an
estimator is stateless, it might still need a call to ``fit`` for
Expand Down
2 changes: 2 additions & 0 deletions doc/developers/tips.rst
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ Other `pytest` options that may become useful include:
- ``-s`` so that pytest does not capture the output of ``print()``
statements
- ``--tb=short`` or ``--tb=line`` to control the length of the logs
- ``--runxfail`` also run tests marked as a known failure (XFAIL) and report
errors.

Since our continuous integration tests will error if
``FutureWarning`` isn't properly caught,
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ addopts =
--ignore maint_tools
--doctest-modules
--disable-pytest-warnings
-rs
-rxXs

filterwarnings =
ignore:the matrix subclass:PendingDeprecationWarning
Expand Down
1 change: 1 addition & 0 deletions sklearn/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
'stateless': False,
'multilabel': False,
'_skip_test': False,
'_xfail_test': False,
'multioutput_only': False,
'binary_only': False,
'requires_fit': True}
Expand Down
8 changes: 8 additions & 0 deletions sklearn/decomposition/_sparse_pca.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,14 @@ def transform(self, X):

return U

def _more_tags(self):
return {
'_xfail_test': {
"check_methods_subset_invariance":
"fails for the transform method"
}
}


class MiniBatchSparsePCA(SparsePCA):
"""Mini-batch Sparse Principal Components Analysis
Expand Down
8 changes: 7 additions & 1 deletion sklearn/dummy.py
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,13 @@ def predict_log_proba(self, X):
return [np.log(p) for p in proba]

def _more_tags(self):
return {'poor_score': True, 'no_validation': True}
return {
'poor_score': True, 'no_validation': True,
'_xfail_test': {
'check_methods_subset_invariance':
'fails for the predict method'
}
}

def score(self, X, y, sample_weight=None):
"""Returns the mean accuracy on the given test data and labels.
Expand Down
8 changes: 8 additions & 0 deletions sklearn/neural_network/_rbm.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,3 +372,11 @@ def fit(self, X, y=None):
begin = end

return self

def _more_tags(self):
return {
'_xfail_test': {
'check_methods_subset_invariance':
'fails for the decision_function method'
}
}
9 changes: 9 additions & 0 deletions sklearn/svm/_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,15 @@ def __init__(self, nu=0.5, kernel='rbf', degree=3, gamma='scale',
break_ties=break_ties,
random_state=random_state)

def _more_tags(self):
return {
'_xfail_test': {
'check_methods_subset_invariance':
'fails for the decision_function method',
'check_class_weight_classifiers': 'class_weight is ignored.'
}
}


class SVR(RegressorMixin, BaseLibSVM):
"""Epsilon-Support Vector Regression.
Expand Down
11 changes: 9 additions & 2 deletions sklearn/tests/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from sklearn.utils import all_estimators
from sklearn.utils._testing import ignore_warnings
from sklearn.exceptions import ConvergenceWarning
from sklearn.utils.estimator_checks import check_estimator
from sklearn.utils.estimator_checks import check_estimator, _safe_tags

import sklearn
from sklearn.base import BiclusterMixin
Expand Down Expand Up @@ -87,12 +87,19 @@ def _tested_estimators():


@parametrize_with_checks(_tested_estimators())
def test_estimators(estimator, check):
def test_estimators(estimator, check, request):
# Common tests for estimator instances
with ignore_warnings(category=(FutureWarning,
ConvergenceWarning,
UserWarning, FutureWarning)):
_set_checking_parameters(estimator)

xfail_checks = _safe_tags(estimator, '_xfail_test')
check_name = _set_check_estimator_ids(check)
if xfail_checks:
if check_name in xfail_checks:
msg = xfail_checks[check_name]
request.applymarker(pytest.mark.xfail(reason=msg))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is probably a way to make this work for contrib projects automatically with parametrize_with_checks, but for now I just want something that we can use in scikit-learn as the ability to XFAIL is blocking multiple PRs.

check(estimator)


Expand Down
14 changes: 0 additions & 14 deletions sklearn/utils/estimator_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -1032,13 +1032,6 @@ def check_methods_subset_invariance(name, estimator_orig):

msg = ("{method} of {name} is not invariant when applied "
"to a subset.").format(method=method, name=name)
# TODO remove cases when corrected
if (name, method) in [('NuSVC', 'decision_function'),
('SparsePCA', 'transform'),
('MiniBatchSparsePCA', 'transform'),
('DummyClassifier', 'predict'),
('BernoulliRBM', 'score_samples')]:
raise SkipTest(msg)

if hasattr(estimator, method):
result_full, result_by_batch = _apply_on_subsets(
Expand Down Expand Up @@ -2243,13 +2236,6 @@ def check_regressors_no_decision_function(name, regressor_orig):

@ignore_warnings(category=FutureWarning)
def check_class_weight_classifiers(name, classifier_orig):
if name == "NuSVC":
# the sparse version has a parameter that doesn't do anything
raise SkipTest("Not testing NuSVC class weight as it is ignored.")
if name.endswith("NB"):
# NaiveBayes classifiers have a somewhat different interface.
# FIXME SOON!
raise SkipTest

if _safe_tags(classifier_orig, 'binary_only'):
problems = [2]
Expand Down