Repository navigation
MAINT: Refactor dtype conversion functions to be more similar - #15300
Conversation
d11910b to
4ae10a4
Compare
4ae10a4 to
eb84a9b
Compare
Before there were three signatures: * `PyArray_Descr*(...)` * `PyArray_Descr*(..., int* err_flag)` * `int(..., PyArray_Descr** ret)` This unifies them all into `PyArray_Descr*(...)`, and introduces a special singeton to distinguish "error" from "gave up" cases. Also changes function names for consistency.
eb84a9b to
2877369
Compare
|
Is this an intermediate step or do you expect that this "address" will stick around? Mainly curious, in other places I think we use |
Yes
I thought about this, but it would require some annoying I can update this patch to use that solution if you'd prefer. |
|
Yeah, I realize that is not ideal either. So dunno, I was mainly toying with the thought of making it a small function similar to |
|
Updated with a fixup commit to use |
|
Brief glance... Yeah looks reasonable, and probably a more expected pattern at least. I will look over it (and I assume merge it) tonight, probably. |
|
And one more fixup to make the |
| Py_DECREF(conv); | ||
| } | ||
| Py_DECREF(conv); |
There was a problem hiding this comment.
This is probably quite a good argument for why I'm INCREFing Py_NotImplemented before returning it.
There was a problem hiding this comment.
👍 Not incrementing is just too smart anyway...
|
CI now passing, silly typo. If this looks ready, probably best to squash-merge |
|
Thanks, I let me put this in. Definitely an improvement. I like the Not that I expect it, but when you are done with this, I will try to remember running a ref-count checker over the tests again. Better than being too paranoid about too. |
|
oooops, I forgot to squash merge, damn :(. Oh well. |
Before there were three signatures:
PyArray_Descr*(...)PyArray_Descr*(..., int* err_flag)int(..., PyArray_Descr** ret)This unifies them all into
PyArray_Descr*(...), and introduces a special singeton to distinguish "error" from "gave up" cases.Also changes function names for consistency.
The goal here is to make it very easy to spot returns that forgot to set an error.