From d8715681a23c869f77828f35ffd543125ca78945 Mon Sep 17 00:00:00 2001
From: Mathias Hauser <mathias.hauser@env.ethz.ch>
Date: Fri, 5 Feb 2021 19:28:49 +0100
Subject: [PATCH 1/6] ensure warnings cannot become errors in assert_

---
 xarray/testing.py            | 79 +++++++++++++++++++++---------------
 xarray/tests/test_testing.py | 27 ++++++++++++
 2 files changed, 74 insertions(+), 32 deletions(-)

diff --git a/xarray/testing.py b/xarray/testing.py
index ca72a4bee8e..64a8cfca50a 100644
--- a/xarray/testing.py
+++ b/xarray/testing.py
@@ -1,5 +1,6 @@
 """Testing functions exposed to the user API"""
 import functools
+import warnings
 from typing import Hashable, Set, Union
 
 import numpy as np
@@ -60,13 +61,18 @@ def assert_equal(a, b):
     numpy.testing.assert_array_equal
     """
     __tracebackhide__ = True
-    assert type(a) == type(b)
-    if isinstance(a, (Variable, DataArray)):
-        assert a.equals(b), formatting.diff_array_repr(a, b, "equals")
-    elif isinstance(a, Dataset):
-        assert a.equals(b), formatting.diff_dataset_repr(a, b, "equals")
-    else:
-        raise TypeError("{} not supported by assertion comparison".format(type(a)))
+
+    # sometimes tests elevate warnings to errors -> make sure that does not happen here
+    with warnings.catch_warnings():
+        warnings.simplefilter("always")
+
+        assert type(a) == type(b)
+        if isinstance(a, (Variable, DataArray)):
+            assert a.equals(b), formatting.diff_array_repr(a, b, "equals")
+        elif isinstance(a, Dataset):
+            assert a.equals(b), formatting.diff_dataset_repr(a, b, "equals")
+        else:
+            raise TypeError("{} not supported by assertion comparison".format(type(a)))
 
 
 def assert_identical(a, b):
@@ -87,16 +93,21 @@ def assert_identical(a, b):
     assert_equal, assert_allclose, Dataset.equals, DataArray.equals
     """
     __tracebackhide__ = True
-    assert type(a) == type(b)
-    if isinstance(a, Variable):
-        assert a.identical(b), formatting.diff_array_repr(a, b, "identical")
-    elif isinstance(a, DataArray):
-        assert a.name == b.name
-        assert a.identical(b), formatting.diff_array_repr(a, b, "identical")
-    elif isinstance(a, (Dataset, Variable)):
-        assert a.identical(b), formatting.diff_dataset_repr(a, b, "identical")
-    else:
-        raise TypeError("{} not supported by assertion comparison".format(type(a)))
+
+    # sometimes tests elevate warnings to errors -> make sure that does not happen here
+    with warnings.catch_warnings():
+        warnings.simplefilter("always")
+
+        assert type(a) == type(b)
+        if isinstance(a, Variable):
+            assert a.identical(b), formatting.diff_array_repr(a, b, "identical")
+        elif isinstance(a, DataArray):
+            assert a.name == b.name
+            assert a.identical(b), formatting.diff_array_repr(a, b, "identical")
+        elif isinstance(a, (Dataset, Variable)):
+            assert a.identical(b), formatting.diff_dataset_repr(a, b, "identical")
+        else:
+            raise TypeError("{} not supported by assertion comparison".format(type(a)))
 
 
 def assert_allclose(a, b, rtol=1e-05, atol=1e-08, decode_bytes=True):
@@ -138,21 +149,25 @@ def compat_variable(a, b):
 
         return a.dims == b.dims and (a._data is b._data or equiv(a.data, b.data))
 
