8000 [MRG+2] catch more expected warnings in common tests by ShangwuYao · Pull Request #11151 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

[MRG+2] catch more expected warnings in common tests #11151

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
Jun 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions sklearn/gaussian_process/tests/test_gaussian_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from sklearn.datasets import make_regression
from sklearn.utils.testing import assert_greater, assert_true, assert_raises

pytestmark = pytest.mark.filterwarnings('ignore', category=DeprecationWarning)
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason that DeprecationWarnings are ignored here? (this is not part of the common tests)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR fixed bugs in ignore_warnings and clean_warning_registry, see here, not just common tests. Before this, some warnings that should be raised in test_gaussian_process was somehow suppressed, after this change, they are correctly raised, and this file alone raises more than 4000 new DeprecationWarnings, so we ignored it to see this fix is really working, see here. We probably shouldn't ignore this warning as you said.

Copy link
Member
@lesteve lesteve Jun 13, 2018

Choose a reason for hiding this comment

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

What is the reason that DeprecationWarnings are ignored here? (this is not part of the common tests)

Unless I am missing something sklearn/gaussian_process/gaussian_process.py is deprecated and scheduled to be removed in 0.20.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that's a good reason :-)
Ideally I would say that deprecation warnings are asserted once and then explicitly ignored for other tests, but since this is deprecating a full sub-module and not a method or so, then probably makes sense to just ignore them for the full submodule.


f = lambda x: x * np.sin(x)
X = np.atleast_2d([1., 3., 5., 6., 7., 8.]).T
Expand Down
17 changes: 11 additions & 6 deletions sklearn/tests/test_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from sklearn.utils.testing import assert_greater
from sklearn.utils.testing import assert_in
from sklearn.utils.testing import ignore_warnings
from sklearn.exceptions import ConvergenceWarning

import sklearn
from sklearn.cluster.bicluster import BiclusterMixin
Expand Down Expand Up @@ -91,18 +92,22 @@ def _rename_partial(val):
)
def test_non_meta_estimators(name, Estimator, check):
# Common tests for non-meta estimators
estimator = Estimator()
set_checking_parameters(estimator)
check(name, estimator)
with ignore_warnings(category=(DeprecationWarning, ConvergenceWarning,
UserWarning, FutureWarning)):
estimator = Estimator()
set_checking_parameters(estimator)
check(name, estimator)


@pytest.mark.parametrize("name, Estimator",
_tested_non_meta_estimators())
def test_no_attributes_set_in_init(name, Estimator):
# input validation etc for non-meta estimators
estimator = Estimator()
# check this on class
check_no_attributes_set_in_init(name, estimator)
with ignore_warnings(category=(DeprecationWarning, ConvergenceWarning,
UserWarning, FutureWarning)):
estimator = Estimator()
# check this on class
check_no_attributes_set_in_init(name, estimator)


def test_configure():
Expand Down
5 changes: 2 additions & 3 deletions
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,10 @@ def check_estimator(Estimator):
for check in _yield_all_checks(name, estimator):
try:
check(name, estimator)
except SkipTest as message:
except SkipTest as exception:
# the only SkipTest thrown currently results from not
# being able to import pandas.
warnings.warn(message, SkipTestWarning)
warnings.warn(str(exception), SkipTestWarning)


def _boston_subset(n_samples=200):
Expand All @@ -327,7 +327,6 @@ def set_checking_parameters(estimator):
and not isinstance(estimator, BaseSGD)):
estimator.set_params(n_iter=5)
if "max_iter" in params:
warnings.simplefilter("ignore", ConvergenceWarning)
if estimator.max_iter is not None:
estimator.set_params(max_iter=min(5, estimator.max_iter))
# LinearSVR, LinearSVC
Expand Down
17 changes: 10 additions & 7 deletions sklearn/utils/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ def assert_warns(warning_class, func, *args, **kw):
result : the return value of `func`

"""
# very important to avoid uncontrolled state propagation
clean_warning_registry()
with warnings.catch_warnings(record=True) as w:
# Cause all warnings to always be triggered.
Expand Down Expand Up @@ -321,7 +320,6 @@ def __call__(self, fn):
"""Decorator to catch and hide warnings without visual nesting."""
@wraps(fn)
def wrapper(*args, **kwargs):
# very important to avoid uncontrolled state propagation
clean_warning_registry()
with warnings.catch_warnings():
warnings.simplefilter("ignore", self.category)
Expand All @@ -339,22 +337,22 @@ def __repr__(self):
return "%s(%s)" % (name, ", ".join(args))

def __enter__(self):
clean_warning_registry() # be safe and not propagate state + chaos
warnings.simplefilter("ignore", self.category)
if self._entered:
raise RuntimeError("Cannot enter %r twice" % self)
self._entered = True
self._filters = self._module.filters
self._module.filters = self._filters[:]
self._showwarning = self._module.showwarning
clean_warning_registry()
warnings.simplefilter("ignore", self.category)

def __exit__(self, *exc_info):
if not self._entered:
raise RuntimeError("Cannot exit %r without entering first" % self)
self._module.filters = self._filters
self._module.showwarning = self._showwarning
self.log[:] = []
clean_warning_registry() # be safe and not propagate state + chaos
clean_warning_registry()


assert_less = _dummy.assertLess
Expand Down Expand Up @@ -724,8 +722,13 @@ def run_test(*args, **kwargs):


def clean_warning_registry():
"""Safe way to reset warnings."""
warnings.resetwarnings()
"""Clean Python warning registry for easier testing of warning messages.

We may not need to do this any more when getting rid of Python 2, not
entirely sure. See https://bugs.python.org/issue4180 and
https://bugs.python.org/issue21724 for more details.

"""
reg = "__warningregistry__"
for mod_name, mod in list(sys.modules.items()):
if 'six.moves' in mod_name:
Expand Down
21 changes: 7 additions & 14 deletions sklearn/utils/tests/test_testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,26 +211,19 @@ def context_manager_no_user_multiple_warning():
assert_warns(DeprecationWarning, context_manager_no_user_multiple_warning)


# This class is inspired from numpy 1.7 with an alteration to check
# the reset warning filters after calls to assert_warns.
# This assert_warns behavior is specific to scikit-learn because
# `clean_warning_registry()` is called internally by assert_warns
# and clears all previous filters.
class TestWarns(unittest.TestCase):
def test_warn(self):
def f():
warnings.warn("yo")
return 3

# Test that assert_warns is not impacted by externally set
# filters and is reset internally.
# This is because `clean_warning_registry()` is called internally by
# assert_warns and clears all previous filters.
warnings.simplefilter("ignore", UserWarning)
assert_equal(assert_warns(UserWarning, f), 3)

# Test that the warning registry is empty after assert_warns
assert_equal(sys.modules['warnings'].filters, [])
with warnings.catch_warnings():
warnings.simplefilter("ignore", UserWarning)
filters_orig = warnings.filters[:]
assert_equal(assert_warns(UserWarning, f), 3)
# test that assert_warns doesn't have side effects on warnings
# filters
assert_equal(warnings.filters, filters_orig)

assert_raises(AssertionError, assert_no_warnings, f)
assert_equal(assert_no_warnings(lambda x: x, 1), 1)
Expand Down
0