ENH: add a test for can_cast(complex dtypes) #320
Conversation
| @@ -19,10 +19,18 @@ def non_complex_dtypes(): | |||
| return xps.boolean_dtypes() | hh.real_dtypes | |||
|
|
|||
|
|
|||
| def numeric_dtypes(): | |||
There was a problem hiding this comment.
There's already hh.numeric_dtypes, which is very different from what you're defining here ("numeric dtype" is a term from the standard).
At any rate, can_cast is supposed to work with all dtypes, so this should be using hh.all_dtypes. https://data-apis.org/array-api/latest/API_specification/generated/array_api.can_cast.html#can-cast
| else to) | ||
|
|
||
| from_min, from_max = dh.dtype_ranges[from_dtype] | ||
| to_min, to_max = dh.dtype_ranges[to_dtype] | ||
| expected = from_min >= to_min and from_max <= to_max |
There was a problem hiding this comment.
I don't understand why the logic in this function is written this way. It should just check against the type promotion table, and assert True if it is in there (and nothing if it isn't, since as the comment notes libraries should still choose to do this casting).
I guess maybe it should disallow complex downcasting, although this isn't particularly spelled out in the standard, so it would be better to get clarity there first.
There was a problem hiding this comment.
There are helper functions in dtype_helpers.py to check the type promotion table.
There was a problem hiding this comment.
Well, I don't know why it was written this way. At a guess, could be related to this wording from https://data-apis.org/array-api/latest/API_specification/type_promotion.html#type-promotion
Behavior is also not specified for integers outside of the bounds of a given integer data type. Integers outside of bounds may result in overflow or an error.
But AFAICS it's only related to python scalars, which we're not testing here anyway. So the last commit removes this logic altogether.
ev-br
force-pushed
the
can_cast_cmplx
branch
from
8d77a2a to
8a61100
Compare
1) test all dtypes 2) check against the promotion table 3) remove checking the limits (value-based casting?)
ev-br
force-pushed
the
can_cast_cmplx
branch
from
8a61100 to
9f28ec8
Compare
|
This looks much simpler now. |
| if expected: | ||
| # cross-kind casting is not explicitly disallowed. We can only test | ||
| # the cases where it should return True. TODO: if expected=False, | ||
| # check that the array library actually allows such casts. |
There was a problem hiding this comment.
We could in principle do this (can_cast should really match what casting the functions in the API allow). I don't know if this sort of test is feasible, though.
There was a problem hiding this comment.
Okay, restored the TODO code comment. I've no idea either if this sort of test is feasible or not, so let's kick this can down the road.
|
Let's get this in, and see if this causes issues down the road. Thank you Aaron for the review. |
cross-ref #301