-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: Fail on warning #22699
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
TST: Fail on warning #22699
Changes from all commits
9480916
bd5a419
36279c9
ad69f50
43d7a78
4919b0a
2804192
7ad249a
953fde1
d2dd0af
b813e41
3baba66
3067ed1
614f514
37a3d39
80ce447
61beba7
cf1ff63
a9a672d
da4961a
bed003b
8bd8711
ba46eef
3b0b9b0
e85d8d7
9eeb6a6
a89721e
9813b4c
b7f0198
dfc767c
51f77b5
5a1b8ee
83cd9ae
d10a0cc
cf60217
61587ec
cfb5ae2
2591677
1d39520
5f0eefe
7f618e9
4990fc2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,6 +38,7 @@ | |
import struct | ||
import inspect | ||
from collections import namedtuple | ||
import collections | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using ABCs from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Assuming the subsequent changes didn't generate any failures does this point to a potential lack of compatibility test coverage? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't follow 100%, but I don't think so. On 3.7, you basically couldn't import pandas when |
||
|
||
PY2 = sys.version_info[0] == 2 | ||
PY3 = sys.version_info[0] >= 3 | ||
|
@@ -135,6 +136,11 @@ def lfilter(*args, **kwargs): | |
|
||
from importlib import reload | ||
reload = reload | ||
Hashable = collections.abc.Hashable | ||
Iterable = collections.abc.Iterable | ||
Mapping = collections.abc.Mapping | ||
Sequence = collections.abc.Sequence | ||
Sized = collections.abc.Sized | ||
|
||
else: | ||
# Python 2 | ||
|
@@ -190,6 +196,12 @@ def get_range_parameters(data): | |
|
||
reload = builtins.reload | ||
|
||
Hashable = collections.Hashable | ||
Iterable = collections.Iterable | ||
Mapping = collections.Mapping | ||
Sequence = collections.Sequence | ||
Sized = collections.Sized | ||
|
||
if PY2: | ||
def iteritems(obj, **kw): | ||
return obj.iteritems(**kw) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,7 @@ def cmp_method(self, other): | |
# numpy will show a DeprecationWarning on invalid elementwise | ||
# comparisons, this will raise in the future | ||
with warnings.catch_warnings(record=True): | ||
warnings.filterwarnings("ignore", "elementwise", FutureWarning) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not fully related to this change, but do we actually want to ignore the warning here? Shouldn't we rather fix it if numpy will do something differently in the future? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added to #22698 |
||
with np.errstate(all='ignore'): | ||
result = op(self.values, np.asarray(other)) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ | |
|
||
from pandas._libs.lib import infer_dtype | ||
from pandas.util._decorators import cache_readonly | ||
from pandas.compat import u, range | ||
from pandas.compat import u, range, string_types | ||
from pandas.compat import set_function_name | ||
|
||
from pandas.core.dtypes.cast import astype_nansafe | ||
|
@@ -147,6 +147,11 @@ def coerce_to_array(values, dtype, mask=None, copy=False): | |
dtype = values.dtype | ||
|
||
if dtype is not None: | ||
if (isinstance(dtype, string_types) and | ||
(dtype.startswith("Int") or dtype.startswith("UInt"))): | ||
# Avoid DeprecationWarning from NumPy about np.dtype("Int64") | ||
# https://github.com/numpy/numpy/pull/7476 | ||
dtype = dtype.lower() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jreback could you take a close look here. I think you ran into this when writing a test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a test that causes this warning? (I can see this locally, but I didn't see it appearing when running tests) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, ignore previous comment, I thought pytest automatically shows all warnings, which is not the case. Shouldn't we edit out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a deprecation warning from NumPy right now, which is filtered interactive, but pytest removes that filter by default. Some of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh I didn't know numpy did this:
I guess so, thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. I'm waiting to hear back on numpy/numpy#7476 (comment) If |
||
if not issubclass(type(dtype), _IntegerDtype): | ||
try: | ||
dtype = _dtypes[str(np.dtype(dtype))] | ||
|
@@ -507,7 +512,8 @@ def cmp_method(self, other): | |
|
||
# numpy will show a DeprecationWarning on invalid elementwise | ||
# comparisons, this will raise in the future | ||
with warnings.catch_warnings(record=True): | ||
with warnings.catch_warnings(): | ||
warnings.filterwarnings("ignore", "elementwise", FutureWarning) | ||
with np.errstate(all='ignore'): | ||
result = op(self._data, other) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -758,7 +758,7 @@ def aggregate(self, func_or_funcs, *args, **kwargs): | |
if isinstance(func_or_funcs, compat.string_types): | ||
return getattr(self, func_or_funcs)(*args, **kwargs) | ||
|
||
if isinstance(func_or_funcs, collections.Iterable): | ||
if isinstance(func_or_funcs, compat.Iterable): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in theory we could add a lint rule to avoid using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we're close enough to py3-only that we'll be OK. Writing that regex would be a bit fiddly since we'd need to enumerate all the ABCs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. true, certainly a follow up issue is ok |
||
# Catch instances of lists / tuples | ||
# but not the class list / tuple itself. | ||
ret = self._aggregate_multiple_funcs(func_or_funcs, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,6 +98,7 @@ def cmp_method(self, other): | |
# numpy will show a DeprecationWarning on invalid elementwise | ||
# comparisons, this will raise in the future | ||
with warnings.catch_warnings(record=True): | ||
warnings.filterwarnings("ignore", "elementwise", FutureWarning) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the point in using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. laziness / not wanting to introduce copy-paste errors. New code should essentially always use marks, and I can make the changes here if you want. I mostly cleaned up Panel to remove these There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense. Maybe not a hard rule on this change but it would be helpful to use this instead in functions where there is a big diff |
||
with np.errstate(all='ignore'): | ||
result = op(self.values, np.asarray(other)) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3490,6 +3490,7 @@ def _putmask_smart(v, m, n): | |
|
||
# we ignore ComplexWarning here | ||
with warnings.catch_warnings(record=True): | ||
warnings.simplefilter("ignore", np.ComplexWarning) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does the context manager restore all of the warnings filters after? (I guess that is the point?) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's the idea. Ideally libraries shouldn't modify the global warnings registry without good cause. |
||
nn_at = nn.astype(v.dtype) | ||
|
||
# avoid invalid dtype comparisons | ||
|
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.
you may want to mention that we will actually fail if pytest.warns will fail the linter :>