seberg commented on code in PR #48391:
URL: https://github.com/apache/arrow/pull/48391#discussion_r2601514605
##########
python/pyarrow/src/arrow/python/numpy_to_arrow.cc:
##########
@@ -74,6 +81,37 @@ using internal::NumPyTypeSize;
namespace {
+#if NPY_ABI_VERSION >= 0x02000000
+
+// NumPy exposes StringDType helpers in the C-API table from version 2.0
onward,
+// but the corresponding macros are only available when compiling against a
+// 2.0+ feature level. Arrow still targets an older feature level, so provide
+// local wrappers that call the C-API entries directly.
+
+inline npy_string_allocator* ArrowNpyString_acquire_allocator(
+ const PyArray_StringDTypeObject* descr) {
+ using Func = npy_string_allocator* (*)(const PyArray_StringDTypeObject*);
+ auto func = reinterpret_cast<Func>(PyArray_API[316]);
Review Comment:
The idea is unfortunately right, I think, since you can't put this in a
different compilation unit with `NPY_TARGET_VERSION=NPY_2_0_API_VERSION` and
you can't switch `NPY_TARGET_VERSION` within the same compilation unit.
Maybe for these specific functions that was actually unnecessary (and if
you/@ngoldbaum likes, we could make them _always_ defined in 2.5, it would just
segfault if you ever use it, but we/I missed that it is impossible for there to
be something to use it on).
That way, it might only be available if build with NumPy 2.5, but that is
probably OK in practice, since that is what official builds will use (except
maybe for some old Python version).
That said `NPY_ABI_VERSION >= 0x02000000` isn't good here. If anything it
should be `NPY_FEATURE_VERSION` (which you can enforce via `NPY_TARGET_VERSION`
to indicate a minimal version of NumPy you support at runtime).
While think it's correct to hack it in this vain, I would suggest to guard
it in a way that uses the custom version only when necessary and makes it
obvious how to clean it up in the future.
(Even just copy-paste the C definitions with a `#ifndef ...`!?)
If NumPy offers the required defines (which it will at least on newer
versions or with `NPY_TARGET_VERSION=NPY_2_0_API_VERSION` it is better to stop
using it, there is no guarantee it will remain correct for ever.
##########
python/pyarrow/src/arrow/python/numpy_to_arrow.cc:
##########
@@ -59,6 +60,12 @@
#include "arrow/python/type_traits.h"
#include "arrow/python/vendored/pythoncapi_compat.h"
+#if NPY_ABI_VERSION >= 0x02000000
Review Comment:
The question whether is whether arrow cares to support
`--no-build-isolation` builds in an env that has NumPy 1.x installed.
Quite a few projects do, even though any other (including official) build
uses 2.x anyway (unless you are building for ancient Python versions maybe).
On such builds, `NPY_VSTRING` isn't defined so that is probably the only
place where this (or a similar pattern) is necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
