ENH: Add dtype argument in np.choose #19364
Conversation
| @@ -1857,7 +1857,7 @@ Item selection and manipulation | |||
|
|
|||
| .. c:function:: PyObject* PyArray_Choose( \ | |||
| PyArrayObject* self, PyObject* op, PyArrayObject* ret, \ | |||
| NPY_CLIPMODE clipmode) | |||
| PyArray_Descr *dtype, NPY_CLIPMODE clipmode) | |||
There was a problem hiding this comment.
You can't change this function, its public API and must remain ABI and API compatible. Same for PyArray_ConvertToCommonType, although you can make a private version and add a thin wrapper as public version.
I am confused about this, I thought np.choose worked with sequences, because PyArray_Choose does. But its not the case, the python code apparently only ever choose from a single array. To properly support sequences, the right thing is probably to re-use some of the concatenate logic. But since the Python code effectively always calls np.asarray() already, that might already be unnecessarily complicated.
There was a problem hiding this comment.
At first, I wrote a function PyArray_ConvertToDataType. But it almost used the same logic and the code was almost identical to PyArray_ConvertToCommonType. That's why I made some changes to that existing function.
Regarding the implementation, I looked into the np.concatenate source as you mentioned in the issue. But I thought this was much cleaner than that complex way. didn't really consider the fact that it was a public API.
There was a problem hiding this comment.
Yes, but PyArray_ConvertToCommonType is limited to using type-numbers. That is unfortunately not future-proof API, and we have no choice but stop using it and hope everyone follows.
There was a problem hiding this comment.
will it be a good idea to create a new function for np.choose and call PyArray_Choose from the new function and changing the dtype of the returned array?
There was a problem hiding this comment.
Sorry, I mid-slipped there. arr.choose uses arr as the index array, not for the choices. So it all adds up, and we are firmly at the same place where concatenate also lives.
Are you working of the current branch? I added PyArray_FindConcatenationDescriptor for concatenate, which I would hope you can just use pretty straight forward.
There was a problem hiding this comment.
Yes, I am working on the current branch. I will look into PyArray_FindConcatenationDescriptor.
Thank You.
seberg
added
the
64 - Good Idea
|
It is a good start, but needs work (can't chang epublic API), and as it is 2.5 years old by now closing. Please do not hesitate to make a new PR (also fo rsomeone else picking it up). |
Fixes GH-18798
This PR adds a dtype argument to
np.choose()function.