-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
raise DeprecationWarnings and FutureWarnings as errors #11570
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
Conversation
…e names. this is already tested for strings. Could add a test for dtype object?
this should be everything except model_selection and isolation forest and bagging which uses Imputer (these two have separate issues). |
merged #11593 into this so tests may pass. |
@@ -473,6 +473,7 @@ def test_lars_path_readonly_data(): | |||
_lars_path_residues(X_train, y_train, X_test, y_test, copy=False) | |||
|
|||
|
|||
@ignore_warnings(DeprecationWarning) # positive deprecated, remove in 0.22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will skip the test due to #11594 (also applies to all other tests)
@@ -1,20 +1,21 @@ | |||
"""Test the search module""" | |||
|
|||
from collections import Iterable, Sized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from sklearn.utils.fixes import _Iterable as Iterable, _Sized as Sized
otherwise we get collections ABC warning in Python 3.7
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, right. not sure why @janvanrijn changed that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My code is an (old) version of master. I did not intentionally change that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, that makes sense.
|
||
import numpy as np | ||
import scipy.sparse as sp | ||
import pytest | ||
|
||
from sklearn.utils.fixes import sp_version | ||
from sklearn.utils.fixes import PY3_OR_LATER | ||
from sklearn.utils.fixes import _Iterable as Iterable, _Sized as Sized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? That's necessary to avoid warnings in Python 3.7
I think we need need to test Python 3.7 in Travis and not in a cron job (#11409). Otherwise this would have failed on Python 3.7 but we won't know because we are not testing for it.. |
@rth we should but that seems unrelated ;) |
@amueller you are right. |
I attempted the thing @massich is mentioning in github.com/amueller/futurepast but didn't have the follow-through necessary |
@@ -75,6 +76,7 @@ def test_delegate_to_func(): | |||
) | |||
|
|||
|
|||
@ignore_warnings(FutureWarning) # ignore warning for validate=False 0.22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ignore_warnings(category=FutureWarning)
Towards #11252.
In the end we'd like to make these errors so we can keep this cleaner in the future.