Skip to content

[MRG+2] Fix problematic caveat in ignore_warnings #11599

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 3 commits into from
Jul 17, 2018
Merged

[MRG+2] Fix problematic caveat in ignore_warnings #11599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96b3793
Fix caveat in ignore_warnings.
lesteve Jul 17, 2018
bc98f58
TST warning class should be passed as the category parameter.
lesteve Jul 17, 2018
c36f29d
Use the warning class name in message
lesteve Jul 17, 2018
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
2 changes: 1 addition & 1 deletion sklearn/linear_model/tests/test_sgd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ def test_validation_set_not_used_for_training(self):

assert_array_equal(clf1.coef_, clf2.coef_)

@ignore_warnings(ConvergenceWarning)
@ignore_warnings(category=ConvergenceWarning)
def test_n_iter_no_change(self):
# test that n_iter_ increases monotonically with n_iter_no_change
for early_stopping in [True, False]:
Expand Down
14 changes: 13 additions & 1 deletion sklearn/utils/testing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ def ignore_warnings(obj=None, category=Warning):

Parameters
----------
obj : callable or None
callable where you want to ignore the warnings.
category : warning class, defaults to Warning.
The category to filter. If Warning, all categories will be muted.

Expand All @@ -290,7 +292,17 @@ def ignore_warnings(obj=None, category=Warning):
>>> ignore_warnings(nasty_warn)()
42
"""
if callable(obj):
if isinstance(obj, type) and issubclass(obj, Warning):
# Avoid common pitfall of passing category as the first positional
# argument which result in the test not being run
warning_name = obj.__name__
raise ValueError(
"'obj' should be a callable where you want to ignore warnings. "
"You passed a warning class instead: 'obj={warning_name}'. "
"If you want to pass a warning class to ignore_warnings, "
"you should use 'category={warning_name}'".format(
warning_name=warning_name))
elif callable(obj):
return _IgnoreWarnings(category=category)(obj)
else:
return _IgnoreWarnings(category=category)
Expand Down
16 changes: 16 additions & 0 deletions sklearn/utils/tests/test_testing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

from scipy import sparse

import pytest

from sklearn.utils.deprecation import deprecated
from sklearn.utils.metaestimators import if_delegate_has_method
from sklearn.utils.testing import (
Expand Down Expand Up @@ -210,6 +212,20 @@ def context_manager_no_user_multiple_warning():
assert_warns(UserWarning, context_manager_no_deprecation_multiple_warning)
assert_warns(DeprecationWarning, context_manager_no_user_multiple_warning)

# Check that passing warning class as first positional argument
warning_class = UserWarning
match = "'obj' should be a callable.+you should use 'category=UserWarning'"

with pytest.raises(ValueError, match=match):
silence_warnings_func = ignore_warnings(warning_class)(
_warning_function)
silence_warnings_func()

with pytest.raises(ValueError, match=match):
@ignore_warnings(warning_class)
def test():
pass


class TestWarns(unittest.TestCase):
def test_warn(self):
Expand Down