From f7e14e985be167ca158fd3ee2fe5d8a4f175fa87 Mon Sep 17 00:00:00 2001 From: Francesco Ballarin Date: Sat, 12 Oct 2024 20:19:50 +0200 Subject: [PATCH 1/3] Address regression introduced in #5381 (#5396) * Incomplete attempt to address regression introduced in #5381 * style: pre-commit fixes * Revert "style: pre-commit fixes" This reverts commit 9d107d2f751d76b2c26e90cd08d2ac163022c873. * Revert "Incomplete attempt to address regression introduced in #5381" This reverts commit 8cf1cdbc96ac326ff6d64a36ea291472f74c016f. * Simpler fix for the regression introduced in #5381 * style: pre-commit fixes * Added if constexpr workaround This can probably be done better but at least this is a start. * style: pre-commit fixes * Replace if constexpr with template struct if constexpr was not added until C++ 17. I think this should do the same thing as before. * style: pre-commit fixes * Made comment clearer * Added test cases * style: pre-commit fixes * Fixed is_same_or_base_of reference * style: pre-commit fixes * Added static assert messages * style: pre-commit fixes * Replaced typedef with using * style: pre-commit fixes * Back out `ForwardClassPtr` (to be discussed separately). Tested locally with clang-tidy. * Shuffle new `static_assert()` and leave error messages blank (they are more distracting than helpful here). --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: gentlegiantJGC Co-authored-by: Ralf W. Grosse-Kunstleve --- include/pybind11/cast.h | 15 ++++++++++++--- tests/test_class.cpp | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index c44ff29d3..f6a7e83be 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1564,15 +1564,24 @@ struct function_call { handle init_self; }; +// See PR #5396 for the discussion that led to this +template +struct is_same_or_base_of : std::is_same {}; + +// Only evaluate is_base_of if Derived is complete. +// is_base_of raises a compiler error if Derived is incomplete. +template +struct is_same_or_base_of + : any_of, std::is_base_of> {}; + /// Helper class which loads arguments for C++ functions called from Python template class argument_loader { using indices = make_index_sequence; - template - using argument_is_args = std::is_base_of>; + using argument_is_args = is_same_or_base_of>; template - using argument_is_kwargs = std::is_base_of>; + using argument_is_kwargs = is_same_or_base_of>; // Get kwargs argument position, or -1 if not present: static constexpr auto kwargs_pos = constexpr_last(); diff --git a/tests/test_class.cpp b/tests/test_class.cpp index 9001d86b1..cb84c327a 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -52,8 +52,24 @@ void bind_empty0(py::module_ &m) { } } // namespace pr4220_tripped_over_this + +namespace pr5396_forward_declared_class { +class ForwardClass; +class Args : public py::args {}; +} // namespace pr5396_forward_declared_class + } // namespace test_class +static_assert(py::detail::is_same_or_base_of::value, ""); +static_assert( + py::detail::is_same_or_base_of::value, + ""); +static_assert(!py::detail::is_same_or_base_of< + py::args, + test_class::pr5396_forward_declared_class::ForwardClass>::value, + ""); + TEST_SUBMODULE(class_, m) { m.def("obj_class_name", [](py::handle obj) { return py::detail::obj_class_name(obj.ptr()); }); From 75e48c5f959b4f0a49d8c664e059b6fb4b497102 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20=C5=A0im=C3=A1=C4=8Dek?= Date: Fri, 25 Oct 2024 17:28:15 +0200 Subject: [PATCH 2/3] Skip transient tests on GraalPy (#5422) --- tests/test_call_policies.py | 2 ++ tests/test_class.py | 7 +++++++ tests/test_copy_move.py | 4 ++++ tests/test_custom_type_casters.py | 2 ++ tests/test_eigen_matrix.py | 1 + tests/test_opaque_types.py | 5 ++++- tests/test_operator_overloading.py | 5 ++++- 7 files changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/test_call_policies.py b/tests/test_call_policies.py index 3b614df45..11aab9fd9 100644 --- a/tests/test_call_policies.py +++ b/tests/test_call_policies.py @@ -190,6 +190,7 @@ def test_alive_gc_multi_derived(capture): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_return_none(capture): n_inst = ConstructorStats.detail_reg_inst() with capture: @@ -217,6 +218,7 @@ def test_return_none(capture): assert capture == "Releasing parent." +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_keep_alive_constructor(capture): n_inst = ConstructorStats.detail_reg_inst() diff --git a/tests/test_class.py b/tests/test_class.py index 470d2a326..f424db5c3 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -27,6 +27,9 @@ def test_instance(msg): instance = m.NoConstructor.new_instance() + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") + cstats = ConstructorStats.get(m.NoConstructor) assert cstats.alive() == 1 del instance @@ -35,6 +38,10 @@ def test_instance(msg): def test_instance_new(): instance = m.NoConstructorNew() # .__new__(m.NoConstructor.__class__) + + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") + cstats = ConstructorStats.get(m.NoConstructorNew) assert cstats.alive() == 1 del instance diff --git a/tests/test_copy_move.py b/tests/test_copy_move.py index ee046305f..3a3f29341 100644 --- a/tests/test_copy_move.py +++ b/tests/test_copy_move.py @@ -2,6 +2,7 @@ from __future__ import annotations import pytest +import env # noqa: F401 from pybind11_tests import copy_move_policies as m @@ -17,6 +18,7 @@ def test_lacking_move_ctor(): assert "is neither movable nor copyable!" in str(excinfo.value) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_move_and_copy_casts(): """Cast some values in C++ via custom type casters and count the number of moves/copies.""" @@ -44,6 +46,7 @@ def test_move_and_copy_casts(): assert c_m.alive() + c_mc.alive() + c_c.alive() == 0 +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_move_and_copy_loads(): """Call some functions that load arguments via custom type casters and count the number of moves/copies.""" @@ -77,6 +80,7 @@ def test_move_and_copy_loads(): @pytest.mark.skipif(not m.has_optional, reason="no ") +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_move_and_copy_load_optional(): """Tests move/copy loads of std::optional arguments""" diff --git a/tests/test_custom_type_casters.py b/tests/test_custom_type_casters.py index 689b1e9cb..dcf3ca734 100644 --- a/tests/test_custom_type_casters.py +++ b/tests/test_custom_type_casters.py @@ -2,6 +2,7 @@ from __future__ import annotations import pytest +import env # noqa: F401 from pybind11_tests import custom_type_casters as m @@ -94,6 +95,7 @@ def test_noconvert_args(msg): ) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def test_custom_caster_destruction(): """Tests that returning a pointer to a type that gets converted with a custom type caster gets destroyed when the function has py::return_value_policy::take_ownership policy applied. diff --git a/tests/test_eigen_matrix.py b/tests/test_eigen_matrix.py index 7baf99953..5093ce7d8 100644 --- a/tests/test_eigen_matrix.py +++ b/tests/test_eigen_matrix.py @@ -395,6 +395,7 @@ def test_eigen_return_references(): np.testing.assert_array_equal(a_copy5, c5want) +@pytest.mark.skipif("env.GRAALPY", reason="Cannot reliably trigger GC") def assert_keeps_alive(cl, method, *args): cstats = ConstructorStats.get(cl) start_with = cstats.alive() diff --git a/tests/test_opaque_types.py b/tests/test_opaque_types.py index 342086436..56c9b5db1 100644 --- a/tests/test_opaque_types.py +++ b/tests/test_opaque_types.py @@ -2,6 +2,7 @@ from __future__ import annotations import pytest +import env from pybind11_tests import ConstructorStats, UserType from pybind11_tests import opaque_types as m @@ -30,7 +31,9 @@ def test_pointers(msg): living_before = ConstructorStats.get(UserType).alive() assert m.get_void_ptr_value(m.return_void_ptr()) == 0x1234 assert m.get_void_ptr_value(UserType()) # Should also work for other C++ types - assert ConstructorStats.get(UserType).alive() == living_before + + if not env.GRAALPY: + assert ConstructorStats.get(UserType).alive() == living_before with pytest.raises(TypeError) as excinfo: m.get_void_ptr_value([1, 2, 3]) # This should not work diff --git a/tests/test_operator_overloading.py b/tests/test_operator_overloading.py index 22300eb0f..47949042d 100644 --- a/tests/test_operator_overloading.py +++ b/tests/test_operator_overloading.py @@ -2,7 +2,7 @@ from __future__ import annotations import pytest -import env # noqa: F401 +import env from pybind11_tests import ConstructorStats from pybind11_tests import operators as m @@ -51,6 +51,9 @@ def test_operator_overloading(): v2 /= v1 assert str(v2) == "[2.000000, 8.000000]" + if env.GRAALPY: + pytest.skip("ConstructorStats is incompatible with GraalPy.") + cstats = ConstructorStats.get(m.Vector2) assert cstats.alive() == 3 del v1 From bc041de0db118a30cb5588a435e1d56cac95e60b Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Tue, 5 Nov 2024 13:14:24 -0500 Subject: [PATCH 3/3] Fix buffer protocol implementation (#5407) * Fix buffer protocol implementation According to the buffer protocol, `ndim` is a _required_ field [1], and should always be set correctly. Additionally, `shape` should be set if flags includes `PyBUF_ND` or higher [2]. The current implementation only set those fields if flags was `PyBUF_STRIDES`. [1] https://docs.python.org/3/c-api/buffer.html#request-independent-fields [2] https://docs.python.org/3/c-api/buffer.html#shape-strides-suboffsets * Apply suggestions from review * Obey contiguity requests for buffer protocol If a contiguous buffer is requested, and the underlying buffer isn't, then that should raise. This matches NumPy behaviour if you do something like: ``` struct.unpack_from('5d', np.arange(20.0)[::4]) # Raises for contiguity ``` Also, if a buffer is contiguous, then it can masquerade as a less-complex buffer, either by dropping strides, or even pretending to be 1D. This matches NumPy behaviour if you do something like: ``` a = np.full((3, 5), 30.0) struct.unpack_from('15d', a) # --> Produces 1D tuple from 2D buffer. ``` * Handle review comments * Test buffer protocol against NumPy * Also check PyBUF_FORMAT results --- include/pybind11/detail/class.h | 56 ++++++++++- tests/test_buffers.cpp | 171 ++++++++++++++++++++++++++++++++ tests/test_buffers.py | 160 ++++++++++++++++++++++++++++++ 3 files changed, 382 insertions(+), 5 deletions(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 6c353eb09..6f87f7d4c 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -601,8 +601,10 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla set_error(PyExc_BufferError, "Writable buffer requested for readonly storage"); return -1; } + + // Fill in all the information, and then downgrade as requested by the caller, or raise an + // error if that's not possible. view->obj = obj; - view->ndim = 1; view->internal = info; view->buf = info->ptr; view->itemsize = info->itemsize; @@ -610,15 +612,59 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla for (auto s : info->shape) { view->len *= s; } + view->ndim = static_cast(info->ndim); + view->shape = info->shape.data(); + view->strides = info->strides.data(); view->readonly = static_cast(info->readonly); if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT) { view->format = const_cast(info->format.c_str()); } - if ((flags & PyBUF_STRIDES) == PyBUF_STRIDES) { - view->ndim = (int) info->ndim; - view->strides = info->strides.data(); - view->shape = info->shape.data(); + + // Note, all contiguity flags imply PyBUF_STRIDES and lower. + if ((flags & PyBUF_C_CONTIGUOUS) == PyBUF_C_CONTIGUOUS) { + if (PyBuffer_IsContiguous(view, 'C') == 0) { + std::memset(view, 0, sizeof(Py_buffer)); + delete info; + set_error(PyExc_BufferError, + "C-contiguous buffer requested for discontiguous storage"); + return -1; + } + } else if ((flags & PyBUF_F_CONTIGUOUS) == PyBUF_F_CONTIGUOUS) { + if (PyBuffer_IsContiguous(view, 'F') == 0) { + std::memset(view, 0, sizeof(Py_buffer)); + delete info; + set_error(PyExc_BufferError, + "Fortran-contiguous buffer requested for discontiguous storage"); + return -1; + } + } else if ((flags & PyBUF_ANY_CONTIGUOUS) == PyBUF_ANY_CONTIGUOUS) { + if (PyBuffer_IsContiguous(view, 'A') == 0) { + std::memset(view, 0, sizeof(Py_buffer)); + delete info; + set_error(PyExc_BufferError, "Contiguous buffer requested for discontiguous storage"); + return -1; + } + + } else if ((flags & PyBUF_STRIDES) != PyBUF_STRIDES) { + // If no strides are requested, the buffer must be C-contiguous. + // https://docs.python.org/3/c-api/buffer.html#contiguity-requests + if (PyBuffer_IsContiguous(view, 'C') == 0) { + std::memset(view, 0, sizeof(Py_buffer)); + delete info; + set_error(PyExc_BufferError, + "C-contiguous buffer requested for discontiguous storage"); + return -1; + } + + view->strides = nullptr; + + // Since this is a contiguous buffer, it can also pretend to be 1D. + if ((flags & PyBUF_ND) != PyBUF_ND) { + view->shape = nullptr; + view->ndim = 0; + } } + Py_INCREF(view->obj); return 0; } diff --git a/tests/test_buffers.cpp b/tests/test_buffers.cpp index a6c527c10..ac4489f70 100644 --- a/tests/test_buffers.cpp +++ b/tests/test_buffers.cpp @@ -167,6 +167,125 @@ TEST_SUBMODULE(buffers, m) { sizeof(float)}); }); + // A matrix that uses Fortran storage order. + class FortranMatrix : public Matrix { + public: + FortranMatrix(py::ssize_t rows, py::ssize_t cols) : Matrix(cols, rows) { + print_created(this, + std::to_string(rows) + "x" + std::to_string(cols) + " Fortran matrix"); + } + + float operator()(py::ssize_t i, py::ssize_t j) const { return Matrix::operator()(j, i); } + + float &operator()(py::ssize_t i, py::ssize_t j) { return Matrix::operator()(j, i); } + + using Matrix::data; + + py::ssize_t rows() const { return Matrix::cols(); } + py::ssize_t cols() const { return Matrix::rows(); } + }; + py::class_(m, "FortranMatrix", py::buffer_protocol()) + .def(py::init()) + + .def("rows", &FortranMatrix::rows) + .def("cols", &FortranMatrix::cols) + + /// Bare bones interface + .def("__getitem__", + [](const FortranMatrix &m, std::pair i) { + if (i.first >= m.rows() || i.second >= m.cols()) { + throw py::index_error(); + } + return m(i.first, i.second); + }) + .def("__setitem__", + [](FortranMatrix &m, std::pair i, float v) { + if (i.first >= m.rows() || i.second >= m.cols()) { + throw py::index_error(); + } + m(i.first, i.second) = v; + }) + /// Provide buffer access + .def_buffer([](FortranMatrix &m) -> py::buffer_info { + return py::buffer_info(m.data(), /* Pointer to buffer */ + {m.rows(), m.cols()}, /* Buffer dimensions */ + /* Strides (in bytes) for each index */ + {sizeof(float), sizeof(float) * size_t(m.rows())}); + }); + + // A matrix that uses a discontiguous underlying memory block. + class DiscontiguousMatrix : public Matrix { + public: + DiscontiguousMatrix(py::ssize_t rows, + py::ssize_t cols, + py::ssize_t row_factor, + py::ssize_t col_factor) + : Matrix(rows * row_factor, cols * col_factor), m_row_factor(row_factor), + m_col_factor(col_factor) { + print_created(this, + std::to_string(rows) + "(*" + std::to_string(row_factor) + ")x" + + std::to_string(cols) + "(*" + std::to_string(col_factor) + + ") matrix"); + } + + ~DiscontiguousMatrix() { + print_destroyed(this, + std::to_string(rows() / m_row_factor) + "(*" + + std::to_string(m_row_factor) + ")x" + + std::to_string(cols() / m_col_factor) + "(*" + + std::to_string(m_col_factor) + ") matrix"); + } + + float operator()(py::ssize_t i, py::ssize_t j) const { + return Matrix::operator()(i * m_row_factor, j * m_col_factor); + } + + float &operator()(py::ssize_t i, py::ssize_t j) { + return Matrix::operator()(i * m_row_factor, j * m_col_factor); + } + + using Matrix::data; + + py::ssize_t rows() const { return Matrix::rows() / m_row_factor; } + py::ssize_t cols() const { return Matrix::cols() / m_col_factor; } + py::ssize_t row_factor() const { return m_row_factor; } + py::ssize_t col_factor() const { return m_col_factor; } + + private: + py::ssize_t m_row_factor; + py::ssize_t m_col_factor; + }; + py::class_(m, "DiscontiguousMatrix", py::buffer_protocol()) + .def(py::init()) + + .def("rows", &DiscontiguousMatrix::rows) + .def("cols", &DiscontiguousMatrix::cols) + + /// Bare bones interface + .def("__getitem__", + [](const DiscontiguousMatrix &m, std::pair i) { + if (i.first >= m.rows() || i.second >= m.cols()) { + throw py::index_error(); + } + return m(i.first, i.second); + }) + .def("__setitem__", + [](DiscontiguousMatrix &m, std::pair i, float v) { + if (i.first >= m.rows() || i.second >= m.cols()) { + throw py::index_error(); + } + m(i.first, i.second) = v; + }) + /// Provide buffer access + .def_buffer([](DiscontiguousMatrix &m) -> py::buffer_info { + return py::buffer_info(m.data(), /* Pointer to buffer */ + {m.rows(), m.cols()}, /* Buffer dimensions */ + /* Strides (in bytes) for each index */ + {size_t(m.col_factor()) * sizeof(float) * size_t(m.cols()) + * size_t(m.row_factor()), + size_t(m.col_factor()) * sizeof(float)}); + }); + class BrokenMatrix : public Matrix { public: BrokenMatrix(py::ssize_t rows, py::ssize_t cols) : Matrix(rows, cols) {} @@ -268,4 +387,56 @@ TEST_SUBMODULE(buffers, m) { }); m.def("get_buffer_info", [](const py::buffer &buffer) { return buffer.request(); }); + + // Expose Py_buffer for testing. + m.attr("PyBUF_FORMAT") = PyBUF_FORMAT; + m.attr("PyBUF_SIMPLE") = PyBUF_SIMPLE; + m.attr("PyBUF_ND") = PyBUF_ND; + m.attr("PyBUF_STRIDES") = PyBUF_STRIDES; + m.attr("PyBUF_INDIRECT") = PyBUF_INDIRECT; + m.attr("PyBUF_C_CONTIGUOUS") = PyBUF_C_CONTIGUOUS; + m.attr("PyBUF_F_CONTIGUOUS") = PyBUF_F_CONTIGUOUS; + m.attr("PyBUF_ANY_CONTIGUOUS") = PyBUF_ANY_CONTIGUOUS; + + m.def("get_py_buffer", [](const py::object &object, int flags) { + Py_buffer buffer; + memset(&buffer, 0, sizeof(Py_buffer)); + if (PyObject_GetBuffer(object.ptr(), &buffer, flags) == -1) { + throw py::error_already_set(); + } + + auto SimpleNamespace = py::module_::import("types").attr("SimpleNamespace"); + py::object result = SimpleNamespace("len"_a = buffer.len, + "readonly"_a = buffer.readonly, + "itemsize"_a = buffer.itemsize, + "format"_a = buffer.format, + "ndim"_a = buffer.ndim, + "shape"_a = py::none(), + "strides"_a = py::none(), + "suboffsets"_a = py::none()); + if (buffer.shape != nullptr) { + py::list l; + for (auto i = 0; i < buffer.ndim; i++) { + l.append(buffer.shape[i]); + } + py::setattr(result, "shape", l); + } + if (buffer.strides != nullptr) { + py::list l; + for (auto i = 0; i < buffer.ndim; i++) { + l.append(buffer.strides[i]); + } + py::setattr(result, "strides", l); + } + if (buffer.suboffsets != nullptr) { + py::list l; + for (auto i = 0; i < buffer.ndim; i++) { + l.append(buffer.suboffsets[i]); + } + py::setattr(result, "suboffsets", l); + } + + PyBuffer_Release(&buffer); + return result; + }); } diff --git a/tests/test_buffers.py b/tests/test_buffers.py index 1aff38ea7..2612edb27 100644 --- a/tests/test_buffers.py +++ b/tests/test_buffers.py @@ -239,3 +239,163 @@ def test_buffer_exception(): memoryview(m.BrokenMatrix(1, 1)) assert isinstance(excinfo.value.__cause__, RuntimeError) assert "for context" in str(excinfo.value.__cause__) + + +@pytest.mark.parametrize("type", ["pybind11", "numpy"]) +def test_c_contiguous_to_pybuffer(type): + if type == "pybind11": + mat = m.Matrix(5, 4) + elif type == "numpy": + mat = np.empty((5, 4), dtype=np.float32) + else: + raise ValueError(f"Unknown parametrization {type}") + + info = m.get_py_buffer(mat, m.PyBUF_SIMPLE) + assert info.format is None + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == 5 * 4 * info.itemsize + assert info.ndim == 0 # See discussion on PR #5407. + assert info.shape is None + assert info.strides is None + assert info.suboffsets is None + assert not info.readonly + info = m.get_py_buffer(mat, m.PyBUF_SIMPLE | m.PyBUF_FORMAT) + assert info.format == "f" + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == 5 * 4 * info.itemsize + assert info.ndim == 0 # See discussion on PR #5407. + assert info.shape is None + assert info.strides is None + assert info.suboffsets is None + assert not info.readonly + info = m.get_py_buffer(mat, m.PyBUF_ND) + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == 5 * 4 * info.itemsize + assert info.ndim == 2 + assert info.shape == [5, 4] + assert info.strides is None + assert info.suboffsets is None + assert not info.readonly + info = m.get_py_buffer(mat, m.PyBUF_STRIDES) + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == 5 * 4 * info.itemsize + assert info.ndim == 2 + assert info.shape == [5, 4] + assert info.strides == [4 * info.itemsize, info.itemsize] + assert info.suboffsets is None + assert not info.readonly + info = m.get_py_buffer(mat, m.PyBUF_INDIRECT) + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == 5 * 4 * info.itemsize + assert info.ndim == 2 + assert info.shape == [5, 4] + assert info.strides == [4 * info.itemsize, info.itemsize] + assert info.suboffsets is None # Should be filled in here, but we don't use it. + assert not info.readonly + + +@pytest.mark.parametrize("type", ["pybind11", "numpy"]) +def test_fortran_contiguous_to_pybuffer(type): + if type == "pybind11": + mat = m.FortranMatrix(5, 4) + elif type == "numpy": + mat = np.empty((5, 4), dtype=np.float32, order="F") + else: + raise ValueError(f"Unknown parametrization {type}") + + # A Fortran-shaped buffer can only be accessed at PyBUF_STRIDES level or higher. + info = m.get_py_buffer(mat, m.PyBUF_STRIDES) + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == 5 * 4 * info.itemsize + assert info.ndim == 2 + assert info.shape == [5, 4] + assert info.strides == [info.itemsize, 5 * info.itemsize] + assert info.suboffsets is None + assert not info.readonly + info = m.get_py_buffer(mat, m.PyBUF_INDIRECT) + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == 5 * 4 * info.itemsize + assert info.ndim == 2 + assert info.shape == [5, 4] + assert info.strides == [info.itemsize, 5 * info.itemsize] + assert info.suboffsets is None # Should be filled in here, but we don't use it. + assert not info.readonly + + +@pytest.mark.parametrize("type", ["pybind11", "numpy"]) +def test_discontiguous_to_pybuffer(type): + if type == "pybind11": + mat = m.DiscontiguousMatrix(5, 4, 2, 3) + elif type == "numpy": + mat = np.empty((5 * 2, 4 * 3), dtype=np.float32)[::2, ::3] + else: + raise ValueError(f"Unknown parametrization {type}") + + info = m.get_py_buffer(mat, m.PyBUF_STRIDES) + assert info.itemsize == ctypes.sizeof(ctypes.c_float) + assert info.len == 5 * 4 * info.itemsize + assert info.ndim == 2 + assert info.shape == [5, 4] + assert info.strides == [2 * 4 * 3 * info.itemsize, 3 * info.itemsize] + assert info.suboffsets is None + assert not info.readonly + + +@pytest.mark.parametrize("type", ["pybind11", "numpy"]) +def test_to_pybuffer_contiguity(type): + def check_strides(mat): + # The full block is memset to 0, so fill it with non-zero in real spots. + expected = np.arange(1, 5 * 4 + 1).reshape((5, 4)) + for i in range(5): + for j in range(4): + mat[i, j] = expected[i, j] + # If all strides are correct, the exposed buffer should match the input. + np.testing.assert_array_equal(np.array(mat), expected) + + if type == "pybind11": + cmat = m.Matrix(5, 4) # C contiguous. + fmat = m.FortranMatrix(5, 4) # Fortran contiguous. + dmat = m.DiscontiguousMatrix(5, 4, 2, 3) # Not contiguous. + expected_exception = BufferError + elif type == "numpy": + cmat = np.empty((5, 4), dtype=np.float32) # C contiguous. + fmat = np.empty((5, 4), dtype=np.float32, order="F") # Fortran contiguous. + dmat = np.empty((5 * 2, 4 * 3), dtype=np.float32)[::2, ::3] # Not contiguous. + # NumPy incorrectly raises ValueError; when the minimum NumPy requirement is + # above the version that fixes https://github.com/numpy/numpy/issues/3634 then + # BufferError can be used everywhere. + expected_exception = (BufferError, ValueError) + else: + raise ValueError(f"Unknown parametrization {type}") + + check_strides(cmat) + # Should work in C-contiguous mode, but not Fortran order. + m.get_py_buffer(cmat, m.PyBUF_C_CONTIGUOUS) + m.get_py_buffer(cmat, m.PyBUF_ANY_CONTIGUOUS) + with pytest.raises(expected_exception): + m.get_py_buffer(cmat, m.PyBUF_F_CONTIGUOUS) + + check_strides(fmat) + # These flags imply C-contiguity, so won't work. + with pytest.raises(expected_exception): + m.get_py_buffer(fmat, m.PyBUF_SIMPLE) + with pytest.raises(expected_exception): + m.get_py_buffer(fmat, m.PyBUF_ND) + # Should work in Fortran-contiguous mode, but not C order. + with pytest.raises(expected_exception): + m.get_py_buffer(fmat, m.PyBUF_C_CONTIGUOUS) + m.get_py_buffer(fmat, m.PyBUF_ANY_CONTIGUOUS) + m.get_py_buffer(fmat, m.PyBUF_F_CONTIGUOUS) + + check_strides(dmat) + # Should never work. + with pytest.raises(expected_exception): + m.get_py_buffer(dmat, m.PyBUF_SIMPLE) + with pytest.raises(expected_exception): + m.get_py_buffer(dmat, m.PyBUF_ND) + with pytest.raises(expected_exception): + m.get_py_buffer(dmat, m.PyBUF_C_CONTIGUOUS) + with pytest.raises(expected_exception): + m.get_py_buffer(dmat, m.PyBUF_ANY_CONTIGUOUS) + with pytest.raises(expected_exception): + m.get_py_buffer(dmat, m.PyBUF_F_CONTIGUOUS)