8000 TST: Suppressed warnings by seberg · Pull Request #7099 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

TST: Suppressed warnings #7099

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 13 commits into from
Sep 2, 2016
Merged

TST: Suppressed warnings #7099

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
29a45ef
MAINT: Simplify deprecation test decorator
seberg Jun 19, 2016
968507b
ENH: Make warning testing context managers more specific
seberg Jun 19, 2016
86b0a5e
BUG: Suppress common NaT warnings
seberg Jun 19, 2016
78d7cc4
MAINT: Add missing teardown in deprecation test case
seberg Jun 19, 2016
c1ddf84
ENH: Remove warning ignoring from nanfuncs
seberg Jun 19, 2016
308161c
TST: Use new warnings context manager in all tests
seberg Jun 19, 2016
9bf7d14
ENH: Use new context manager for testing
seberg Jun 19, 2016
f078cb4
ENH: Masked array simplify copying back of unchanged values
seberg Aug 29, 2016
2e86117
TST: Silence warnings otherwise shown in release mode
seberg Aug 29, 2016
b831444
MAINT: Close some filehandlers and PIPEs after being done
seberg Aug 29, 2016
61694be
MAINT: Spell out with and without in ma test functions
seberg Aug 30, 2016
20ea3a2
TST: Replace catch_warnings when recording is not enforced in test_na…
seberg Aug 31, 2016
514d136
TST: Use ComplexWarning suppression only where needed
seberg Aug 31, 2016
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
Prev Previous commit
Next Next commit
ENH: Remove warning ignoring from nanfuncs 
Comment mentions a speedup, but it seems unsure why it should
be there. Instead use an error state in divide_by_count.
Some extra complex warnings had to be ignored (but those seemed correct)
  • Loading branch information
@seberg
seberg committed Sep 2, 2016
commit c1ddf841f6a48248b946a990ae750505b8b91686
72 changes: 33 additions & 39 deletions numpy/lib/nanfunctions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def _divide_by_count(a, b, out=None):
in place. If `a` is a numpy scalar, the division preserves its type.

