Skip to content

ENH: test astype with complex inputs #311

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

Merged
merged 3 commits into from
Nov 28, 2024
Merged

Conversation

ev-br
Copy link
Member

@ev-br ev-br commented Nov 18, 2024

In fact, I'm not entirely sure if the spec means to prohibit `astype(complex_array, real_dtype):

Casting a complex floating-point array to a real-valued data type should not be permitted.

but at least array_api_strict allows it and emits the same warning is does numpy:

>>> import array_api_strict as xp
>>> xp.astype(xp.ones(3, dtype=xp.complex128), xp.float64)
/home/br/miniforge3/envs/array-api-tests/lib/python3.11/site-packages/array_api_strict/_data_type_functions.py:45: ComplexWarning: Casting complex values to real discards the imaginary part
  return Array._new(x._array.astype(dtype=dtype._np_dtype, copy=copy), device=device)
Array([1., 1., 1.], dtype=array_api_strict.float64)

@asmeurer
Copy link
Member

"Casting a complex floating-point array to a real-valued data type should not be permitted."

https://data-apis.org/array-api/latest/API_specification/generated/array_api.astype.html#astype

So that probably needs to be fixed in array-api-strict, and based on the language in the standard, we should test that it actually raises an exception (many libraries will fail that test).

@ev-br
Copy link
Member Author

ev-br commented Nov 23, 2024

Re-reading the phrase again, "casting ... should not be permitted", ISTM it's the RFC2119 should not, so a conforming implementation is allowed to cast and emit a warning?

@asmeurer
Copy link
Member

Yes, that looks right. Of course, the whole point of array-api-strict is to treat all "should"s in the standard as if they were "must"s. But this does mean that we don't necessarily have to update the test suite to check this, or array-api-compat to make numpy issue an exception instead of a warning.

That's assuming the "should" there was intentional. It also says, "in order to avoid ambiguity and to promote clarity, this specification requires that array API consumers explicitly express which component should be cast to a specified real-valued data type." which sort of implies that it is required. I had thought I remembered deciding that it would be a required error, but maybe it was indeed decided to make it implementation dependent instead to avoid requiring numpy to change here.

@ev-br
Copy link
Member Author

ev-br commented Nov 26, 2024

Okay, pushed an update to skip astype(complex, not complex) dtype combinations to accompany data-apis/array-api-strict#101

@asmeurer asmeurer self-requested a review November 26, 2024 22:48
@ev-br ev-br merged commit f1c3ed2 into data-apis:master Nov 28, 2024
4 checks passed
@ev-br
Copy link
Member Author

ev-br commented Nov 28, 2024

Thanks for the review Aaron!

@ev-br ev-br mentioned this pull request Nov 28, 2024
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants