Fix handling of conversion of 0d buffers (#5092)

Also fixes conversion for 64 bit integer buffers. I added a buffer-test package
so we can test 0d buffer conversion code without using numpy. In the future
ideally we should add more test buffer types to buffer-test so that we can move
more coverage of the buffer conversion code out of test_numpy and into the main
test suite.

Resolves pyodide#5084.

.circleci/config.yml

@@ -659,7 +659,7 @@ workflows:
 
       - test-main:
           name: test-core-chrome-nodylink
-          test-params: --runtime=chrome-no-host -m 'not requires_dynamic_linking' -k "not webworker" src packages/micropip  packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/
+          test-params: --runtime=chrome-no-host -m 'not requires_dynamic_linking' -k "not webworker" src packages/micropip  packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/ packages/buffer-test/
           requires:
             - build-core-nodylink
           filters:
@@ -668,7 +668,7 @@ workflows:
 
       - test-main:
           name: test-core-firefox-nodylink
-          test-params: --runtime=firefox-no-host -m 'not requires_dynamic_linking' -k "not webworker" src packages/micropip packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/
+          test-params: --runtime=firefox-no-host -m 'not requires_dynamic_linking' -k "not webworker" src packages/micropip packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/ packages/buffer-test/
           requires:
             - build-core-nodylink
           filters:
@@ -677,7 +677,7 @@ workflows:
 
       - test-main:
           name: test-core-node-nodylink
-          test-params: --runtime=node-no-host -m 'not requires_dynamic_linking' -k "not cmdline_runner" src packages/micropip packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/
+          test-params: --runtime=node-no-host -m 'not requires_dynamic_linking' -k "not cmdline_runner" src packages/micropip packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/ packages/buffer-test/
           requires:
             - build-core-nodylink
           filters:
@@ -686,7 +686,7 @@ workflows:
 
       - test-main:
           name: test-core-chrome
-          test-params: --runtime=chrome-no-host -k "not webworker" src packages/micropip  packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/
+          test-params: --runtime=chrome-no-host -k "not webworker" src packages/micropip  packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/ packages/buffer-test/
           requires:
             - build-core
           filters:
@@ -695,7 +695,7 @@ workflows:
 
       - test-main:
           name: test-core-firefox
-          test-params: --runtime=firefox-no-host -k "not webworker" src packages/micropip packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/
+          test-params: --runtime=firefox-no-host -k "not webworker" src packages/micropip packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/ packages/buffer-test/
           requires:
             - build-core
           filters:
@@ -704,7 +704,7 @@ workflows:
 
       - test-main:
           name: test-core-node
-          test-params: --runtime=node-no-host src packages/micropip packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/
+          test-params: --runtime=node-no-host src packages/micropip packages/fpcast-test packages/sharedlib-test-py/ packages/cpp-exceptions-test/ packages/buffer-test/
           requires:
             - build-core
           filters:

docs/project/changelog.md

@@ -63,6 +63,12 @@ myst:
   for namespace packages.
   {pr}`5039`
 
+- {{ Fix }} It now works to convert a 0d Python buffer to JavaScript.
+  {pr}`5092`
+
+- {{ Fix }} It now works to convert buffers of 64 bit signed or unsigned integers to JavaScript.
+  {pr}`5092`
+
 ### Packages
 
 - Upgraded `scikit-learn` to 1.5.2 {pr}`4823`, {pr}`5016`, {pr}`5072`

packages/buffer-test/buffer-test/buffer-test.c

@@ -0,0 +1,98 @@
+#define PY_SSIZE_T_CLEAN
+#include <Python.h>
+
+// clang-format off
+typedef struct
+{
+  PyObject_HEAD
+  Py_ssize_t byteLength; // invariant: byteLength should be equal to length * itemsize
+  Py_ssize_t length;
+  char data[16];
+  char format[2];
+  Py_ssize_t itemsize;
+} ZeroDBufferObject;
+// clang-format on
+
+static int
+ZeroDBuffer_init(PyObject* o, PyObject* args, PyObject* kwds)
+{
+  ZeroDBufferObject* self = (ZeroDBufferObject*)o;
+  Py_buffer buf;
+  int fmt;
+  if (!PyArg_ParseTuple(args, "Cy*", &fmt, &buf)) {
+    return -1;
+  }
+  for (int i = 0; i < buf.len && i < 16; i++) {
+    self->data[i] = ((char*)buf.buf)[i];
+  }
+  self->itemsize = buf.len;
+  PyBuffer_Release(&buf);
+  self->format[0] = fmt;
+  self->format[1] = 0;
+  return 0;
+}
+
+static void
+ZeroDBuffer_dealloc(PyObject* self)
+{
+}
+
+static int
+ZeroDBuffer_GetBuffer(PyObject* obj, Py_buffer* view, int flags)
+{
+  ZeroDBufferObject* self = (ZeroDBufferObject*)obj;
+  view->obj = NULL;
+  // This gets decremented automatically by PyBuffer_Release (even though
+  // bf_releasebuffer is NULL)
+  Py_INCREF(self);
+
+  view->buf = &self->data;
+  view->obj = (PyObject*)self;
+  view->len = 1;
+  view->readonly = 0;
+  view->itemsize = self->itemsize;
+  view->format = self->format;
+  view->ndim = 0;
+  view->shape = NULL;
+  view->strides = NULL;
+  view->suboffsets = NULL;
+
+  return 0;
+}
+
+static PyBufferProcs ZeroDBuffer_BufferProcs = {
+  .bf_getbuffer = ZeroDBuffer_GetBuffer,
+  .bf_releasebuffer = NULL,
+};
+
+static PyTypeObject ZeroDBufferType = {
+  .tp_name = "ZeroDBuffer",
+  .tp_basicsize = sizeof(ZeroDBufferObject),
+  .tp_dealloc = ZeroDBuffer_dealloc,
+  .tp_as_buffer = &ZeroDBuffer_BufferProcs,
+  .tp_flags = Py_TPFLAGS_DEFAULT,
+  .tp_doc = PyDoc_STR("An internal helper buffer"),
+  .tp_init = ZeroDBuffer_init,
+  .tp_new = PyType_GenericNew,
+};
+
+static struct PyModuleDef module = {
+  PyModuleDef_HEAD_INIT,
+  "buffer_test",       /* name of module */
+  "Tests for buffers", /* module documentation, may be NULL */
+  -1,                  /* size of per-interpreter state of the module,
+                          or -1 if the module keeps state in global variables. */
+};
+
+PyMODINIT_FUNC
+PyInit_buffer_test(void)
+{
+  PyObject* module_object = PyModule_Create(&module);
+  if (module_object == NULL) {
+    return NULL;
+  }
+  if (PyModule_AddType(module_object, &ZeroDBufferType) == -1) {
+    return NULL;
+  }
+  return module_object;
+}

packages/buffer-test/buffer-test/setup.py

@@ -0,0 +1,16 @@
+from setuptools import Extension, setup
+
+setup(
+    name="buffer-test",
+    version="0.1.1",
+    author="Hood Chatham",
+    author_email="roberthoodchatham@gmail.com",
+    description="Test Python buffers",
+    long_description_content_type="text/markdown",
+    classifiers=[
+        "Programming Language :: Python :: 3",
+        "License :: OSI Approved :: MIT License",
+        "Operating System :: OS Independent",
+    ],
+    ext_modules=[Extension("buffer_test", ["buffer-test.c"])],
+)

packages/buffer-test/meta.yaml

@@ -0,0 +1,10 @@
+package:
+  name: buffer-test
+  version: "0.1.1"
+  tag:
+    - core
+    - pyodide.test
+  top-level:
+    - buffer_test
+source:
+  path: buffer-test

packages/buffer-test/test_buffer.py

@@ -0,0 +1,94 @@
+import pytest
+from pytest_pyodide import run_in_pyodide
+
+
+@pytest.mark.requires_dynamic_linking
+@run_in_pyodide(packages=["buffer-test"])
+def test_zerod_buffers(selenium):
+    from buffer_test import ZeroDBuffer
+
+    from pyodide.ffi import to_js
+
+    int8Buf = ZeroDBuffer("b", bytes([((~18) & 255) + 1]))
+    jsInt8Buf = to_js(int8Buf)
+    assert jsInt8Buf.constructor.name == "Int8Array"
+    assert jsInt8Buf.length == 1
+    assert jsInt8Buf.byteLength == 1
+    assert jsInt8Buf[0] == -18
+
+    uint8Buf = ZeroDBuffer("B", bytes([130]))
+    jsUint8Buf = to_js(uint8Buf)
+    assert jsUint8Buf.constructor.name == "Uint8Array"
+    assert jsUint8Buf.length == 1
+    assert jsUint8Buf.byteLength == 1
+    assert jsUint8Buf[0] == 130
+
+    int16Buf = ZeroDBuffer("h", bytes([18, 2]))
+    jsInt16Buf = to_js(int16Buf)
+    assert jsInt16Buf.constructor.name == "Int16Array"
+    assert jsInt16Buf.length == 1
+    assert jsInt16Buf.byteLength == 2
+    assert jsInt16Buf[0] == 18 + 2 * 256
+
+    uint16Buf = ZeroDBuffer("H", bytes([18, 2]))
+    jsUint16Buf = to_js(uint16Buf)
+    assert jsUint16Buf.constructor.name == "Uint16Array"
+    assert jsUint16Buf.length == 1
+    assert jsUint16Buf.byteLength == 2
+    assert jsUint16Buf[0] == 18 + 2 * 256
+
+    int32Buf = ZeroDBuffer("i", bytes([18, 2, 0, 1]))
+    jsInt32Buf = to_js(int32Buf)
+    assert jsInt32Buf.constructor.name == "Int32Array"
+    assert jsInt32Buf.length == 1
+    assert jsInt32Buf.byteLength == 4
+    assert jsInt32Buf[0] == 18 + 2 * 256 + 1 * 256 * 256 * 256
+
+    uint32Buf = ZeroDBuffer("I", bytes([18, 2, 0, 1]))
+    jsUint32Buf = to_js(uint32Buf)
+    assert jsUint32Buf.constructor.name == "Uint32Array"
+    assert jsUint32Buf.length == 1
+    assert jsUint32Buf.byteLength == 4
+    assert jsUint32Buf[0] == 18 + 2 * 256 + 1 * 256 * 256 * 256
+
+    int64Buf = ZeroDBuffer("q", bytes([18, 2, 0, 1, 0, 0, 0, 1]))
+    jsInt64Buf = to_js(int64Buf)
+    assert jsInt64Buf.constructor.name == "BigInt64Array"
+    assert jsInt64Buf.length == 1
+    assert jsInt64Buf.byteLength == 8
+    assert jsInt64Buf[0] == 18 + 2 * 256 + 1 * 256 * 256 * 256 + pow(256, 7)
+
+    uint64Buf = ZeroDBuffer("Q", bytes([18, 2, 0, 1, 0, 0, 0, 1]))
+    jsUint64Buf = to_js(uint64Buf)
+    assert jsUint64Buf.constructor.name == "BigUint64Array"
+    assert jsUint64Buf.length == 1
+    assert jsUint64Buf.byteLength == 8
+    assert jsUint64Buf[0] == 18 + 2 * 256 + 1 * 256 * 256 * 256 + pow(256, 7)
+
+    float32Buf = ZeroDBuffer("f", bytes([0, 0, 224, 64]))
+    jsFloat32Buf = to_js(float32Buf)
+    assert jsFloat32Buf.constructor.name == "Float32Array"
+    assert jsFloat32Buf.length == 1
+    assert jsFloat32Buf.byteLength == 4
+    assert jsFloat32Buf[0] == 7
+
+    float64Buf = ZeroDBuffer("d", bytes([0, 0, 0, 0, 0, 0, 28, 64]))
+    jsFloat64Buf = to_js(float64Buf)
+    assert jsFloat64Buf.constructor.name == "Float64Array"
+    assert jsFloat64Buf.length == 1
+    assert jsFloat64Buf.byteLength == 8
+    assert jsFloat64Buf[0] == 7
+
+    # Attempt to convert float16 array should fail with a ConversionError
+    float16Buf = ZeroDBuffer("e", bytes([0, 71]))
+    err = None
+    try:
+        to_js(float16Buf)
+    except Exception as e:
+        err = e
+
+    assert err
+    try:
+        assert str(err.__cause__) == "Error: Javascript has no Float16 support."
+    finally:
+        del err

src/core/jslib.c

@@ -373,7 +373,7 @@ EM_JS_VAL(JsVal, JsvPromise_Resolve, (JsVal obj), {
 
 // clang-format off
 EM_JS_NUM(errcode, jslib_init_buffers_js, (), {
-  const dtypes_str = ["b", "B", "h", "H", "i", "I", "f", "d"].join(
+  const dtypes_str = Array.from("bBhHiIqQfd").join(
     String.fromCharCode(0)
   );
   const dtypes_ptr = stringToNewUTF8(dtypes_str);
@@ -391,6 +391,8 @@ EM_JS_NUM(errcode, jslib_init_buffers_js, (), {
     ["Uint32Array", [dtypes_map["I"], 4, true]],
     ["Float32Array", [dtypes_map["f"], 4, true]],
     ["Float64Array", [dtypes_map["d"], 8, true]],
+    ["BigInt64Array", [dtypes_map["q"], 8, true]],
+    ["BigUint64Array", [dtypes_map["Q"], 8, true]],
     // These last two default to Uint8. They have checked : false to allow use
     // with other types.
     ["DataView", [dtypes_map["B"], 1, false]],

src/core/jsproxy.c

@@ -3353,7 +3353,7 @@ Buffer_cinit(Buffer* self,
   return 0;
 }
 
-void
+static void
 Buffer_dealloc(PyObject* self)
 {
   PyMem_Free(((Buffer*)self)->data);

src/core/python2js_buffer.js

@@ -175,17 +175,21 @@ Module.python2js_buffer_1d_noncontiguous = function (
  * @private
  */
 Module._python2js_buffer_recursive = function (ptr, curdim, bufferData) {
+  const { shape, strides, ndim, converter, itemsize, suboffsets } = bufferData;
   // Stride and suboffset are signed, n is unsigned.
-  let n = DEREF_U32(bufferData.shape, curdim);
-  let stride = DEREF_I32(bufferData.strides, curdim);
+  let n = DEREF_U32(shape, curdim);
+  let stride = DEREF_I32(strides, curdim);
   let suboffset = -1;
-  if (bufferData.suboffsets !== 0) {
-    suboffset = DEREF_I32(bufferData.suboffsets, curdim);
+  if (ndim === 0) {
+    return converter(Module.python2js_buffer_1d_contiguous(ptr, itemsize, 1));
   }
-  if (curdim === bufferData.ndim - 1) {
+  if (suboffsets !== 0) {
+    suboffset = DEREF_I32(suboffsets, curdim);
+  }
+  if (curdim === ndim - 1) {
     // Last dimension, use appropriate 1d converter
     let arraybuffer;
-    if (stride === bufferData.itemsize && suboffset < 0) {
+    if (stride === itemsize && suboffset < 0) {
       arraybuffer = Module.python2js_buffer_1d_contiguous(ptr, stride, n);
     } else {
       arraybuffer = Module.python2js_buffer_1d_noncontiguous(
@@ -193,10 +197,10 @@ Module._python2js_buffer_recursive = function (ptr, curdim, bufferData) {
         stride,
         suboffset,
         n,
-        bufferData.itemsize,
+        itemsize,
       );
     }
-    return bufferData.converter(arraybuffer);
+    return converter(arraybuffer);
   }
 
   let result = [];