WIP: ENH/TST: `xp_assert_` enhancements #267

mdhaber · 2025-04-16T17:55:03Z

Toward #17. As discussed in mdhaber/marray#110 (comment).

This adds a few of the features from SciPy's xp_assert_ functions to the xpx xp_assert functions. Tests will probably fail in this first commit; I'll comment inline.

I decided not to add a check_namespace toggle: if check_namespace is False and the namespaces don't match, other operations might fail. I didn't want to work on fixing this if it's not an important feature. There is only one file in SciPy that uses check_namespace=False, and it probably shouldn't.

Once this is looking good, I'll add xp_assert_less as a clone of xp_assert_equal.

mdhaber

Easier to review with "Hide whitespace". @lucascolley do you see what is going on with

FAILED tests/test_testing.py::test_assert_close_equal_shape[sparse-True-xp_assert_close]
FAILED tests/test_testing.py::test_assert_close_equal_dtype[sparse-True-xp_assert_close]

?

Also, it looks to me like the codecov failures are misleading. It doesn't look to me like the new code is actually missing coverage if tests are run with all backends.

mdhaber · 2025-04-16T17:55:21Z

src/array_api_extra/_lib/_testing.py

@@ -9,13 +9,15 @@
 from types import ModuleType
 from typing import cast

+import numpy as np


Is this OK? Sometimes it was imported within test functions below.

mdhaber · 2025-04-16T17:56:11Z

src/array_api_extra/_lib/_testing.py

+        msg = f"dtypes do not match: {actual.dtype} != {desired.dtype}"
+        assert actual.dtype == desired.dtype, msg
+
+    if is_numpy_namespace(actual_xp) and check_scalar:


Suggested change

if is_numpy_namespace(actual_xp) and check_scalar:

if is_numpy_namespace(actual_xp) and check_arrayness:

?
I seem to remember some discussion about naming this parameter when adding to SciPy (check_0d). I don't really care what it is called. check_type is also on the table.

mdhaber · 2025-04-16T17:57:06Z

src/array_api_extra/_lib/_testing.py

    if is_pydata_sparse_namespace(xp):
        return array.todense()  # type: ignore[attr-defined]  # pyright: ignore[reportAttributeAccessIssue]
    if is_array_api_strict_namespace(xp):
        # Note: we deliberately did not add a `.to_device` method in _typing.pyi
        # even if it is required by the standard as many backends don't support it
        return array.to_device(xp.Device("CPU_DEVICE"))  # type: ignore[attr-defined]  # pyright: ignore[reportAttributeAccessIssue]
-    # Note: nothing to do for CuPy, because it uses a bespoke test function
+    if is_cupy_namespace(xp):
+        return xp.asnumpy(array)


This is effectively what CuPy does anyway.
https://github.com/cupy/cupy/blob/1794c835a086dda4cfb82d1152a495b61a82cbdc/cupy/testing/_array.py#L11
If torch is the only backend that really defines its own assertions, and if it doesn't even have an assert_equal equivalent, might as well just always use NumPy.

mdhaber · 2025-04-16T17:59:39Z

tests/test_testing.py

