array-api
array-api copied to clipboard
Add complex number support to `astype`
This PR:
-
adds complex number support to
astype
. Asastype
generally supports copying an array to any specified data type irrespective of type promotion rules, no exception to general behavior is made for complex number arrays.Accordingly, following C, NumPy, and others, when casting a complex floating-point array to a real-valued data type, the imaginary component must be discarded and only the real component cast to the desired data type.
Notes
- The alternative to discarding the imaginary component is to require users call
real(x)
before callingastype
. However, asastype
is more generally used to force a type conversion, prohibiting casting complex-valued arrays to real-valued arrays did not seem ideal, nor consistent with, say, casting a floating-point array to an integral array (i.e., we allow casting a floating-point array to an integral array and don't requireastype(floor(x))
). Hence, this PR chooses to follow existing precedent and require that the imaginary component be discarded when performing the conversion.
In [1]: z = np.asarray(1.5 + 2.6j)
In [2]: z.astype(dtype="float64")
<ipython-input-9-bea51b1612d9>:1: ComplexWarning: Casting complex values to real discards the imaginary part
z.astype(dtype="float64")
Out[2]: array(1.5)
In [3]: z.astype(dtype="bool")
Out[3]: array(True)
In [4]: np.asarray(True).astype(dtype="complex128")
Out[4]: array(1.+0.j)
In [5]: np.asarray(False).astype(dtype="complex128")
Out[5]: array(0.+0.j)
In [6]: z.astype(dtype="int32")
<ipython-input-13-5f154e24a33c>:1: ComplexWarning: Casting complex values to real discards the imaginary part
z.astype(dtype="int32")
Out[6]: array(1, dtype=int32)
In [7]: np.asarray(0+0j).astype(dtype="bool")
Out[7]: array(False)
@leofang Agreed. I don't think there is any appetite for requiring implementations to display a warning (ala NumPy).
Was about to say this should raise a warning 😂
That said, I kind of wonder if this is a good idea as opposed to simply forcing users to do .real.astype(...)
instead, which seems clearer. Having .astype(...)
shortcut this case seems prone to subtle bugs in user code
What about saying .astype(...)
behavior is undefined for types that are not complex. IOW this could still be used for upcasting from complex64
to complex128
and implementers might use the behavior defined here, but it shouldn't be expected
Based on discussion in today's meeting it sounds like a warning/error is inconsistent with the implied casting
of "unsafe"
that the spec currently uses. That said, there may be value in providing users a way to influence this behavior (IOW raise in some of these cases) to avoid subtle bugs. Have raised issue ( https://github.com/data-apis/array-api/issues/463 ) about adding this via a casting
flag.
Quoting myself from https://github.com/data-apis/array-api/issues/463#issuecomment-1178393667:
Based on https://github.com/data-apis/array-api/pull/427 (real) and https://github.com/data-apis/array-api/pull/446 (conj) I think we are leaning toward a more restrictive API design as opposed to permissive. I seem to remember the permissive choice was favored during one of the meetings, but I can't recall it was for which library (and I also forgot which side I picked, I'd say restrictive but I am not certain anymore after a long week...😅) That's probably why I let go in https://github.com/data-apis/array-api/pull/445 (astype). But, for the sake of consistency it'd be nicer if astype does not allow complex -> real?
This PR has now been updated to reflect the current workgroup consensus to disallow casting from a complex floating-point dtype to a real-valued dtype.
As discussed in the 3 November 2022 meeting, no objections were raised. This PR should be ready for merge.
@leofang would you mind taking one last look and then approving?
Thanks, @leofang! In it goes!