ENH: implement descending sorts for StringDType - #32286
Conversation
There was a problem hiding this comment.
Thanks @ngoldbaum, looks good! I was half-hoping we could keep reusing the default loops and maybe expand the static_data field to contain two PyArray_CompareFuncs, but I'm not entirely sure how that could be designed (though, an array of two seems reasonable honestly). If you think that's too out of scope or better for follow-up, completely happy to put this in as-is, just let me know and I'll merge!
|
@MaanasArora thanks for the suggestion! I think this is simpler, let me know what you think. |
There was a problem hiding this comment.
it turns out if you add a second release note with the same content as the first, towncrier generates a single entry that references both PRs.
| switch (sort_params->flags) { | ||
| if (cmp == NULL) { | ||
| npy_gil_error(PyExc_ValueError, | ||
| "descending sort not supported for this DType"); |
There was a problem hiding this comment.
Can this not be ascending too?
|
Thanks @ngoldbaum, yes this looks great! Just an error handling change inline. |
|
Thanks, LGTM now - will merge when CI passes! |
PR summary
This implements support for descending sorts, following #31431 and #31345
Pretty straightforward! Mostly this is wiring things up.
Ping @MaanasArora I'd appreciate your opinion here.
AI Disclosure
I used an AI to help with the test and review my code.