-    if isinstance(a, Variable):
-        allclose = compat_variable(a, b)
-        assert allclose, formatting.diff_array_repr(a, b, compat=equiv)
-    elif isinstance(a, DataArray):
-        allclose = utils.dict_equiv(
-            a.coords, b.coords, compat=compat_variable
-        ) and compat_variable(a.variable, b.variable)
-        assert allclose, formatting.diff_array_repr(a, b, compat=equiv)
-    elif isinstance(a, Dataset):
-        allclose = a._coord_names == b._coord_names and utils.dict_equiv(
-            a.variables, b.variables, compat=compat_variable
-        )
-        assert allclose, formatting.diff_dataset_repr(a, b, compat=equiv)
-    else:
-        raise TypeError("{} not supported by assertion comparison".format(type(a)))
+    # sometimes tests elevate warnings to errors -> make sure that does not happen here
+    with warnings.catch_warnings():
+        warnings.simplefilter("always")
+
+        if isinstance(a, Variable):
+            allclose = compat_variable(a, b)
+            assert allclose, formatting.diff_array_repr(a, b, compat=equiv)
+        elif isinstance(a, DataArray):
+            allclose = utils.dict_equiv(
+                a.coords, b.coords, compat=compat_variable
+            ) and compat_variable(a.variable, b.variable)
+            assert allclose, formatting.diff_array_repr(a, b, compat=equiv)
+        elif isinstance(a, Dataset):
+            allclose = a._coord_names == b._coord_names and utils.dict_equiv(
+                a.variables, b.variables, compat=compat_variable
+            )
+            assert allclose, formatting.diff_dataset_repr(a, b, compat=equiv)
+        else:
+            raise TypeError("{} not supported by assertion comparison".format(type(a)))
 
 
 def _format_message(x, y, err_msg, verbose):
diff --git a/xarray/tests/test_testing.py b/xarray/tests/test_testing.py
index 30ea6aaaee9..a9f04d42f3b 100644
--- a/xarray/tests/test_testing.py
+++ b/xarray/tests/test_testing.py
@@ -1,3 +1,5 @@
+import warnings
+
 import numpy as np
 import pytest
 
@@ -127,3 +129,28 @@ def test_assert_duckarray_equal(duckarray, obj1, obj2):
     b = duckarray(obj2)
 
     xr.testing.assert_duckarray_equal(a, b)
+
+
+@pytest.mark.parametrize(
+    "func", ["assert_equal", "assert_identical", "assert_allclose"]
+)
+def test_ensure_warnings_not_elevated(func):
+    # make sure warnings are not elevated to errors in the assertion functions
+    # e.g. by @pytest.mark.filterwarnings("error")
+    # see https://github.com/pydata/xarray/pull/4760#issuecomment-774101639
+
+    # define a custom Variable class that raises a warning in assert_*
+    class WarningVariable(xr.Variable):
+        @property  # type: ignore
+        def dims(self):
+            warnings.warn("warning in test")
+            return super().dims
+
+    a = WarningVariable("x", [1])
+    b = WarningVariable("x", [2])
+
+    with warnings.catch_warnings(record=True):
+        # elevate warnings to errors
+        warnings.filterwarnings("error")
+        with pytest.raises(AssertionError):
+            getattr(xr.testing, func)(a, b)

From 6cc3b0908a0a20d7b84b6a205e3516f6be04be4f Mon Sep 17 00:00:00 2001
From: Mathias Hauser <mathias.hauser@env.ethz.ch>
Date: Fri, 5 Feb 2021 19:39:26 +0100
Subject: [PATCH 2/6] also assert_duckarray_allclose

---
 xarray/testing.py            | 11 +++++++++--
 xarray/tests/test_testing.py |  8 +++++++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/xarray/testing.py b/xarray/testing.py
