ENH: allow using dtype classes in array creation functions #23404
Conversation
|
After this,
Most of these should probably be updated to accept dtype classes as well, I think? |
|
Yeah, some of these (e.g. |
9576244 to
171dc85
Compare
|
Failure is due to a transient issue on anaconda.org |
There was a problem hiding this comment.
Cool, thanks. Sorry for the long N.B. I should move that to an issue probably (but will let it sink a bit more).
I would lean towards not passing newtype. Otherwise, I am worried that calling AdaptDTypeToArray is just too much at that place and as I said there, we could just skip it with two possible inclusions:
- Fast path if the descr matches the type (unless you think that we should guarantee canonical/native result, which I don't think really, the N.B. is a bit tricky, but...).
- If non-parametric, you could grab the singleton and use it.
Happy if you wish to drill in deeper, but this is just a fast-path in the end...
This enables writing np.array(some_object, dtype=type(np.dtype('i'))). This
is a follow-on from numpy#23154, see that PR
for more details.
I had to add a new include to `ctors.h` to bring in the definition of the
`npy_dtype_info` struct. Since `ctors.h` is included in many other files inside
numpy, I found that I needed to modify fewer includes across numpy if I moved
the definition of `npy_dtype_info` to `common.h` from `descriptor.h`. The new
includes of `common.h` are needed to support later includes of `ctors.h` in
those files. If anyone has an alternate place to put `npy_dtype_info` that would
cause less churn of includes I'd love to hear about it.
I spent a bunch of time tweaking the reference counts. I'm reasonably confident
this is correct but not 100%, an additional careful pass over the reference
count logic from a reviewer would be very appreciated.
I could have made `_PyArray_FromAny` and `_PyArray_CheckFromAny` take just a
`npy_dtype_info` struct, but I found it made the reference count logic more
complicated, since `PyArray_FromAny` and `PyArray_CheckFromAny` steal the
reference to the descriptor they are passed and I needed to conserve that
behavior. Also both functions support passing in a `NULL` pointer for the
descriptor and I needed to maintain that behavior as well.
The change to `ucsnarrow.h` fixes a preexisting conflict with the prototype
in `ucsnarrow.c` that triggered a compiler error while I was working on this.
Also make internal versions accept a dtype and a descriptor.
171dc85 to
5ae51e2
Compare
|
It occurs to me that I can back out all the include changes now that |
2e2c2c1 to
c45a101
Compare
There was a problem hiding this comment.
This looks good to me now, thanks. Planning to put it in in soon unless anyone minds.
I think the behavior is good, the only thing might be the how we implement it exactly, and I don't think we run too deep down a dead-end even if we decide another way is better.
|
Time to put it in, I think. Thanks Nathan :). As I said, if anyone dislikes the implementation, happy to follow up. |
This enables writing
np.array(some_object, dtype=type(np.dtype('i'))). This is a follow-on from #23154, see that PR for more details.I had to add a new include toctors.hto bring in the definition of thenpy_dtype_infostruct. Sincectors.his included in many other files inside numpy, I found that I needed to modify fewer includes across numpy if I moved the definition ofnpy_dtype_infotocommon.hfromdescriptor.h. The new includes ofcommon.hare needed to support later includes ofctors.hin those files. If anyone has an alternate place to putnpy_dtype_infothat would cause less churn of includes I'd love to hear about it.I spent a bunch of time tweaking the reference counts. I'm reasonably confident this is correct but not 100%, an additional careful pass over the reference count logic from a reviewer would be very appreciated.
I could have made_PyArray_FromAnyand_PyArray_CheckFromAnytake just anpy_dtype_infostruct, but I found it made the reference count logic more complicated, sincePyArray_FromAnyandPyArray_CheckFromAnysteal the reference to the descriptor they are passed and I needed to conserve that behavior. Also both functions support passing in aNULLpointer for the descriptor and I needed to maintain that behavior as well.The change to
ucsnarrow.hfixes a preexisting conflict with the prototype inucsnarrow.cthat triggered a compiler error while I was working on this.Update 3/20/2023: Edited out some out of date stuff from the PR description.