"""
with np.errstate(invalid='ignore'):
with np.errstate(invalid='ignore', divide='ignore'):
if isinstance(a, np.ndarray):
if out is None:
return np.divide(a, b, out=a, casting='unsafe')
Expand Down Expand Up @@ -815,12 +815,9 @@ def nanmean(a, axis=None, dtype=None, out=None, keepdims=np._NoValue):
if out is not None and not issubclass(out.dtype.type, np.inexact):
raise TypeError("If a is inexact, then out must be inexact")

# The warning context speeds things up.
with warnings.catch_warnings():
warnings.simplefilter('ignore')
cnt = np.sum(~mask, axis=axis, dtype=np.intp, keepdims=keepdims)
tot = np.sum(arr, axis=axis, dtype=dtype, out=out, keepdims=keepdims)
avg = _divide_by_count(tot, cnt, out=out)
cnt = np.sum(~mask, axis=axis, dtype=np.intp, keepdims=keepdims)
tot = np.sum(arr, axis=axis, dtype=dtype, out=out, keepdims=keepdims)
Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't understand why it would speed things up, I somewhat assumed that adding divide=ignore for _divide_by_count does the trick here as well. (same below)

Copy link
Member Author

Choose a reason for hiding this comment

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

(had to ignore more complex warnings then, but that seemed right)

avg = _divide_by_count(tot, cnt, out=out)

isbad = (cnt == 0)
if isbad.any():
Expand Down Expand Up @@ -1288,38 +1285,35 @@ def nanvar(a, axis=None, dtype=None, out=None, ddof=0, keepdims=np._NoValue):
if out is not None and not issubclass(out.dtype.type, np.inexact):
raise TypeError("If a is inexact, then out must be inexact")

with warnings.catch_warnings():
warnings.simplefilter('ignore')

# Compute mean
if type(arr) is np.matrix:
_keepdims = np._NoValue
else:
_keepdims = True
# we need to special case matrix for reverse compatibility
# in order for this to work, these sums need to be called with
# keepdims=True, however matrix now raises an error in this case, but
# the reason that it drops the keepdims kwarg is to force keepdims=True
# so this used to work by serendipity.
cnt = np.sum(~mask, axis=axis, dtype=np.intp, keepdims=_keepdims)
avg = np.sum(arr, axis=axis, dtype=dtype, keepdims=_keepdims)
avg = _divide_by_count(avg, cnt)

# Compute squared deviation from mean.
np.subtract(arr, avg, out=arr, casting='unsafe')
arr = _copyto(arr, 0, mask)
if issubclass(arr.dtype.type, np.complexfloating):
sqr = np.multiply(arr, arr.conj(), out=arr).real
else:
sqr = np.multiply(arr, arr, out=arr)

# Compute variance.
var = np.sum(sqr, axis=axis, dtype=dtype, out=out, keepdims=keepdims)
if var.ndim < cnt.ndim:
# Subclasses of ndarray may ignore keepdims, so check here.
cnt = cnt.squeeze(axis)
dof = cnt - ddof
var = _divide_by_count(var, dof)
# Compute mean
if type(arr) is np.matrix:
_keepdims = np._NoValue
else:
_keepdims = True
# we need to special case matrix for reverse compatibility
# in order for this to work, these sums need to be called with
# keepdims=True, however matrix now raises an error in this case, but
# the reason that it drops the keepdims kwarg is to force keepdims=True
# so this used to work by serendipity.
cnt = np.sum(~mask, axis=axis, dtype=np.intp, keepdims=_keepdims)
avg = np.sum(arr, axis=axis, dtype=dtype, keepdims=_keepdims)
avg = _divide_by_count(avg, cnt)

# Compute squared deviation from mean.
np.subtract(arr, avg, out=arr, casting='unsafe')
arr = _copyto(arr, 0, mask)
if issubclass(arr.dtype.type, np.complexfloating):
sqr = np.multiply(arr, arr.conj(), out=arr).real
else:
sqr = np.multiply(arr, arr, out=arr)

# Compute variance.
var = np.sum(sqr, axis=axis, dtype=dtype, out=out, keepdims=keepdims)
if var.ndim < cnt.ndim:
# Subclasses of ndarray may ignore keepdims, so check here.
cnt = cnt.squeeze(axis)
dof = cnt - ddof
var = _divide_by_count(var, dof)

isbad = (dof <= 0)
if np.any(isbad):
Expand Down
62 changes: 32 additions & 30 deletions numpy/lib/tests/test_nanfunctions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import numpy as np
from numpy.testing import (
run_module_suite, TestCase, assert_, assert_equal, assert_almost_equal,
assert_warns, assert_no_warnings, assert_raises, assert_array_equal
assert_no_warnings, assert_raises, assert_array_equal, suppress_warnings
)


Expand Down Expand Up @@ -317,26 +317,30 @@ def test_dtype_from_dtype(self):
codes = 'efdgFDG'
for nf, rf in zip(self.nanfuncs, self.stdfuncs):
for c in codes:
tgt = rf(mat, dtype=np.dtype(c), axis=1).dtype.type
res = nf(mat, dtype=np.dtype(c), axis=1).dtype.type
assert_(res is tgt)
# scalar case
tgt = rf(mat, dtype=np.dtype(c), axis=None).dtype.type
res = nf(mat, dtype=np.dtype(c), axis=None).dtype.type
assert_(res is tgt)
with suppress_warnings() as sup:
Copy link
Member

Choose a reason for hiding this comment

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

So this was not needed before. I assume that it is needed now because the lower level warning is no longer caught. Is that what we want?

Copy link
Member Author

Choose a reason for hiding this comment

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

Did we used to have a once filter maybe, or did we even listen to these warnings at all?

I am not quite sure what you mean here, sorry. I guess the outer level does not catch the warning anymore, you could argue about it, but this is very explicit (i.e. someone writing a new test realizes there is a warning going on).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, it is np.nanstd issuing a warning. Funnily np.std does not issue it (I guess this is a slight bug).

Copy link
Member

Choose a reason for hiding this comment

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

Maybe open an issue for nanstd with a reference to this line so we can fix it.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is the correct fix. The class is a mixin, and fails for one specific function, so suppressing the warning for all functions is over kill. We should either fix the test or fix the function.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or maybe I will add a bit non-pretty if rf in {nanstd, nanvar}: sup.filter(...) with a comment + new issue....

Copy link
Member Author

Choose a reason for hiding this comment

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

I made the test specific with this hack (also fixed the other things like removing the full stuff and used more suppress_warnings, though for now only in the nanfunc tests.... Probably lots of places where we could do it more and I am willing to work a bit more on it later. But for the sake of keeping this not too long, maybe it is better to wait.

Copy link
Member Author

Choose a reason for hiding this comment

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

Did not have a quick idea how to fix the spurious warning bug in a very "obvious" way, but can think about it some more....

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the quick hack was the first thing to occur to me also. Not pretty, but at least it is documented.

Copy link
Member

Choose a reason for hiding this comment

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

But for the sake of keeping this not too long, maybe it is better to wait.

Yeah, as long as we have a good idea as to what needs fixing, I'd like to get this in as soon as possible.

sup.filter(np.ComplexWarning)
tgt = rf(mat, dtype=np.dtype(c), axis=1).dtype.type
res = nf(mat, dtype=np.dtype(c), axis=1).dtype.type
assert_(res is tgt)
# scalar case
tgt = rf(mat, dtype=np.dtype(c), axis=None).dtype.type
res = nf(mat, dtype=np.dtype(c), axis=None).dtype.type
assert_(res is tgt)

def test_dtype_from_char(self):
mat = np.eye(3)
codes = 'efdgFDG'
for nf, rf in zip(self.nanfuncs, self.stdfuncs):
for c in codes:
tgt = rf(mat, dtype=c, axis=1).dtype.type
res = nf(mat, dtype=c, axis=1).dtype.type
assert_(res is tgt)
# scalar case
tgt = rf(mat, dtype=c, axis=None).dtype.type
res = nf(mat, dtype=c, axis=None).dtype.type
assert_(res is tgt)
with suppress_warnings() as sup:
Copy link
Member

Choose a reason for hiding this comment

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

See previous comment.

sup.filter(np.ComplexWarning)
tgt = rf(mat, dtype=c, axis=1).dtype.type
res = nf(mat, dtype=c, axis=1).dtype.type
assert_(res is tgt)
# scalar case
tgt = rf(mat, dtype=c, axis=None).dtype.type
res = nf(mat, dtype=c, axis=None).dtype.type
assert_(res is tgt)

def test_dtype_from_input(self):
codes = 'efdgFDG'
Expand Down Expand Up @@ -524,16 +528,16 @@ def test_ddof_too_big(self):
dsize = [len(d) for d in _rdat]
for nf, rf in zip(nanfuncs, stdfuncs):
for ddof in range(5):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
with suppress_warnings() as sup:
sup.record(RuntimeWarning)
sup.filter(np.ComplexWarning)
tgt = [ddof >= d for d in dsize]
res = nf(_ndat, axis=1, ddof=ddof)
assert_equal(np.isnan(res), tgt)
if any(tgt):
assert_(len(w) == 1)
assert_(issubclass(w[0].category, RuntimeWarning))
assert_(len(sup.log) == 1)
else:
assert_(len(w) == 0)
assert_(len(sup.log) == 0)

def test_allnans(self):
mat = np.array([np.nan]*9).reshape(3, 3)
Expand Down Expand Up @@ -642,22 +646,20 @@ def test_result_values(self):
def test_allnans(self):
mat = np.array([np.nan]*9).reshape(3, 3)
for axis in [None, 0, 1]:
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
warnings.simplefilter('ignore', FutureWarning)
with suppress_warnings() as sup:
sup.record(RuntimeWarning)

assert_(np.isnan(np.nanmedian(mat, axis=axis)).all())
if axis is None:
assert_(len(w) == 1)
assert_(len(sup.log) == 1)
else:
assert_(len(w) == 3)
assert_(issubclass(w[0].category, RuntimeWarning))
assert_(len(sup.log) == 3)
# Check scalar
assert_(np.isnan(np.nanmedian(np.nan)))
if axis is None:
assert_(len(w) == 2)
assert_(len(sup.log) == 2)
else:
assert_(len(w) == 4)
assert_(issubclass(w[0].category, RuntimeWarning))
assert_(len(sup.log) == 4)

def test_empty(self):
mat = np.zeros((0, 3))
Expand Down Expand Up @@ -686,7 +688,7 @@ def test_extended_axis_invalid(self):

def test_float_special(self):
with warnings.catch_warnings(record=True):
warnings.simplefilter('ignore', RuntimeWarning)
warnings.simplefilter('always', RuntimeWarning)
Copy link
Member

Choose a reason for hiding this comment

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

Whoa, was that a bug?

Copy link
Member

Choose a reason for hiding this comment

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

So I think ignoring this was intended, what changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just avoiding all ignore filters, catch_warnings does not propagate outside so setting it to "always" effectively ignores it. Might be a bit cleaner to ues the new context though. At the beginning I did not always.

Copy link
Member

Choose a reason for hiding this comment

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

OK. The record was never looked at anyway, so we could probably do without a filter of anysort. However, using supress_warnings might be more informative as that it what is going on.

a = np.array([[np.inf, np.nan], [np.nan, np.nan]])
assert_equal(np.nanmedian(a, axis=0), [np.inf, np.nan])
assert_equal(np.nanmedian(a, axis=1), [np.inf, np.nan])
Expand Down
2 changes: 1 addition & 1 deletion numpy/lib/tests/test_twodim_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from numpy.testing import (
TestCase, run_module_suite, assert_equal, assert_array_equal,
assert_array_max_ulp, assert_array_almost_equal, assert_raises
assert_array_max_ulp, assert_array_almost_equal, assert_raises,
)

from numpy import (
Expand Down
0