index 64a8cfca50a..a0fed112d82 100644
--- a/xarray/testing.py
+++ b/xarray/testing.py
@@ -203,8 +203,15 @@ def assert_duckarray_allclose(
     """ Like `np.testing.assert_allclose`, but for duckarrays. """
     __tracebackhide__ = True
 
-    allclose = duck_array_ops.allclose_or_equiv(actual, desired, rtol=rtol, atol=atol)
-    assert allclose, _format_message(actual, desired, err_msg=err_msg, verbose=verbose)
+    with warnings.catch_warnings():
+        warnings.simplefilter("always")
+
+        allclose = duck_array_ops.allclose_or_equiv(
+            actual, desired, rtol=rtol, atol=atol
+        )
+        assert allclose, _format_message(
+            actual, desired, err_msg=err_msg, verbose=verbose
+        )
 
 
 def assert_duckarray_equal(x, y, err_msg="", verbose=True):
diff --git a/xarray/tests/test_testing.py b/xarray/tests/test_testing.py
index a9f04d42f3b..705337d7d6b 100644
--- a/xarray/tests/test_testing.py
+++ b/xarray/tests/test_testing.py
@@ -132,7 +132,13 @@ def test_assert_duckarray_equal(duckarray, obj1, obj2):
 
 
 @pytest.mark.parametrize(
-    "func", ["assert_equal", "assert_identical", "assert_allclose"]
+    "func",
+    [
+        "assert_equal",
+        "assert_identical",
+        "assert_allclose",
+        "assert_duckarray_allclose",
+    ],
 )
 def test_ensure_warnings_not_elevated(func):
     # make sure warnings are not elevated to errors in the assertion functions

From 7fee3138419e084a2c79f8d85d30a5fc714379c4 Mon Sep 17 00:00:00 2001
From: Mathias Hauser <mathias.hauser@env.ethz.ch>
Date: Fri, 5 Feb 2021 19:43:58 +0100
Subject: [PATCH 3/6] add whats new

---
 doc/whats-new.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/whats-new.rst b/doc/whats-new.rst
index 471e91a8512..65ed4be8c00 100644
--- a/doc/whats-new.rst
+++ b/doc/whats-new.rst
@@ -126,6 +126,8 @@ Internal Changes
   in ipython (:issue:`4741`, :pull:`4742`). By `Richard Kleijn <https://github.com/rhkleijn>`_.
 - Added the ``set_close`` method to ``Dataset`` and ``DataArray`` for beckends to specify how to voluntary release
   all resources. (:pull:`#4809`), By `Alessandro Amici <https://github.com/alexamici>`_.
+- Ensure warnings cannot be turned into exceptions in :py:func:`testing.assert_equal` and
+  the other `assert_*` functions (:pull:`4864`). By `Mathias Hauser <https://github.com/mathause>`_.
 
 .. _whats-new.0.16.2:
 
@@ -142,7 +144,7 @@ Deprecations
 
 - :py:attr:`~core.accessor_dt.DatetimeAccessor.weekofyear` and :py:attr:`~core.accessor_dt.DatetimeAccessor.week`
   have been deprecated. Use ``DataArray.dt.isocalendar().week``
-  instead (:pull:`4534`). By `Mathias Hauser <https://github.com/mathause>`_,
+  instead (:pull:`4534`). By `Mathias Hauser <https://github.com/mathause>`_.
   `Maximilian Roos <https://github.com/max-sixty>`_, and `Spencer Clark <https://github.com/spencerkclark>`_.
 - :py:attr:`DataArray.rolling` and :py:attr:`Dataset.rolling` no longer support passing ``keep_attrs``
   via its constructor. Pass ``keep_attrs`` via the applied function, i.e. use

From dd573e58ce4547528e7da0a4cd2358e999a54527 Mon Sep 17 00:00:00 2001
From: Mathias Hauser <mathias.hauser@env.ethz.ch>
Date: Fri, 5 Feb 2021 19:49:53 +0100
Subject: [PATCH 4/6] ensure warnings are raised

---
 xarray/tests/test_testing.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xarray/tests/test_testing.py b/xarray/tests/test_testing.py
index 705337d7d6b..bbab76e5eca 100644
--- a/xarray/tests/test_testing.py
+++ b/xarray/tests/test_testing.py
@@ -155,8 +155,10 @@ def dims(self):
     a = WarningVariable("x", [1])
     b = WarningVariable("x", [2])
 
-    with warnings.catch_warnings(record=True):
+    with warnings.catch_warnings(record=True) as w:
         # elevate warnings to errors
         warnings.filterwarnings("error")
         with pytest.raises(AssertionError):
             getattr(xr.testing, func)(a, b)
+
+        assert len(w) > 0

From 7a04898153c086f0c141a0cc26203216dbd70a03 Mon Sep 17 00:00:00 2001
From: Mathias Hauser <mathause@users.noreply.github.com>
Date: Fri, 5 Feb 2021 21:36:45 +0100
Subject: [PATCH 5/6] Update doc/whats-new.rst

---
 doc/whats-new.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/whats-new.rst b/doc/whats-new.rst
index 65ed4be8c00..258b8b0dde1 100644
--- a/doc/whats-new.rst
+++ b/doc/whats-new.rst
@@ -127,7 +127,7 @@ Internal Changes
 - Added the ``set_close`` method to ``Dataset`` and ``DataArray`` for beckends to specify how to voluntary release
   all resources. (:pull:`#4809`), By `Alessandro Amici <https://github.com/alexamici>`_.
 - Ensure warnings cannot be turned into exceptions in :py:func:`testing.assert_equal` and
-  the other `assert_*` functions (:pull:`4864`). By `Mathias Hauser <https://github.com/mathause>`_.
+  the other ``assert_*`` functions (:pull:`4864`). By `Mathias Hauser <https://github.com/mathause>`_.
 
 .. _whats-new.0.16.2:
 

From adf48da47d0090caa75dedec3f30b4d2061e81c6 Mon Sep 17 00:00:00 2001
From: Mathias Hauser <mathias.hauser@env.ethz.ch>
Date: Sun, 7 Feb 2021 17:50:38 +0100
Subject: [PATCH 6/6] use a decorator

---
 xarray/testing.py            | 107 +++++++++++++++++------------------
 xarray/tests/test_testing.py |   5 ++
 2 files changed, 57 insertions(+), 55 deletions(-)

diff --git a/xarray/testing.py b/xarray/testing.py
index a0fed112d82..1b68a5f1b0c 100644
--- a/xarray/testing.py
+++ b/xarray/testing.py
@@ -22,6 +22,19 @@
 )
 
 
