iframe-proxy

riku-sakamoto · 2025-12-29T05:17:27Z

Changes

Implement __array_interface__ for NumPy scalar types to avoid creating a temporary array.
Set the read-only flag in the __array_interface__ dict for NumPy scalars.

As a consequence of this change, numpy.ctypeslib.as_ctypes now raises TypeError when NumPy scalar types are passed directly.

Closes #30354

Scalar types expose `__array_interface__` by creating a temporary array. To ensure the address remains valid, the temporary array must be kept alive. (See numpy#30354)

ngoldbaum · 2025-12-29T18:15:42Z

-        PyDict_SetItemString(inter, "__ref", (PyObject *)arr);
+    if (inter == NULL) {
+        Py_DECREF(arr);
+        Py_DECREF(inter);


Aren't you decrefing a pointer that has to be NULL here?

this also indicates this error path isn't tested - might be worth adding a test!

Aren't you decrefing a pointer that has to be NULL here?

Thank you for catching this. you're absolutely right.
I fixed the incorrect decref.

this also indicates this error path isn't tested - might be worth adding a test!

This if branch seems to be unreachable because the array object always provides __array_interface__.
Instead of adding a direct test for this path, I improved the test coverage by:

increasing the number of scalar types under test

adding checks for unconversible types

ngoldbaum

There's some code that is doing questionable things and it only happens to work because scalars are read-only. See comments inline.

ngoldbaum · 2025-12-31T15:18:04Z

+    int _flags = flags;
+    _flags &= ~(NPY_ARRAY_WRITEBACKIFCOPY | NPY_ARRAY_OWNDATA);
+    _flags |= NPY_ARRAY_NOTSWAPPED;


This is all a no-op because _flags isn't used below. See my other comment below for speculation about why you defined this variable.

ngoldbaum · 2025-12-31T15:32:01Z

+
+    return Py_BuildValue(
+            "NO", PyLong_FromVoidPtr(scalar_value(self, NULL)),
+            ((flags & PyBUF_WRITEABLE) == PyBUF_WRITEABLE) ? Py_False : Py_True);


Why are you checking PyBUF_WRITEABLE here - that isn't an ndarray flag. PyBUF_WRITEABLE is part of the CPython buffer request API and will only show up in calls directly from CPython for handling the buffer protocol, which has nothing to do with this code as far as I can see.

Looking at the implementation of array_dataptr_get I think you meant to use _flags here and not do the PyBUF_WRITEABLE check at all.

Also I don't think this check for read-only matters at all for scalars because scalars are always read-only:

>>> a = np.int32(5) >>> a[:] = 7 Traceback (most recent call last): File "<python-input-3>", line 1, in <module> a[:] = 7 ~^^^ TypeError: 'numpy.int32' object does not support item assignment >>> a.data <memory at 0x200012a6e50> >>> a.data[:] = 7 Traceback (most recent call last): File "<python-input-5>", line 1, in <module> a.data[:] = 7 ~~~~~~^^^ TypeError: cannot modify read-only memory

Maybe you can avoid all this messing with flags and just set statically that this is read-only.

Also why not set the other items in the __array_interface__ dict like shape etc? Seems like there's no reason not to and it should be trivial to do that for scalars. I'd follow the ndarray implementation in array_interface_get.

Thank you for taking time to review my PR.

I’d like to briefly explain my original reasoning, and then align with your suggestions.

Why I do not set read-only flag for scalar array?

As you point out, scalars are always read-only.
However, according to the code below in ctypeslib, if the array is read-only, as_ctypes raise an error. So, I decided to follow the way flags are created in __array_struct__.

numpy/numpy/ctypeslib/_ctypeslib.py

Lines 594 to 595 in 3556042

if readonly:

raise TypeError("readonly arrays unsupported")

Maybe, is it better to set flag as writable ? (Or forbid as_ctypes for scalars altogether?)

use of PyBUF_WRITEABLE flag

This was a clear mistake on my part. I confused ndarray's flags with buffer protocol's flags. As you pointed out, PyBUF_WRITEABLE is not appropriate here, and I will remove this entirely.

_flags variable

You are also right here. I intended to use _flags to check writability, but the current code does not do that correctly.
I am sorry for the confusion.

why not set the other items in the array_interface dict like shape etc? Seems like there's no reason not to and it should be trivial to do that for scalars. I'd follow the ndarray implementation in array_interface_get.

Sure! I will implement it following to array_interface_get.

I've updated the PR accordingly except for the what kind of flags to set for scalar arrays.

seberg · 2026-01-05T11:00:42Z

+
+    /* NOTE: sclar array is always readonly, but we need writable flags
+     * to use `ctypeslib.as_ctyps` via `__array_interface__`.
+     * So, we set the flag borrowing from the temporary array. 


This doesn't really work out, I think. The content of the scalar is clearly readonly (except for void ones).
It seems that __array_interface__ right now doesn't consider it to be readonly though, which is technically true, because it refers to __ref which is undocumented.

We have two options here:

Document that __ref thing

Change the flag to return readonly here (maybe always, also for void).

I would be tempted to do the second one but that might backfire in practice and we need to ask a few others.

In the second case, ctypes needs to work around whatever we do here. That may be just ignoring the readonly part or creating a copy explicitly.

Actually, it may make sense to simply error in ctypeslib because ctypeslib was clearly always very broken with a reasonably high chance of generating bad results without an error.

Thanks for the comment!

I agree with the second option. Under this option, we would not change anything in ctypeslib.as_ctypes.
As a result, scalar arrays would no longer be usable with ctypeslib.as_ctypes, but this behavior seems more robust.

we need to ask a few others.

Is there anyone I should ping about this, or would it be appropriate to bring this topic to the next triage meeting?

@mtsokol do you have any context about where your initial report about this came from?

It appeared in our custom compiler:
https://github.com/finch-tensor/finch-tensor-lite/pull/248/files#diff-70a950803da49a25141ebb1d3bd6a6e33d417e2c3e0c42b0a34ed662bd690b2cL211 - within code meant for serializing NumPy scalars to C. We ended up using 0-d arrays here so we won't be impacted if as_ctypes stops supporting scalars.

Yeah, just to be clear "asking around", I was just thinking of brining it up at the triage meeting and if there is a general consensus to try, try it (but not backport!).

I doubt we'll easily find someone who might actually be affected. The problem is that the main use-case may be something like a Cython function that uses a typed memory view (like double data[::1]) that could use a const (signalling read-only is OK), but doesn't...

And we should maybe think whether such Cython use-cases would hit this (I am not sure), because if they do, than that might be an unfortunately large change.

🤦 this is __array_interface__ so doesn't matter for Cython of course, i.e. the chance of someone actually caring/noticing is a lot smaller.

Thanks everyone for the discussion.
It seems reasonable to try this out, but just in case, I’ve added it as a topic for the next triage meeting.

I've updated the flags in __array_interface__ for scalars, following the discussion in the last triage meeting.
I've also updated the tests accordingly.

I checked repositories under the numba organization to see whether ctypeslib.as_ctypes is used and did not find any usage.
For reference:
https://github.com/search?q=org%3Anumba+%22as_ctypes%28%22&type=code

riku-sakamoto · 2026-01-22T23:58:16Z

@ngoldbaum @seberg

Just a gentle ping.
I’ve updated the code based on previous triage mtg.
Please let me know if you have any remaining concerns!
I’m happy to address them.

ngoldbaum · 2026-01-23T15:36:52Z

I think this needs a release note.

Can you also update the PR description to match what it actually does? The PR description will show up in the final merge commit.

ngoldbaum

The C API uses and reference counting all look correct to me.

riku-sakamoto · 2026-01-25T08:20:01Z

@ngoldbaum
Thank you for taking another look.
I've updated the PR description to clarify the behavior and added a release note.

seberg

LGTM, one nit is that we should also note the change in __array_interface__ maybe?

I would prefer if we could refactor this so that we have a single function that build the array interface but gets strides, shape, ptr and descr passed in.
(Right now we have twice practically the identical code.)

But considering how long this has been sitting, I am also happy if we don't do that.

seberg · 2026-01-26T13:15:07Z

+This change was made to avoid the issue `gh-30354 <https://github.com/numpy/numpy/issues/30354>`__
+and to enforce the readonly nature of scalar types in NumPy.
+Users who need to convert scalar types to ctypes should first convert them to an array
+(e.g., ``numpy.asarray``) before passing them to ``numpy.ctypeslib.as_ctypes``.


Might be worth pointing out that this was always shady? :).

Can you add a second note about the change in __array_interface__? (or add both in one.)

seberg · 2026-01-26T13:19:58Z

+    }
+    PyList_SET_ITEM(res, 0, dobj);
+    return res;
+}


It would be cool to refactor these so that we don't repeat them in both files (i.e. by making them take the PyArray_Descr *).

But I can live without doing that at this point :).

Thanks for pointing this out.

I've refactored the duplicated logic into a common helper function in descriptor.c and updated the relevant call to use it.

I left some functions, such as gentype_shape_get, unchanged since they are also used in other parts of scalartypes.c.src.
If you'd prefer to refactor those as well, I'm happy to follow up.

seberg · 2026-01-26T15:43:28Z

+This behavior has been removed to avoid the issue `gh-30354 <https://github.com/numpy/numpy/issues/30354>`__.
+Now, scalar types will not have a ``__ref`` entry in their ``__array_interface__``.
+Instead, ``data`` entry in ``__array_interface__`` will directly point to the memory location of the scalar value and
+will be marked as readonly.


Thanks for the quick update! I was actually thinking the main thing is that we export it as readonly now.

(But maybe I'll just think about it a bit myself and push an update later)

Thanks! That makes sense. Let me know if you'd like me to make any adjustments on my side.

seberg · 2026-01-26T19:48:36Z

+        return NULL;
+    }
+
+    obj = array_protocol_descr_get(PyArray_DescrFromScalar(self));


Hmm, thanks for the update, unfortunately PyArray_DescrFromScalar returns a new reference (and actually can in theory error). So we need to do refcounting here unfortunately.
(I was going to just "suggest" it, but it spans the next block also, a goto finish: that also cleans up inter may be nicer.

Thanks for pointing it out.

I've updated this commit to handle the refcounting for PyArray_DescrFromScalar
in scalartypes.c.src:
5632bf0

In addition, I also refactored the code to use a common helper when creating
__array_interface__ for both arrays and scalars.

Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>

…tion

seberg

OK, thanks @riku-sakamoto for all the follow-ups and checking what it might break. Let's get this in. (Small nit/comment: I think the building function would be slightly nicer to take the dtype/descr object to consolidate more of the logic there, but doens't matter if you feel about it, you can follow up).

If anyone finds this PR because the readonly or ctypes changes affect you, please let us know. I can see this affecting downstream, and if it does we have to check what to do about ctypes.

(I know that using ctypes is quite typical, my assumption is that it is not all that typical to use it on scalars and since it was broken memory ownership wise, I am hoping it isn't used much -- but I realize that the e.g. the fact that we cache this type of small temporary arrays may have made this work in practice!)

Review from an earlier iteration of the PR.

riku-sakamoto added 2 commits December 29, 2025 14:04

FIX: keep a reference to the __ref value

dc1cc42

Scalar types expose `__array_interface__` by creating a temporary array. To ensure the address remains valid, the temporary array must be kept alive. (See numpy#30354)

implement gentype_dataptr_get for scalar __array_interface__

71b0b32

charris changed the title ~~FIX: keep a reference to the __ref value~~ BUG: keep a reference to the __ref value Dec 29, 2025

charris added the 00 - Bug label Dec 29, 2025

riku-sakamoto changed the title ~~BUG: keep a reference to the __ref value~~ BUG: Ensure stable data address in scalar __array_interface__ Dec 29, 2025

tweak the timing of Py_DECREF

2236e5e

riku-sakamoto mentioned this pull request Dec 29, 2025

BUG: ctypes conversions of numpy scalars share the same buffer #30354

Closed

riku-sakamoto marked this pull request as ready for review December 29, 2025 06:47

ngoldbaum reviewed Dec 29, 2025

View reviewed changes

charris added the 09 - Backport-Candidate PRs tagged should be backported label Dec 29, 2025

riku-sakamoto added 2 commits December 30, 2025 13:21

delete inappropriate DECREF

17a6602

add tests

e2cc0ec

ngoldbaum previously requested changes Dec 31, 2025

View reviewed changes

riku-sakamoto added 2 commits January 1, 2026 18:42

add other fields in __array_interface__ for scalars

5247ddc

add test cases

9035770

seberg reviewed Jan 5, 2026

View reviewed changes

seberg added the triage review Issue/PR to be discussed at the next triage meeting label Jan 6, 2026

enh: set readonly flag to scalar's __array_interface__

3f5be5a

seberg added triaged Issue/PR that was discussed in a triage meeting and removed triage review Issue/PR to be discussed at the next triage meeting labels Jan 21, 2026

ngoldbaum added the 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes label Jan 23, 2026

ngoldbaum reviewed Jan 23, 2026

View reviewed changes

DOC: add a release note

2b58cfc

riku-sakamoto force-pushed the fix_scalar_array_interface branch from 076e8c7 to 2b58cfc Compare January 25, 2026 07:43

seberg removed 56 - Needs Release Note. Needs an entry in doc/release/upcoming_changes 09 - Backport-Candidate PRs tagged should be backported labels Jan 26, 2026

seberg approved these changes Jan 26, 2026

View reviewed changes

riku-sakamoto added 2 commits January 26, 2026 23:37

DOC: add a release note about changes in __array_interface__

b21508e

ENH: refactor to use same function

5f97099

seberg reviewed Jan 26, 2026

View reviewed changes

riku-sakamoto and others added 5 commits January 27, 2026 09:27

Update doc/release/upcoming_changes/30538.change.rst

ef13a16

Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>

Update numpy/_core/src/multiarray/descriptor.c

51ea4ea

Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>

fix release note

6238d4c

fix scalartypes.c.src to handle descr's ref count

5632bf0

MAINT: refactor to use a common function for __array_interface__ crea…

eac75cd

…tion

seberg approved these changes Jan 27, 2026

View reviewed changes

seberg merged commit 1e5b6c5 into numpy:main Jan 27, 2026
75 checks passed

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Uh oh!

Conversation

riku-sakamoto commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

riku-sakamoto Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngoldbaum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtsokol Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

riku-sakamoto commented Jan 22, 2026

Uh oh!

ngoldbaum commented Jan 23, 2026

Uh oh!

ngoldbaum left a comment

Choose a reason for hiding this comment

Uh oh!

riku-sakamoto commented Jan 25, 2026

Uh oh!

seberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

riku-sakamoto commented Dec 29, 2025 •

edited

Loading

riku-sakamoto Dec 30, 2025 •

edited

Loading

mtsokol Jan 6, 2026 •

edited

Loading