-
-
Notifications
You must be signed in to change notification settings - Fork 12.2k
ENH, API: New sorting mechanism for DType API #28516
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
Changes from all commits
87f410b
ecd3ee1
7e0bf44
45f5008
d235dc9
6a65c07
379022f
36fa05f
a7c6792
84e7421
5caea7f
f700725
e7cf5c2
21fb7e7
9244ea3
9f09b13
7aeba26
c7481b8
aa3415a
e725ed5
6d0ba21
23204c5
ede3462
889ee11
beba242
61e6f16
434005e
3c168e4
d9d9872
a448475
ab8f394
cd38b15
dcc465a
30b6c9c
60d7f80
38e29e0
bbbd274
178445d
29cd764
852783f
5aaa15f
28e8c3a
6e65f8d
73bb5f8
5eca653
9d3ae70
f46a498
b471562
d022a2d
b1222b2
4d7d592
b734527
7d9ac65
c21a164
ffdb683
fe91295
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 @@ | ||
| New comparison and null handling enums for sorting in dtype API | ||
| --------------------------------------------------------------- | ||
|
|
||
| Using the new `NPY_DT_sort_compare` slot, user-defined dtypes can | ||
| now specify how to compare elements during sorting operations. | ||
| The sort compare function should return a member of the | ||
| `NPY_COMPARE_RESULT` enum to indicate the result of the comparison, | ||
| including support for unordered comparisons. | ||
|
|
||
| The sorting context of type `PyArrayMethod_SortContext` is passed | ||
| to the `NPY_DT_get_sort_function` and `NPY_DT_get_argsort_function` | ||
| functions and contains a boolean `descending` flag and a | ||
| `nan_position` of type `NPY_SORT_NAN_POSITION`, which can be used to | ||
| control the behavior of sorting with respect to NaN values. | ||
| Currently, sorts are always ascending and nulls are always sorted last, | ||
| but this must be checked in the context passed to the sort function | ||
| to allow for future features. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| New sorting function slots `NPY_DT_get_sort_function`, `NPY_DT_get_argsort_function` for dtype API | ||
| --------------------------------------------------------------------------------------------------- | ||
|
|
||
| User-defined dtypes can now provide specific sorting functions for use with NumPy's sort methods. | ||
| The new slots `NPY_DT_get_sort_function` and `NPY_DT_get_argsort_function` should be functions that | ||
| return function pointers implementing sorting functionality for the dtype, while considering the | ||
| sort-kind and order. The old arrfunc slots ``NPY_DT_PyArray_ArrFuncs_sort`` and | ||
| ``NPY_DT_PyArray_ArrFuncs_argsort`` may be deprecated in the future. | ||
|
|
||
| Additionally, the new `NPY_DT_sort_compare` slot can be used to provide a comparison function for | ||
| sorting, which will replace the default comparison function for the dtype in sorting functions. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1873,6 +1873,36 @@ described below. | |
| pointer. Currently this is used for zero-filling and clearing arrays storing | ||
| embedded references. | ||
|
|
||
| .. c:type:: int (PyArray_SortFuncWithContext)( \ | ||
| PyArrayMethod_SortContext *context, void *data, \ | ||
| npy_intp num, NpyAuxData *auxdata) | ||
|
|
||
| A function to sort a buffer of data. The *data* is a pointer to the | ||
| beginning of the contiguous buffer containing *num* elements. A function | ||
| of this type is returned by the `get_sort_function` function in the DType | ||
| slots, where *context* is passed in containing the descriptor for the | ||
| array. Returns 0 on success, -1 on failure. | ||
|
|
||
| .. c:type:: int (PyArray_ArgSortFuncWithContext)( \ | ||
| PyArrayMethod_SortContext *context, void *data, \ | ||
| npy_intp *tosort, npy_intp num, NpyAuxData *auxdata) | ||
|
|
||
| A function to arg-sort a buffer of data. The *data* is a pointer to the | ||
| beginning of the buffer containing *num* elements. The *tosort* is a | ||
|
Member
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. @charris to confirm, even for argsorting it probably makes sense to always use a contiguous buffer for sorting?
Member
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. Maybe some, but we don't sort the data being argsorted, so it will eventually be accessed out of order in any case if the data is random. All this is to say, it depends :) |
||
| pointer to an array of indices that will be filled in with the | ||
| indices of the sorted elements. A function of this type is returned by | ||
| the `get_argsort_function` function in the DType slots, where | ||
| *context* is passed in containing the descriptor for the array. | ||
| Returns 0 on success, -1 on failure. | ||
|
|
||
| .. c:type:: NPY_COMPARE_RESULT (PyArray_SortCompareFunc) ( \ | ||
| const void *a, const void *b, PyArray_Descr *descr) | ||
|
|
||
| A function to compare two elements of an array for sorting. The *a* and *b* | ||
| pointers point to the elements to compare, and *descr* is the descriptor for | ||
| the array. Returns a value of type :c:type:`NPY_COMPARE_RESULT` indicating | ||
| the result of the comparison, including whether each element is unordered. | ||
|
|
||
| API Functions and Typedefs | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
|
|
@@ -3521,6 +3551,40 @@ member of ``PyArrayDTypeMeta_Spec`` struct. | |
| force newly created arrays to have a newly created descriptor | ||
| instance, no matter what input descriptor is provided by a user. | ||
|
|
||
| .. c:macro:: NPY_DT_get_sort_function | ||
|
|
||
| .. c:type:: int *(PyArrayDTypeMeta_GetSortFunction)(PyArray_Descr *, \ | ||
| npy_intp sort_kind, PyArray_SortFuncWithContext **out_sort, \ | ||
| NpyAuxData **out_auxdata, NPY_ARRAYMETHOD_FLAGS *out_flags) | ||
|
|
||
| .. versionadded:: 2.4 | ||
|
|
||
| If defined, sets a custom sorting function for the DType for each of | ||
| the sort kinds numpy implements. Currently, sorts are always descending | ||
| and always use nulls to the end, and this must be checked in the | ||
| implementation. Returns 0 on success. | ||
|
Member
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. Where are the sorting functions set? That is, does
Contributor
Author
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. The actual functions are set in the DType slots. As for |
||
|
|
||
| .. c:macro:: NPY_DT_get_argsort_function | ||
|
|
||
|
Member
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. Should macros be all caps?
Member
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. Looks like this actually a variable?
Contributor
Author
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. Hmm, they seem to be, but all the dtype slots seem to be marked as macros right now. |
||
| .. c:type:: int *(PyArrayDTypeMeta_GetArgSortFunction)(PyArray_Descr *, \ | ||
| npy_intp sort_kind, PyArray_ArgSortFuncWithContext **out_argsort, \ | ||
| NpyAuxData **out_auxdata, NPY_ARRAYMETHOD_FLAGS *out_flags) | ||
|
|
||
| .. versionadded:: 2.4 | ||
|
|
||
| If defined, sets a custom argsorting function for the DType for each of | ||
| the sort kinds numpy implements. Currently, sorts are always descending | ||
| and always use nulls to the end, and this must be checked in the | ||
| implementation. Returns 0 on success. | ||
|
|
||
| .. c:macro:: NPY_DT_sort_compare | ||
|
|
||
| .. versionadded:: 2.4 | ||
|
|
||
| If defined, sets a custom comparison function for the DType for use in | ||
| sorting, which will replace `NPY_DT_PyArray_ArrFuncs_compare`. Implements | ||
| ``PyArray_CompareFunc``. | ||
|
Member
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. Add a
Contributor
Author
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. Done, thanks |
||
|
|
||
| PyArray_ArrFuncs slots | ||
| ^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
|
|
@@ -3547,6 +3611,8 @@ DType API slots but for now we have exposed the legacy | |
| .. c:macro:: NPY_DT_PyArray_ArrFuncs_compare | ||
|
|
||
| Computes a comparison for `numpy.sort`, implements ``PyArray_CompareFunc``. | ||
| If `NPY_DT_sort_compare` is defined, it will be used instead. This slot may | ||
| be deprecated in the future. | ||
|
|
||
| .. c:macro:: NPY_DT_PyArray_ArrFuncs_argmax | ||
|
|
||
|
|
@@ -3590,13 +3656,17 @@ DType API slots but for now we have exposed the legacy | |
|
|
||
| An array of PyArray_SortFunc of length ``NPY_NSORTS``. If set, allows | ||
| defining custom sorting implementations for each of the sorting | ||
| algorithms numpy implements. | ||
| algorithms numpy implements. If `NPY_DT_get_sort_function` is | ||
| defined, it will be used instead. This slot may be deprecated in the | ||
| future. | ||
|
|
||
| .. c:macro:: NPY_DT_PyArray_ArrFuncs_argsort | ||
|
|
||
| An array of PyArray_ArgSortFunc of length ``NPY_NSORTS``. If set, | ||
| allows defining custom argsorting implementations for each of the | ||
| sorting algorithms numpy implements. | ||
| sorting algorithms numpy implements. If `NPY_DT_get_argsort_function` | ||
| is defined, it will be used instead. This slot may be deprecated in | ||
|
Member
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. Where does it need to be defined?
Contributor
Author
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. These are dtype slots, so they will be defined with the user dtype. Here is the example for StringDType. |
||
| the future. | ||
|
|
||
| Macros and Static Inline Functions | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
|
|
@@ -4340,6 +4410,40 @@ Enumerated Types | |
| :c:data:`NPY_STABLESORT` are aliased to each other and may refer to one | ||
| of several stable sorting algorithms depending on the data type. | ||
|
|
||
| .. c:enum:: NPY_SORT_NAN_POSITION | ||
|
|
||
| An enum used to indicate the position of NaN values in sorting. | ||
|
|
||
| .. c:enumerator:: NPY_SORT_NAN_TO_START | ||
|
|
||
| Indicates that NaN values should be sorted to the start. | ||
|
|
||
| .. c:enumerator:: NPY_SORT_NAN_TO_END | ||
|
|
||
| Indicates that NaN values should be sorted to the end. | ||
|
|
||
| .. c:enum:: NPY_COMPARE_RESULT | ||
|
|
||
|
Member
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. IIRC, C enums may be unsigned depending on whether they are explicitly defined. Are these actually enums, or just here for markup?
Contributor
Author
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. They are actually enums, defined in |
||
| An enum used to indicate the result of a comparison operation. | ||
| The unordered comparisons are used to indicate that the | ||
| comparison is not well-defined for one or both of the operands, | ||
| such as when comparing NaN values. | ||
|
|
||
| .. c:enumerator:: NPY_LESS | ||
|
|
||
| .. c:enumerator:: NPY_EQUAL | ||
|
|
||
| .. c:enumerator:: NPY_GREATER | ||
|
|
||
| .. c:enumerator:: NPY_UNORDERED_LEFT | ||
|
|
||
| .. c:enumerator:: NPY_UNORDERED_RIGHT | ||
|
|
||
| .. c:enumerator:: NPY_UNORDERED_BOTH | ||
|
|
||
| .. c:enumerator:: NPY_COMPARE_ERROR | ||
|
|
||
| Indicates that an error occurred during the comparison operation. | ||
|
Member
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. I wanted to ping you directly once @charris last week, I mentioned this one again. The generic
Member
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. Thinking about it.
Member
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. Hmm, what do you mean by "passing in"? Currently, the comparison function is inline code except for generic types. My thought is that there would be code for each case, with maybe an error if undefined. Do we intend to support topological sorts?
Contributor
Author
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. If I understand correctly, we can (as one example) pass in to Here is the implementation for StringDType right now. If we do this here we would directly return an integer that is currently returned by |
||
|
|
||
| .. c:enum:: NPY_SCALARKIND | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -792,6 +792,41 @@ PyArrayMethod_Context and PyArrayMethod_Spec | |
| An array of slots for the method. Slot IDs must be one of the values | ||
| below. | ||
|
|
||
| .. _arraymethod-sort-context: | ||
|
|
||
| PyArrayMethod_SortContext | ||
| ------------------------- | ||
|
|
||
| .. c:type:: PyArrayMethod_SortContext | ||
|
|
||
| A struct used to provide context for sorting methods. | ||
|
|
||
| .. code-block:: c | ||
|
|
||
| typedef struct { | ||
| PyArray_Descr *descriptor; | ||
| PyArray_SortCompareFunc *compare; | ||
| npy_bool descending; | ||
| NPY_SORT_NAN_POSITION nan_position; | ||
| } PyArrayMethod_SortContext | ||
|
|
||
| .. c:member:: PyArray_Descr *descriptor | ||
|
|
||
| The descriptor for the data type being sorted. | ||
|
|
||
| .. c:member:: PyArray_SortCompareFunc *compare | ||
|
|
||
| A pointer to the comparison function used for sorting. This function | ||
| can be NULL if the sort is not based on a comparison function. | ||
|
|
||
| .. c:member:: npy_bool descending | ||
|
|
||
| A flag indicating whether the sort is descending. | ||
|
|
||
| .. c:member:: NPY_SORT_NAN_POSITION nan_position | ||
|
|
||
| The position of NaN values in the sort order. | ||
|
Member
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. OK, makes sense to pass these as information, and I guess that means it makes sense to use a custom context and not try to rely on an existing one (the ufunc/casting one is more complex, and the traverse one too simple maybe.) Strictly speaking, we could avoid the sortcompare slot here, but I think it's maybe pragmatic to just include it and leave it NULL. |
||
|
|
||
| .. _dtypemeta: | ||
|
|
||
| PyArray_DTypeMeta and PyArrayDTypeMeta_Spec | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -368,6 +368,12 @@ typedef int (PyArrayMethod_PromoterFunction)(PyObject *ufunc, | |||||
| #define NPY_DT_get_fill_zero_loop 10 | ||||||
| #define NPY_DT_finalize_descr 11 | ||||||
|
|
||||||
| #if NPY_API_VERSION >= NPY_2_4_API_VERSION | ||||||
| #define NPY_DT_get_sort_function 12 | ||||||
| #define NPY_DT_get_argsort_function 13 | ||||||
| #define NPY_DT_sort_compare 14 | ||||||
| #endif | ||||||
|
|
||||||
| // These PyArray_ArrFunc slots will be deprecated and replaced eventually | ||||||
| // getitem and setitem can be defined as a performance optimization; | ||||||
| // by default the user dtypes call `legacy_getitem_using_DType` and | ||||||
|
|
@@ -477,4 +483,45 @@ typedef PyArray_Descr *(PyArrayDTypeMeta_FinalizeDescriptor)(PyArray_Descr *dtyp | |||||
| typedef int(PyArrayDTypeMeta_SetItem)(PyArray_Descr *, PyObject *, char *); | ||||||
| typedef PyObject *(PyArrayDTypeMeta_GetItem)(PyArray_Descr *, char *); | ||||||
|
|
||||||
| typedef enum { | ||||||
| NPY_LESS = -1, | ||||||
| NPY_EQUAL = 0, | ||||||
| NPY_GREATER = 1, | ||||||
| NPY_UNORDERED_LEFT = 2, | ||||||
| NPY_UNORDERED_RIGHT = 3, | ||||||
| NPY_UNORDERED_BOTH = 4, | ||||||
| NPY_COMPARE_ERROR = 5, | ||||||
| } NPY_COMPARE_RESULT; | ||||||
|
|
||||||
|
Member
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. Does the comparison function need to handle all those values? What if it doesn't care? The current sorts only require NPY_LESS, and only use the (-1, 0, 1) sequence where that is conventional for code that comes from outside of Numpy.
Contributor
Author
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. Yeah, perhaps it doesn't, though the main issue comes up in nan sorting (as in dataframes) as was mentioned by @seberg. If we don't include this, NaNs would be treated consistently in both ascending and descending sorts, causing their position to be flipped, if that is a problem.
Member
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. I could see making sure that -1, 0, 1 retain their meaning for simplicity. You could also just not bother here and say if you want it you need to use the more complicated API...
Contributor
Author
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. Would this mean certain user dtypes (that choose not to bother) would not support NaN order sensitive sorting? Should we allow to indicate that somehow?
Member
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. Yeah, so dunno if it's good. Maybe the best thing is to pass in the information, then the choice to ignore it is rather obvious :). Another thing is, @charris do you feel we should prepare for EDIT: I suppose maybe passing it in but returning -1, 0, 1 is the best? |
||||||
| typedef struct PyArrayMethod_SortContext_tag PyArrayMethod_SortContext; | ||||||
|
|
||||||
| typedef NPY_COMPARE_RESULT (PyArray_SortCompareFunc)( | ||||||
| const void *a, const void *b, PyArray_Descr *descr); | ||||||
|
|
||||||
| typedef enum { | ||||||
| NPY_SORT_NAN_TO_START = 0, | ||||||
| NPY_SORT_NAN_TO_END = 1, | ||||||
| } NPY_SORT_NAN_POSITION; | ||||||
|
|
||||||
| struct PyArrayMethod_SortContext_tag { | ||||||
| PyArray_Descr *descriptor; | ||||||
| PyArray_SortCompareFunc *compare; | ||||||
| npy_bool descending; | ||||||
| NPY_SORT_NAN_POSITION nan_position; | ||||||
| }; | ||||||
|
|
||||||
| typedef int (PyArray_SortFuncWithContext)(PyArrayMethod_SortContext *, | ||||||
| void *, npy_intp, | ||||||
| NpyAuxData *); | ||||||
|
Member
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. What is in NpyAuxData?
Contributor
Author
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. Any additional information that the sorting may need to access. I suppose it's for future compatibility, if say, we need to add dtypes that use auxiliary data in sorting or even sort multiple related arrays in some way, but @seberg would probably have a better idea of the use cases he thought of.
Member
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. Auxdata is what all casts (and ufuncs) are passed. It allows holding on to temporary data. E.g. casts need it for structured dtypes (and this could make sense here in the same way). |
||||||
| typedef int (PyArray_ArgSortFuncWithContext)(PyArrayMethod_SortContext *, | ||||||
| void *, npy_intp *, npy_intp, | ||||||
| NpyAuxData *); | ||||||
|
Member
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. These two need different names and you need to leave the original typedefs in
Contributor
Author
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. Thanks for reviewing! This is done.
Member
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. Is it intended that a single sort function should handle all variations of the context, or is this basically a dispatch function?
Member
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. If dispatch, I would put that in the name: |
||||||
|
|
||||||
| typedef int *(PyArrayDTypeMeta_GetSortFunction)(PyArray_Descr *, | ||||||
| npy_intp, PyArray_SortFuncWithContext **, NpyAuxData **, | ||||||
| NPY_ARRAYMETHOD_FLAGS *); | ||||||
| typedef int *(PyArrayDTypeMeta_GetArgSortFunction)(PyArray_Descr *, | ||||||
| npy_intp, PyArray_ArgSortFuncWithContext **, NpyAuxData **, | ||||||
| NPY_ARRAYMETHOD_FLAGS *); | ||||||
|
|
||||||
|
Member
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. New stuff in the public API needs new API docs as well as a release note describing the new features. Maybe also as a proof-of-concept, it looks like both quaddtype and mpfdtype in numpy-user-dtypes implement sorting - would you be willing to update them to use the new API in a PR to numpy-user-dtypes that depends on this PR to numpy? That should give you a feeling for whether this API is helpful for someone writing a new user dtype. It'll also be a form of documentation - we don't have great docs for writing user dtypes besides the examples in numpy-user-dtypes.
Member
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. Also what should we do about the flags that got added before we made the dtype API public, e.g.
Member
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. It's easy to generate a deprecation warning during registration (a bit tedious maybe, as you need explicit check).
Contributor
Author
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. Sure, I'll add API docs and a release note, and willing to make a PR to numpy-user-dtypes! Will look into that. Just to be clear,
Member
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. We can't change slot numbers (unless they are guarded as private)! So the numbers are fixed (until they have not been used for a bit at least). So, we just have to live with the numbering we got, I half thought I asked for an offset for the [^depr] I think this is as simple as asking users to compile with the new NumPy, and then adding
Member
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 is an offset, numpy/numpy/_core/src/multiarray/dtypemeta.h Lines 94 to 95 in 9389862
|
||||||
| #endif /* NUMPY_CORE_INCLUDE_NUMPY___DTYPE_API_H_ */ | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -194,6 +194,9 @@ dtypemeta_initialize_struct_from_spec( | |
| NPY_DT_SLOTS(DType)->getitem = NULL; | ||
| NPY_DT_SLOTS(DType)->get_clear_loop = NULL; | ||
| NPY_DT_SLOTS(DType)->get_fill_zero_loop = NULL; | ||
| NPY_DT_SLOTS(DType)->get_sort_function = NULL; | ||
| NPY_DT_SLOTS(DType)->get_argsort_function = NULL; | ||
| NPY_DT_SLOTS(DType)->sort_compare = NULL; | ||
|
Member
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. must come at the end to not break ABI!
Contributor
Author
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. Yes, this is done! |
||
| NPY_DT_SLOTS(DType)->finalize_descr = NULL; | ||
| NPY_DT_SLOTS(DType)->f = default_funcs; | ||
|
|
||
|
|
@@ -1230,6 +1233,15 @@ dtypemeta_wrap_legacy_descriptor( | |
| dtype_class->flags |= NPY_DT_NUMERIC; | ||
| } | ||
|
|
||
| if (dt_slots->sort_compare == NULL) { | ||
ngoldbaum marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (!NPY_DT_is_legacy(dtype_class) && !NPY_DT_is_user_defined(dtype_class)) { | ||
| PyErr_SetString(PyExc_RuntimeError, | ||
| "DType has no sort_compare function."); | ||
ngoldbaum marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Py_DECREF(dtype_class); | ||
| return -1; | ||
| } | ||
| } | ||
|
|
||
| if (_PyArray_MapPyTypeToDType(dtype_class, descr->typeobj, | ||
| PyTypeNum_ISUSERDEF(dtype_class->type_num)) < 0) { | ||
| Py_DECREF(dtype_class); | ||
|
|
||
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 a note that the old arrfuncs slots may be deprecated in the future.
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.
Added, thanks!