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
DEP: Deprecate conversion of out-of-bound Python integers #22385
Changes from all commits
508722f
0a821c1
afcc560
eec99b4
7a951f9
0ca3a6d
fb44bd1
9434081
5f11024
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
Conversion of out-of-bound Python integers | ||
------------------------------------------ | ||
Attempting a conversion from a Python integer to a NumPy | ||
value will now always check whether the result can be | ||
represented by NumPy. This means the following examples will | ||
fail in the future and give a ``DeprecationWarning`` now:: | ||
|
||
np.uint8(-1) | ||
np.array([3000], dtype=np.int8) | ||
|
||
Many of these did succeed before. Such code was mainly | ||
useful for unsigned integers with negative values such as | ||
`np.uint8(-1)` giving `np.iinfo(np.uint8).max`. | ||
|
||
Note that conversion between NumPy integers is unaffected, | ||
so that `np.array(-1).astype(np.uint8)` continues to work | ||
and use C integer overflow logic. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2958,6 +2958,7 @@ PyUFunc_Reduce(PyUFuncObject *ufunc, | |
int iaxes, ndim; | ||
npy_bool reorderable; | ||
npy_bool axis_flags[NPY_MAXDIMS]; | ||
PyArrayObject *result = NULL; | ||
PyObject *identity; | ||
const char *ufunc_name = ufunc_get_name_cstr(ufunc); | ||
/* These parameters come from a TLS global */ | ||
|
@@ -2983,11 +2984,21 @@ PyUFunc_Reduce(PyUFuncObject *ufunc, | |
return NULL; | ||
} | ||
|
||
/* | ||
* Promote and fetch ufuncimpl (currently needed to fix up the identity). | ||
*/ | ||
PyArray_Descr *descrs[3]; | ||
PyArrayMethodObject *ufuncimpl = reducelike_promote_and_resolve(ufunc, | ||
arr, out, signature, NPY_FALSE, descrs, "reduce"); | ||
if (ufuncimpl == NULL) { | ||
return NULL; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a request from Numba to expose the ufunc loop used for a dtype. They did not explicitly ask for reductions, but I imagine that will be the next request. Will it be sufficient to expose There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I have to think abou the API. Could would probably consider a single function/entrypoint. But with Such new API might also just always live in a NEP 50 future. I don't think it matters much for this PR. To me the decision here seems mainly whether we prefer this churn, over the churn of having a special (internal for now) API for Python integers. (And maybe the timing of both) |
||
/* Get the identity */ | ||
/* TODO: Both of these should be provided by the ArrayMethod! */ | ||
identity = _get_identity(ufunc, &reorderable); | ||
if (identity == NULL) { | ||
return NULL; | ||
goto finish; | ||
} | ||
|
||
/* Get the initial value */ | ||
|
@@ -3003,33 +3014,42 @@ PyUFunc_Reduce(PyUFuncObject *ufunc, | |
initial = Py_None; | ||
Py_INCREF(initial); | ||
} | ||
else if (PyTypeNum_ISUNSIGNED(descrs[2]->type_num) | ||
&& PyLong_CheckExact(initial)) { | ||
/* | ||
* This is a bit of a hack until we have truly loop specific | ||
* identities. Python -1 cannot be cast to unsigned so convert | ||
* it to a NumPy scalar, but we use -1 for bitwise functions to | ||
* signal all 1s. | ||
* (A builtin identity would not overflow here, although we may | ||
* unnecessary convert 0 and 1.) | ||
*/ | ||
Py_SETREF(initial, PyObject_CallFunctionObjArgs( | ||
(PyObject *)&PyLongArrType_Type, initial, NULL)); | ||
if (initial == NULL) { | ||
goto finish; | ||
} | ||
} | ||
} else { | ||
Py_DECREF(identity); | ||
Py_INCREF(initial); /* match the reference count in the if above */ | ||
} | ||
|
||
PyArray_Descr *descrs[3]; | ||
PyArrayMethodObject *ufuncimpl = reducelike_promote_and_resolve(ufunc, | ||
arr, out, signature, NPY_FALSE, descrs, "reduce"); | ||
if (ufuncimpl == NULL) { | ||
Py_DECREF(initial); | ||
return NULL; | ||
} | ||
|
||
PyArrayMethod_Context context = { | ||
.caller = (PyObject *)ufunc, | ||
.method = ufuncimpl, | ||
.descriptors = descrs, | ||
}; | ||
|
||
PyArrayObject *result = PyUFunc_ReduceWrapper(&context, | ||
result = PyUFunc_ReduceWrapper(&context, | ||
arr, out, wheremask, axis_flags, reorderable, keepdims, | ||
initial, reduce_loop, ufunc, buffersize, ufunc_name, errormask); | ||
|
||
finish: | ||
for (int i = 0; i < 3; i++) { | ||
Py_DECREF(descrs[i]); | ||
} | ||
Py_DECREF(initial); | ||
Py_XDECREF(initial); | ||
return result; | ||
} | ||
|
||
|
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.
Maybe add the
astype()
workaround, i.e.np.array(-1, np.uint8)
will raise here, butnp.array(-1).astype(np.uint8)
will do the right thing? I am not sure how to express that this deep in the code path...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.
Good idea, also added a release note. If this becomes too painful for downstream, we could consider allowing
np.uint8(-1)
making it explicitly work for negative values. (I would still restrict this to the range of the corresponding signed integer, so-128
foruint8
)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.
we might need that as a fallback