+def ensure_warnings(func):
+    # sometimes tests elevate warnings to errors
+    # -> make sure that does not happen in the assert_* functions
+    @functools.wraps(func)
+    def wrapper(*args, **kwargs):
+        with warnings.catch_warnings():
+            warnings.simplefilter("always")
+
+            return func(*args, **kwargs)
+
+    return wrapper
+
+
 def _decode_string_data(data):
     if data.dtype.kind == "S":
         return np.core.defchararray.decode(data, "utf-8", "replace")
@@ -39,6 +52,7 @@ def _data_allclose_or_equiv(arr1, arr2, rtol=1e-05, atol=1e-08, decode_bytes=Tru
         return duck_array_ops.allclose_or_equiv(arr1, arr2, rtol=rtol, atol=atol)
 
 
+@ensure_warnings
 def assert_equal(a, b):
     """Like :py:func:`numpy.testing.assert_array_equal`, but for xarray
     objects.
@@ -61,20 +75,16 @@ def assert_equal(a, b):
     numpy.testing.assert_array_equal
     """
     __tracebackhide__ = True
-
-    # sometimes tests elevate warnings to errors -> make sure that does not happen here
-    with warnings.catch_warnings():
-        warnings.simplefilter("always")
-
-        assert type(a) == type(b)
-        if isinstance(a, (Variable, DataArray)):
-            assert a.equals(b), formatting.diff_array_repr(a, b, "equals")
-        elif isinstance(a, Dataset):
-            assert a.equals(b), formatting.diff_dataset_repr(a, b, "equals")
-        else:
-            raise TypeError("{} not supported by assertion comparison".format(type(a)))
+    assert type(a) == type(b)
+    if isinstance(a, (Variable, DataArray)):
+        assert a.equals(b), formatting.diff_array_repr(a, b, "equals")
+    elif isinstance(a, Dataset):
+        assert a.equals(b), formatting.diff_dataset_repr(a, b, "equals")
+    else:
+        raise TypeError("{} not supported by assertion comparison".format(type(a)))
 
 
+@ensure_warnings
 def assert_identical(a, b):
     """Like :py:func:`xarray.testing.assert_equal`, but also matches the
     objects' names and attributes.
@@ -93,23 +103,19 @@ def assert_identical(a, b):
     assert_equal, assert_allclose, Dataset.equals, DataArray.equals
     """
     __tracebackhide__ = True
-
-    # sometimes tests elevate warnings to errors -> make sure that does not happen here
-    with warnings.catch_warnings():
-        warnings.simplefilter("always")
-
-        assert type(a) == type(b)
-        if isinstance(a, Variable):
-            assert a.identical(b), formatting.diff_array_repr(a, b, "identical")
-        elif isinstance(a, DataArray):
-            assert a.name == b.name
-            assert a.identical(b), formatting.diff_array_repr(a, b, "identical")
-        elif isinstance(a, (Dataset, Variable)):
-            assert a.identical(b), formatting.diff_dataset_repr(a, b, "identical")
-        else:
-            raise TypeError("{} not supported by assertion comparison".format(type(a)))
+    assert type(a) == type(b)
+    if isinstance(a, Variable):
+        assert a.identical(b), formatting.diff_array_repr(a, b, "identical")
+    elif isinstance(a, DataArray):
+        assert a.name == b.name
+        assert a.identical(b), formatting.diff_array_repr(a, b, "identical")
+    elif isinstance(a, (Dataset, Variable)):
+        assert a.identical(b), formatting.diff_dataset_repr(a, b, "identical")
+    else:
+        raise TypeError("{} not supported by assertion comparison".format(type(a)))
 
 
+@ensure_warnings
 def assert_allclose(a, b, rtol=1e-05, atol=1e-08, decode_bytes=True):
     """Like :py:func:`numpy.testing.assert_allclose`, but for xarray objects.
 
@@ -149,25 +155,21 @@ def compat_variable(a, b):
 
         return a.dims == b.dims and (a._data is b._data or equiv(a.data, b.data))
 
-    # sometimes tests elevate warnings to errors -> make sure that does not happen here
-    with warnings.catch_warnings():
-        warnings.simplefilter("always")
-
-        if isinstance(a, Variable):
-            allclose = compat_variable(a, b)
-            assert allclose, formatting.diff_array_repr(a, b, compat=equiv)
-        elif isinstance(a, DataArray):
-            allclose = utils.dict_equiv(
-                a.coords, b.coords, compat=compat_variable
-            ) and compat_variable(a.variable, b.variable)
-            assert allclose, formatting.diff_array_repr(a, b, compat=equiv)
-        elif isinstance(a, Dataset):
-            allclose = a._coord_names == b._coord_names and utils.dict_equiv(
-                a.variables, b.variables, compat=compat_variable
-            )
-            assert allclose, formatting.diff_dataset_repr(a, b, compat=equiv)
-        else:
-            raise TypeError("{} not supported by assertion comparison".format(type(a)))
+    if isinstance(a, Variable):
+        allclose = compat_variable(a, b)
+        assert allclose, formatting.diff_array_repr(a, b, compat=equiv)
+    elif isinstance(a, DataArray):
+        allclose = utils.dict_equiv(
+            a.coords, b.coords, compat=compat_variable
+        ) and compat_variable(a.variable, b.variable)
+        assert allclose, formatting.diff_array_repr(a, b, compat=equiv)
+    elif isinstance(a, Dataset):
+        allclose = a._coord_names == b._coord_names and utils.dict_equiv(
+            a.variables, b.variables, compat=compat_variable
+        )
+        assert allclose, formatting.diff_dataset_repr(a, b, compat=equiv)
+    else:
+        raise TypeError("{} not supported by assertion comparison".format(type(a)))
 
 
 def _format_message(x, y, err_msg, verbose):
@@ -197,23 +199,18 @@ def _format_message(x, y, err_msg, verbose):
     return "\n".join(parts)
 
 
+@ensure_warnings
 def assert_duckarray_allclose(
     actual, desired, rtol=1e-07, atol=0, err_msg="", verbose=True
 ):
     """ Like `np.testing.assert_allclose`, but for duckarrays. """
     __tracebackhide__ = True
 
-    with warnings.catch_warnings():
-        warnings.simplefilter("always")
-
-        allclose = duck_array_ops.allclose_or_equiv(
-            actual, desired, rtol=rtol, atol=atol
-        )
-        assert allclose, _format_message(
-            actual, desired, err_msg=err_msg, verbose=verbose
-        )
+    allclose = duck_array_ops.allclose_or_equiv(actual, desired, rtol=rtol, atol=atol)
+    assert allclose, _format_message(actual, desired, err_msg=err_msg, verbose=verbose)
 
 
+@ensure_warnings
 def assert_duckarray_equal(x, y, err_msg="", verbose=True):
     """ Like `np.testing.assert_array_equal`, but for duckarrays """
     __tracebackhide__ = True
diff --git a/xarray/tests/test_testing.py b/xarray/tests/test_testing.py
index bbab76e5eca..b6dec846c54 100644
--- a/xarray/tests/test_testing.py
+++ b/xarray/tests/test_testing.py
@@ -137,6 +137,7 @@ def test_assert_duckarray_equal(duckarray, obj1, obj2):
         "assert_equal",
         "assert_identical",
         "assert_allclose",
+        "assert_duckarray_equal",
         "assert_duckarray_allclose",
     ],
 )
@@ -152,6 +153,10 @@ def dims(self):
             warnings.warn("warning in test")
             return super().dims
 
+        def __array__(self):
+            warnings.warn("warning in test")
+            return super().__array__()
+
     a = WarningVariable("x", [1])
     b = WarningVariable("x", [2])
 
