-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Unpin array api strict #10983
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
base: main
Are you sure you want to change the base?
Unpin array api strict #10983
Conversation
|
Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient. |
|
I don't understand why the mypy tests fail after these changes, so can't really come up with a way to fix this (except for giving an ignore to mypy :) ) |
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.
Array api strict is now typed since 2.4.0: data-apis/array-api-strict#135
This is the erroring code:
xarray/xarray/tests/test_dtypes.py
Lines 9 to 18 in 58594f8
| try: | |
| import array_api_strict | |
| except ImportError: | |
| class DummyArrayAPINamespace: | |
| bool = None # type: ignore[unused-ignore,var-annotated] | |
| int32 = None # type: ignore[unused-ignore,var-annotated] | |
| float64 = None # type: ignore[unused-ignore,var-annotated] | |
| array_api_strict = DummyArrayAPINamespace |
Redefinitions with different types is in general not a good idea for type safety. mypy choose the first one and goes from there. Since array api strict wasn't typed <2.4.0, the type was
Any which made the except case valid too, not anymore though.
We usually just ignore in these optional import cases, examples are here:
Lines 47 to 50 in 58594f8
| try: | |
| from dask.array import Array as DaskArray | |
| except ImportError: | |
| DaskArray = np.ndarray # type: ignore[misc, assignment, unused-ignore] |
| actual = xp_arr + 7 | ||
| assert isinstance(actual.data, Array) | ||
| assert_equal(actual, expected) | ||
| assert_equal(expected, actual) |
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.
I think array api strict had the right idea here.
It's not a good idea to assume different types of arrays will work with each other.
Should we instead explicitly convert to numpy?
actual = np.asarray(xp_arr + 7)
Closes #10427
In array-api-strict>2.4.1, arithmetic operations no longer accept NumPy arrays. By inverting the assert_equal variables, we use numpy's
__eq__instead of the array-api-strict__eq__