10000 BUG: Add a lock to assert_equal and other testing functions · numpy/numpy@355d921 · GitHub
[go: up one dir, main page]

Skip to content

Commit 355d921

Browse files
committed
BUG: Add a lock to assert_equal and other testing functions
This lock prevents unsafe warning filter manipulations during testing if downstream packages do parallel testing. Warning filtering is generally not threadsafe in python, this is also true for `catch_warnings` or `suppress_warinings`. In NumPy 1.12 however, `assert_equal` and the array comparison asserts, use this to filter out some comparison warnings. Since removing this filter may also affect downstream projects and skimage (and possibly more) do parallel manual parallel testing using `assert_equal`, a quick fix seems to be to lock the less obvious threading trap. Ideally (in master this is the case), there should simply not be warning filter logic in the assert functions themself. While probably not perfectly safe in principle, it is sufficient in the case of skimage and probably most testing scenarios and the chance of deadlocks seems very unlikely. Closes gh-8413
1 parent 2d4ec45 commit 355d921

File tree

1 file changed

+26
-3
lines changed

1 file changed

+26
-3
lines changed

numpy/testing/utils.py

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@
2323
else:
2424
from StringIO import StringIO
2525

26+
try:
27+
from threading import Lock
28+
except ImportError:
29+
from dummy_threading import Lock
30+
31+
2632
__all__ = [
2733
'assert_equal', 'assert_almost_equal', 'assert_approx_equal',
2834
'assert_array_equal', 'assert_array_less', 'assert_string_equal',
@@ -42,6 +48,20 @@ class KnownFailureException(Exception):
4248
pass
4349

4450

51+
# Warning filtering is generally not threadsafe in python, this is also
52+
# true for `catch_warnings` or `suppress_warinings`. In NumPy 1.12
53+
# however, `assert_equal` and the array comparison asserts, use this
54+
# to filter out some comparison warnings. Since removing this filter
55+
# may also affect downstream projects and skimage (and possibly more)
56+
# do parallel manual parallel testing using `assert_equal`, a quick fix
57+
# seems to be to lock the less obvious threading trap. Ideally (in
58+
# master this is the case), there should simply not be warning filter
59+
# logic in the assert functions themself.
60+
# While probably not perfectly safe in principle, it is sufficient
61+
# in the case of skimage and probably most testing scenarios and the
62+
# chance of deadlocks seems very unlikely.
63+
_assert_comparison_lock = Lock()
64+
4565
KnownFailureTest = KnownFailureException # backwards compat
4666
verbose = 0
4767

@@ -395,15 +415,17 @@ def assert_equal(actual,desired,err_msg='',verbose=True):
395415
except (TypeError, ValueError, NotImplementedError):
396416
pass
397417

398-
# Explicitly use __eq__ for comparison, ticket #2552
399-
with suppress_warnings() as sup:
418+
# Put lock around the warning filter, see comment at lock definition
419+
with _assert_comparison_lock, suppress_warnings() as sup:
400420
# TODO: Better handling will to needed when change happens!
401421
sup.filter(DeprecationWarning, ".*NAT ==")
402422
sup.filter(FutureWarning, ".*NAT ==")
423+
# Explicitly use __eq__ for comparison, ticket #2552
403424
if not (desired == actual):
404425
raise AssertionError(msg)
405426

406427

428+
407429
def print_assert_equal(test_string, actual, desired):
408430
"""
409431
Test if two objects are equal, and print an error message if test fails.
@@ -691,7 +713,8 @@ def safe_comparison(*args, **kwargs):
691713
# pass (or maybe eventually catch the errors and return False, I
692714
# dunno, that's a little trickier and we can figure that out when the
693715
# time comes).
694-
with suppress_warnings() as sup:
716+
# Note: Put a lock around warning filter (comment at lock definition)
717+
with _assert_comparison_lock, suppress_warnings() as sup:
695718
sup.filter(DeprecationWarning, ".*==")
696719
sup.filter(FutureWarning, ".*==")
697720
return comparison(*args, **kwargs)

0 commit comments

Comments
 (0)
0