-
-
Notifications
You must be signed in to change notification settings - Fork 11.1k
Description
Proposed new feature or change:
(Discussed in #23912 (comment))
The function np.clip
arguably has surprising casting behaviour:
a = np.arange(5, dtype='u1')
np.clip(a, -1, 3)
# OverflowError with NEP 50
np.clip(a, np.int64(-1), np.int64(3))
# array([0, 1, 2, 3, 3])
# int64 dtype with NEP 50
# (before NEP 50, both examples gave int16)
I would naively have expected for the output dtype to always be the same as the input one. That this does not happen is because internally np.clip
calls a ufunc:
Lines 92 to 101 in d885b0b
def _clip(a, min=None, max=None, out=None, **kwargs): | |
if min is None and max is None: | |
raise ValueError("One of max or min must be given") | |
if min is None: | |
return um.minimum(a, max, out=out, **kwargs) | |
elif max is None: | |
return um.maximum(a, min, out=out, **kwargs) | |
else: | |
return um.clip(a, min, max, out=out, **kwargs) |
and these treat the arguments symmetrically.
It is possible to get the output dtype by setting out
or dtype
, but in the current implementation that still gives either the OverflowError
or casting errors:
np.clip(a, np.int64(-1), np.int64(3), out=a) # or dtype=a.dtype
# UFuncTypeError: Cannot cast ufunc 'clip' output from dtype('int64') to dtype('uint8') with casting rule 'same_kind'
adding casting="unsafe"
gives the wrong answer, because -1
becomes 255
.
I think it should be possible to make the np.clip
function (probably not the ufunc
) cast the min
and max
to a.dtype
, but ensure that the valid ranges are respected (i.e., negative integers would become 0 if the dtype is unsigned). This would be similar to what was done in #24915, i.e., ideally we have the behaviour of np.clip
be identical to
min = -1
max = 3
out = a.copy()
out[a<min] = min
out[a>max] = max
return out
(which still gives an out-of-bound error for min=-1
because of __setitem__
, but works for min=np.int64(-1)
)
But perhaps this is more work than is warranted.