From bed8d23ad4c254f5636a46985a3e500cc12df86a Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Wed, 22 Feb 2023 12:08:33 -0700 Subject: [PATCH 1/6] Add clear loop implementation --- stringdtype/meson.build | 1 - stringdtype/pyproject.toml | 6 ++++++ stringdtype/stringdtype/src/casts.c | 3 ++- stringdtype/stringdtype/src/dtype.c | 33 +++++++++++++++++++++++++++++ stringdtype/stringdtype/src/main.c | 8 +++---- 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/stringdtype/meson.build b/stringdtype/meson.build index d63589de..64b6a114 100644 --- a/stringdtype/meson.build +++ b/stringdtype/meson.build @@ -43,7 +43,6 @@ py.install_sources( py.extension_module( '_main', srcs, - c_args: ['-g', '-O0', '-pg'], install: true, subdir: 'stringdtype', include_directories: includes diff --git a/stringdtype/pyproject.toml b/stringdtype/pyproject.toml index 73a160b8..3ad86297 100644 --- a/stringdtype/pyproject.toml +++ b/stringdtype/pyproject.toml @@ -28,3 +28,9 @@ dependencies = [ [tool.ruff] line-length = 79 per-file-ignores = {"__init__.py" = ["F401"]} + +[tool.meson-python.args] +dist = [] +setup = ["-Ddebug=true", "-Doptimization=0"] +compile = [] +install = [] diff --git a/stringdtype/stringdtype/src/casts.c b/stringdtype/stringdtype/src/casts.c index ea75e4fa..8ad331ce 100644 --- a/stringdtype/stringdtype/src/casts.c +++ b/stringdtype/stringdtype/src/casts.c @@ -220,9 +220,10 @@ unicode_to_string(PyArrayMethod_Context *context, char *const data[], ss *out_ss = ssnewempty(out_num_bytes); if (out_ss == NULL) { gil_error(PyExc_MemoryError, "ssnewempty failed"); + return -1; } char *out_buf = out_ss->buf; - for (int i = 0; i < num_codepoints; i++) { + for (size_t i = 0; i < num_codepoints; i++) { // get code point Py_UCS4 code = in[i]; diff --git a/stringdtype/stringdtype/src/dtype.c b/stringdtype/stringdtype/src/dtype.c index 1a8b492a..800d7ecf 100644 --- a/stringdtype/stringdtype/src/dtype.c +++ b/stringdtype/stringdtype/src/dtype.c @@ -20,6 +20,8 @@ new_stringdtype_instance(void) new->base.alignment = _Alignof(ss *); new->base.flags |= NPY_NEEDS_INIT; new->base.flags |= NPY_LIST_PICKLE; + new->base.flags |= NPY_ITEM_REFCOUNT; + new->base.flags |= NPY_ITEM_IS_POINTER; return new; } @@ -172,6 +174,36 @@ stringdtype_ensure_canonical(StringDTypeObject *self) return self; } +static int +stringdtype_clear_loop(void *NPY_UNUSED(traverse_context), + PyArray_Descr *NPY_UNUSED(descr), char *data, + npy_intp size, npy_intp stride, + NpyAuxData *NPY_UNUSED(auxdata)) +{ + while (size--) { + if (data != NULL) { + free(*(ss **)data); + } + data += stride; + } + + return 0; +} + +static int +stringdtype_get_clear_loop(void *NPY_UNUSED(traverse_context), + PyArray_Descr *NPY_UNUSED(descr), + int NPY_UNUSED(aligned), + npy_intp NPY_UNUSED(fixed_stride), + traverse_loop_function **out_loop, + NpyAuxData **NPY_UNUSED(out_auxdata), + NPY_ARRAYMETHOD_FLAGS *flags) +{ + *flags = NPY_METH_REQUIRES_PYAPI | NPY_METH_NO_FLOATINGPOINT_ERRORS; + *out_loop = &stringdtype_clear_loop; + return 0; +} + static PyType_Slot StringDType_Slots[] = { {NPY_DT_common_instance, &common_instance}, {NPY_DT_common_dtype, &common_dtype}, @@ -181,6 +213,7 @@ static PyType_Slot StringDType_Slots[] = { {NPY_DT_getitem, &stringdtype_getitem}, {NPY_DT_ensure_canonical, &stringdtype_ensure_canonical}, {NPY_DT_PyArray_ArrFuncs_compare, &compare_strings}, + {NPY_DT_get_clear_loop, &stringdtype_get_clear_loop}, {0, NULL}}; static PyObject * diff --git a/stringdtype/stringdtype/src/main.c b/stringdtype/stringdtype/src/main.c index 27666c35..24648a08 100644 --- a/stringdtype/stringdtype/src/main.c +++ b/stringdtype/stringdtype/src/main.c @@ -29,9 +29,9 @@ _memory_usage(PyObject *NPY_UNUSED(self), PyObject *obj) return NULL; } - NpyIter *iter = - NpyIter_New(arr, NPY_ITER_READONLY | NPY_ITER_EXTERNAL_LOOP, - NPY_KEEPORDER, NPY_NO_CASTING, NULL); + NpyIter *iter = NpyIter_New( + arr, NPY_ITER_READONLY | NPY_ITER_EXTERNAL_LOOP | NPY_ITER_REFS_OK, + NPY_KEEPORDER, NPY_NO_CASTING, NULL); if (iter == NULL) { return NULL; @@ -90,7 +90,7 @@ PyInit__main(void) if (_import_array() < 0) { return NULL; } - if (import_experimental_dtype_api(8) < 0) { + if (import_experimental_dtype_api(9) < 0) { return NULL; } From b68746e052a3ff8fbd9bb8b7ea74e4222468103f Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 23 Feb 2023 11:40:17 -0700 Subject: [PATCH 2/6] respond to code review comments --- stringdtype/stringdtype/src/dtype.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/stringdtype/stringdtype/src/dtype.c b/stringdtype/stringdtype/src/dtype.c index 800d7ecf..6576e0e6 100644 --- a/stringdtype/stringdtype/src/dtype.c +++ b/stringdtype/stringdtype/src/dtype.c @@ -183,6 +183,7 @@ stringdtype_clear_loop(void *NPY_UNUSED(traverse_context), while (size--) { if (data != NULL) { free(*(ss **)data); + *(ss **)data = NULL; } data += stride; } @@ -199,7 +200,7 @@ stringdtype_get_clear_loop(void *NPY_UNUSED(traverse_context), NpyAuxData **NPY_UNUSED(out_auxdata), NPY_ARRAYMETHOD_FLAGS *flags) { - *flags = NPY_METH_REQUIRES_PYAPI | NPY_METH_NO_FLOATINGPOINT_ERRORS; + *flags = NPY_METH_NO_FLOATINGPOINT_ERRORS; *out_loop = &stringdtype_clear_loop; return 0; } From e9d60ac13d14c78329cfdd2e51788273d372af1a Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Thu, 23 Feb 2023 11:44:32 -0700 Subject: [PATCH 3/6] remove NPY_ITEM_IS_POINTER flag --- stringdtype/stringdtype/src/dtype.c | 1 - 1 file changed, 1 deletion(-) diff --git a/stringdtype/stringdtype/src/dtype.c b/stringdtype/stringdtype/src/dtype.c index 6576e0e6..ff7aafc1 100644 --- a/stringdtype/stringdtype/src/dtype.c +++ b/stringdtype/stringdtype/src/dtype.c @@ -21,7 +21,6 @@ new_stringdtype_instance(void) new->base.flags |= NPY_NEEDS_INIT; new->base.flags |= NPY_LIST_PICKLE; new->base.flags |= NPY_ITEM_REFCOUNT; - new->base.flags |= NPY_ITEM_IS_POINTER; return new; } From 67f3bbbc073eca88c48ce3a1659325d1f795b0a4 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 27 Feb 2023 09:28:48 -0700 Subject: [PATCH 4/6] update experimental API version in all dtypes --- asciidtype/asciidtype/src/asciidtype_main.c | 2 +- metadatadtype/metadatadtype/src/metadatadtype_main.c | 2 +- mpfdtype/mpfdtype/src/mpfdtype_main.c | 2 +- quaddtype/quaddtype/src/quaddtype_main.c | 2 +- unytdtype/unytdtype/src/unytdtype_main.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/asciidtype/asciidtype/src/asciidtype_main.c b/asciidtype/asciidtype/src/asciidtype_main.c index 4bf027ad..1574eb91 100644 --- a/asciidtype/asciidtype/src/asciidtype_main.c +++ b/asciidtype/asciidtype/src/asciidtype_main.c @@ -22,7 +22,7 @@ PyInit__asciidtype_main(void) return NULL; } - if (import_experimental_dtype_api(8) < 0) { + if (import_experimental_dtype_api(9) < 0) { return NULL; } diff --git a/metadatadtype/metadatadtype/src/metadatadtype_main.c b/metadatadtype/metadatadtype/src/metadatadtype_main.c index bdb21c66..c564398e 100644 --- a/metadatadtype/metadatadtype/src/metadatadtype_main.c +++ b/metadatadtype/metadatadtype/src/metadatadtype_main.c @@ -21,7 +21,7 @@ PyInit__metadatadtype_main(void) if (_import_array() < 0) { return NULL; } - if (import_experimental_dtype_api(8) < 0) { + if (import_experimental_dtype_api(9) < 0) { return NULL; } diff --git a/mpfdtype/mpfdtype/src/mpfdtype_main.c b/mpfdtype/mpfdtype/src/mpfdtype_main.c index 44ade5da..0f68b586 100644 --- a/mpfdtype/mpfdtype/src/mpfdtype_main.c +++ b/mpfdtype/mpfdtype/src/mpfdtype_main.c @@ -22,7 +22,7 @@ PyInit__mpfdtype_main(void) if (_import_array() < 0) { return NULL; } - if (import_experimental_dtype_api(8) < 0) { + if (import_experimental_dtype_api(9) < 0) { return NULL; } diff --git a/quaddtype/quaddtype/src/quaddtype_main.c b/quaddtype/quaddtype/src/quaddtype_main.c index 855ef9ab..5f13c079 100644 --- a/quaddtype/quaddtype/src/quaddtype_main.c +++ b/quaddtype/quaddtype/src/quaddtype_main.c @@ -23,7 +23,7 @@ PyInit__quaddtype_main(void) return NULL; // Fail to init if the experimental DType API version 5 isn't supported - if (import_experimental_dtype_api(8) < 0) { + if (import_experimental_dtype_api(9) < 0) { PyErr_SetString(PyExc_ImportError, "Error encountered importing the experimental dtype API."); return NULL; diff --git a/unytdtype/unytdtype/src/unytdtype_main.c b/unytdtype/unytdtype/src/unytdtype_main.c index 81a0e0ce..26e116ba 100644 --- a/unytdtype/unytdtype/src/unytdtype_main.c +++ b/unytdtype/unytdtype/src/unytdtype_main.c @@ -21,7 +21,7 @@ PyInit__unytdtype_main(void) if (_import_array() < 0) { return NULL; } - if (import_experimental_dtype_api(8) < 0) { + if (import_experimental_dtype_api(9) < 0) { return NULL; } From ba174d4f11eae545a9ed1029f9d9fd18e47cb70f Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 27 Feb 2023 10:01:31 -0700 Subject: [PATCH 5/6] add NPY_NEEDS_PYAPI flag to fix seg fault in sort test --- stringdtype/stringdtype/src/dtype.c | 1 + 1 file changed, 1 insertion(+) diff --git a/stringdtype/stringdtype/src/dtype.c b/stringdtype/stringdtype/src/dtype.c index ff7aafc1..3a39d8f0 100644 --- a/stringdtype/stringdtype/src/dtype.c +++ b/stringdtype/stringdtype/src/dtype.c @@ -21,6 +21,7 @@ new_stringdtype_instance(void) new->base.flags |= NPY_NEEDS_INIT; new->base.flags |= NPY_LIST_PICKLE; new->base.flags |= NPY_ITEM_REFCOUNT; + new->base.flags |= NPY_NEEDS_PYAPI; return new; } From 0eee0133dfeac8f975c311799767cfa3a8481375 Mon Sep 17 00:00:00 2001 From: Nathan Goldbaum Date: Mon, 27 Feb 2023 11:54:44 -0700 Subject: [PATCH 6/6] Remove the NPY_NEEDS_PYAPI flag. Fix this in numpy instead, see https://github.com/numpy/numpy/pull/23292 --- stringdtype/stringdtype/src/dtype.c | 1 - 1 file changed, 1 deletion(-) diff --git a/stringdtype/stringdtype/src/dtype.c b/stringdtype/stringdtype/src/dtype.c index 3a39d8f0..ff7aafc1 100644 --- a/stringdtype/stringdtype/src/dtype.c +++ b/stringdtype/stringdtype/src/dtype.c @@ -21,7 +21,6 @@ new_stringdtype_instance(void) new->base.flags |= NPY_NEEDS_INIT; new->base.flags |= NPY_LIST_PICKLE; new->base.flags |= NPY_ITEM_REFCOUNT; - new->base.flags |= NPY_NEEDS_PYAPI; return new; }