@@ -24,7 +25,9 @@
        xp_assert_equal,
        pytest.param(
            xp_assert_close,
-            marks=pytest.mark.xfail_xp_backend(Backend.SPARSE, reason="no isdtype"),
+            marks=pytest.mark.xfail_xp_backend(
+                Backend.SPARSE, reason="no isdtype", strict=False


I can't figure out what's going on with the sparse failures. I thought by using param_assert_equal_close as a decorator would avoid testing with sparse?

mdhaber · 2025-04-16T18:00:10Z

src/array_api_extra/_lib/_testing.py

-            atol=atol,
-            err_msg=err_msg,
-        )
+    # JAX/Dask arrays work directly with `np.testing`


Do they need to be transfered from GPU to CPU?

mdhaber · 2025-04-16T18:00:58Z

src/array_api_extra/_lib/_testing.py

+def xp_assert_equal(
+    actual: Array,
+    desired: Array,
+    *,


I assume err_msg was intended to be keyword-only. OK to make it so?

lucascolley · 2025-04-16T18:41:07Z

It doesn't look to me like the new code is actually missing coverage if tests are run with all backends.

Yeah, this is a constant problem. It eventually flips to passed once it registers the backends run. I think initial investigation didn't find a solution, but there is probably some sort of config/workaround to fix it.

lucascolley · 2025-04-16T18:57:04Z

The failures are XPASSes: [XPASS(strict)] sparse:no isdtype

Judging by the message, we're expecting the test to fail due to sparse not implementing isdtype, but it is now passing. Could that be because of any of your changes?

mdhaber · 2025-04-16T21:31:56Z

That's what I figured, which is why I tried to set the xfail strict=False in param_assert_equal_close. I guess that option is not respected as written.

The trouble is that when check_shape or check_dtype is False, _check_ns_shape_dtype passes, the code continues, and then we run into the AttributeError.

        xp = _check_ns_shape_dtype(actual, desired, check_dtype, check_shape, check_scalar)
    
       floating = xp.isdtype(actual.dtype, ("real floating", "complex floating"))
       # AttributeError: module 'sparse' has no attribute 'isdtype'. Did you mean: 'astype'?

So whether the test will fail with sparse arrays due to the AttributeError depends on check_shape/check_dtype. We cannot simply use the param_assert_equal_close decorator to xfail (strict) the sparse tests, nor can we remove it.

So what would you prefer:

Mark the test with pytest.xfail inside the test function considering the value of check_shape/check_dtype (in addition to the backend)
Create some alternative to param_assert_equal_close that effectively does the same thing as the previous option (but is harder for me to write)
Make the XFAIL with sparse arrays not strict (perhaps I can figure out why strict=False was not enough?)
Just skip the test with sparse arrays
Remove the parameterization and write two separate tests just to accommodate for sparse arrays
Something else?

If it were me, I would just do option 1. I have no qualms about using pytest.xfail within a function, but since someone went through the trouble of creating param_assert_equal_close, it seems that is not desirable to everyone.

lucascolley · 2025-04-16T21:49:03Z

Multiple of those options seem fine with me (at least for now), but I'll ping @crusaderky as the original author first

hameerabbasi · 2025-04-17T17:46:03Z

The failures are XPASSes: [XPASS(strict)] sparse:no isdtype

Judging by the message, we're expecting the test to fail due to sparse not implementing isdtype, but it is now passing. Could that be because of any of your changes?

We recently released 0.16.0; I believe that may have added isdtype.

mdhaber · 2025-04-17T18:26:36Z

@hameerabbasi If so, it does not appear to be documented at https://sparse.pydata.org/en/stable/api/. Is that where I should be looking?

Also - I'm not very familiar with the sparse project - is there somewhere that summarizes the current relationship between scipy.sparse and sparse projects?

hameerabbasi · 2025-04-17T20:13:29Z

Whoops: It's in a non-standard experimental part of the codebase; not exposed to the user. My bad.

lucascolley · 2025-04-17T20:27:48Z

is there somewhere that summarizes the current relationship between scipy.sparse and sparse projects?

https://labs.quansight.org/blog/sparse-array-ecosystem is probably a good start

WIP: xp_assert enhancements

1140a1a

mdhaber commented Apr 16, 2025

View reviewed changes

mdhaber marked this pull request as draft April 16, 2025 18:17

lucascolley changed the title ~~WIP: xp_assert_ enhancements~~ WIP: ENH: xp_assert_ enhancements Apr 16, 2025

lucascolley added enhancement New feature or request xpx.testing testing labels Apr 16, 2025

lucascolley changed the title ~~WIP: ENH: xp_assert_ enhancements~~ WIP: ENH/TST: xp_assert_ enhancements Apr 16, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: ENH/TST: `xp_assert_` enhancements #267

WIP: ENH/TST: `xp_assert_` enhancements #267

mdhaber commented Apr 16, 2025 •

edited

Loading

mdhaber left a comment •

edited

Loading

mdhaber Apr 16, 2025

mdhaber Apr 16, 2025 •

edited

Loading

mdhaber Apr 16, 2025 •

edited

Loading

mdhaber Apr 16, 2025

mdhaber Apr 16, 2025

mdhaber Apr 16, 2025 •

edited

Loading

lucascolley commented Apr 16, 2025 •

edited

Loading

lucascolley commented Apr 16, 2025

mdhaber commented Apr 16, 2025 •

edited

Loading

lucascolley commented Apr 16, 2025 •

edited

Loading

hameerabbasi commented Apr 17, 2025

mdhaber commented Apr 17, 2025 •

edited

Loading

hameerabbasi commented Apr 17, 2025

lucascolley commented Apr 17, 2025

	if is_numpy_namespace(actual_xp) and check_scalar:
	if is_numpy_namespace(actual_xp) and check_arrayness:

WIP: ENH/TST: xp_assert_ enhancements #267

Are you sure you want to change the base?

WIP: ENH/TST: xp_assert_ enhancements #267

Conversation

mdhaber commented Apr 16, 2025 • edited Loading

mdhaber left a comment • edited Loading

Choose a reason for hiding this comment

mdhaber Apr 16, 2025

Choose a reason for hiding this comment

mdhaber Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

mdhaber Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

mdhaber Apr 16, 2025

Choose a reason for hiding this comment

mdhaber Apr 16, 2025

Choose a reason for hiding this comment

mdhaber Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

lucascolley commented Apr 16, 2025 • edited Loading

lucascolley commented Apr 16, 2025

mdhaber commented Apr 16, 2025 • edited Loading

lucascolley commented Apr 16, 2025 • edited Loading

hameerabbasi commented Apr 17, 2025

mdhaber commented Apr 17, 2025 • edited Loading

hameerabbasi commented Apr 17, 2025

lucascolley commented Apr 17, 2025

WIP: ENH/TST: `xp_assert_` enhancements #267

WIP: ENH/TST: `xp_assert_` enhancements #267

mdhaber commented Apr 16, 2025 •

edited

Loading

mdhaber left a comment •

edited

Loading

mdhaber Apr 16, 2025 •

edited

Loading

mdhaber Apr 16, 2025 •

edited

Loading

mdhaber Apr 16, 2025 •

edited

Loading

lucascolley commented Apr 16, 2025 •

edited

Loading

mdhaber commented Apr 16, 2025 •

edited

Loading

lucascolley commented Apr 16, 2025 •

edited

Loading

mdhaber commented Apr 17, 2025 •

edited

Loading