Skip to content

support for readonly buffers (#863) #1466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions include/pybind11/buffer_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,33 +22,38 @@ struct buffer_info {
ssize_t ndim = 0; // Number of dimensions
std::vector<ssize_t> shape; // Shape of the tensor (1 entry per dimension)
std::vector<ssize_t> strides; // Number of bytes between adjacent entries (for each per dimension)
bool readonly = false; // flag to indicate if the underlying storage may be written to

buffer_info() { }

buffer_info(void *ptr, ssize_t itemsize, const std::string &format, ssize_t ndim,
detail::any_container<ssize_t> shape_in, detail::any_container<ssize_t> strides_in)
detail::any_container<ssize_t> shape_in, detail::any_container<ssize_t> strides_in, bool readonly=false)
: ptr(ptr), itemsize(itemsize), size(1), format(format), ndim(ndim),
shape(std::move(shape_in)), strides(std::move(strides_in)) {
shape(std::move(shape_in)), strides(std::move(strides_in)), readonly(readonly) {
if (ndim != (ssize_t) shape.size() || ndim != (ssize_t) strides.size())
pybind11_fail("buffer_info: ndim doesn't match shape and/or strides length");
for (size_t i = 0; i < (size_t) ndim; ++i)
size *= shape[i];
}

template <typename T>
buffer_info(T *ptr, detail::any_container<ssize_t> shape_in, detail::any_container<ssize_t> strides_in)
: buffer_info(private_ctr_tag(), ptr, sizeof(T), format_descriptor<T>::format(), static_cast<ssize_t>(shape_in->size()), std::move(shape_in), std::move(strides_in)) { }
buffer_info(T *ptr, detail::any_container<ssize_t> shape_in, detail::any_container<ssize_t> strides_in, bool readonly=false)
: buffer_info(private_ctr_tag(), ptr, sizeof(T), format_descriptor<T>::format(), static_cast<ssize_t>(shape_in->size()), std::move(shape_in), std::move(strides_in), readonly) { }

buffer_info(void *ptr, ssize_t itemsize, const std::string &format, ssize_t size)
: buffer_info(ptr, itemsize, format, 1, {size}, {itemsize}) { }
buffer_info(void *ptr, ssize_t itemsize, const std::string &format, ssize_t size, bool readonly=false)
: buffer_info(ptr, itemsize, format, 1, {size}, {itemsize}, readonly) { }

template <typename T>
buffer_info(T *ptr, ssize_t size)
: buffer_info(ptr, sizeof(T), format_descriptor<T>::format(), size) { }
buffer_info(T *ptr, ssize_t size, bool readonly=false)
: buffer_info(ptr, sizeof(T), format_descriptor<T>::format(), size, readonly) { }

template <typename T>
buffer_info(const T *ptr, ssize_t size, bool readonly=true)
: buffer_info(const_cast<T*>(ptr), sizeof(T), format_descriptor<T>::format(), size, readonly) { }

explicit buffer_info(Py_buffer *view, bool ownview = true)
: buffer_info(view->buf, view->itemsize, view->format, view->ndim,
{view->shape, view->shape + view->ndim}, {view->strides, view->strides + view->ndim}) {
{view->shape, view->shape + view->ndim}, {view->strides, view->strides + view->ndim}, view->readonly) {
this->view = view;
this->ownview = ownview;
}
Expand All @@ -70,6 +75,7 @@ struct buffer_info {
strides = std::move(rhs.strides);
std::swap(view, rhs.view);
std::swap(ownview, rhs.ownview);
readonly = rhs.readonly;
return *this;
}

Expand All @@ -81,8 +87,8 @@ struct buffer_info {
struct private_ctr_tag { };

buffer_info(private_ctr_tag, void *ptr, ssize_t itemsize, const std::string &format, ssize_t ndim,
detail::any_container<ssize_t> &&shape_in, detail::any_container<ssize_t> &&strides_in)
: buffer_info(ptr, itemsize, format, ndim, std::move(shape_in), std::move(strides_in)) { }
detail::any_container<ssize_t> &&shape_in, detail::any_container<ssize_t> &&strides_in, bool readonly)
: buffer_info(ptr, itemsize, format, ndim, std::move(shape_in), std::move(strides_in), readonly) { }

Py_buffer *view = nullptr;
bool ownview = false;
Expand Down
7 changes: 7 additions & 0 deletions include/pybind11/detail/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,13 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla
view->len = view->itemsize;
for (auto s : info->shape)
view->len *= s;
view->readonly = info->readonly;
if ((flags & PyBUF_WRITABLE) == PyBUF_WRITABLE && info->readonly) {
if (view)
view->obj = nullptr;
PyErr_SetString(PyExc_BufferError, "Writable buffer requested for readonly storage");
return -1;
}
if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT)
view->format = const_cast<char *>(info->format.c_str());
if ((flags & PyBUF_STRIDES) == PyBUF_STRIDES) {
Expand Down
2 changes: 1 addition & 1 deletion include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1345,7 +1345,7 @@ class memoryview : public object {
buf.strides = py_strides.data();
buf.shape = py_shape.data();
buf.suboffsets = nullptr;
buf.readonly = false;
buf.readonly = info.readonly;
buf.internal = nullptr;

m_ptr = PyMemoryView_FromBuffer(&buf);
Expand Down
2 changes: 1 addition & 1 deletion tests/constructor_stats.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ class ConstructorStats {
}
}
}
catch (const std::out_of_range &) {}
catch (const std::out_of_range&) {}
if (!t1) throw std::runtime_error("Unknown class passed to ConstructorStats::get()");
auto &cs1 = get(*t1);
// If we have both a t1 and t2 match, one is probably the trampoline class; return whichever
Expand Down
26 changes: 26 additions & 0 deletions tests/test_buffers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,30 @@ TEST_SUBMODULE(buffers, m) {
.def_readwrite("value", (int32_t DerivedBuffer::*) &DerivedBuffer::value)
.def_buffer(&DerivedBuffer::get_buffer_info);

struct BufferReadOnly {
const uint8_t value = 0;
BufferReadOnly(uint8_t value): value(value) {}

py::buffer_info get_buffer_info() {
return py::buffer_info(&value, 1);
}
};
py::class_<BufferReadOnly>(m, "BufferReadOnly", py::buffer_protocol())
.def(py::init<uint8_t>())
.def_buffer(&BufferReadOnly::get_buffer_info);

struct BufferReadOnlySelect {
uint8_t value = 0;
bool readonly = false;

py::buffer_info get_buffer_info() {
return py::buffer_info(&value, 1, readonly);
}
};
py::class_<BufferReadOnlySelect>(m, "BufferReadOnlySelect", py::buffer_protocol())
.def(py::init<>())
.def_readwrite("value", &BufferReadOnlySelect::value)
.def_readwrite("readonly", &BufferReadOnlySelect::readonly)
.def_buffer(&BufferReadOnlySelect::get_buffer_info);

}
31 changes: 31 additions & 0 deletions tests/test_buffers.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import io
import struct
import sys

import pytest

from pybind11_tests import buffers as m
from pybind11_tests import ConstructorStats

PY3 = sys.version_info[0] >= 3

pytestmark = pytest.requires_numpy

with pytest.suppress(ImportError):
Expand Down Expand Up @@ -85,3 +91,28 @@ def test_pointer_to_member_fn():
buf.value = 0x12345678
value = struct.unpack('i', bytearray(buf))[0]
assert value == 0x12345678


@pytest.unsupported_on_pypy
def test_readonly_buffer():
buf = m.BufferReadOnly(0x64)
view = memoryview(buf)
assert view[0] == 0x64 if PY3 else b'd'
assert view.readonly


@pytest.unsupported_on_pypy
def test_selective_readonly_buffer():
buf = m.BufferReadOnlySelect()

memoryview(buf)[0] = 0x64 if PY3 else b'd'
assert buf.value == 0x64

io.BytesIO(b'A').readinto(buf)
assert buf.value == ord(b'A')

buf.readonly = True
with pytest.raises(TypeError):
memoryview(buf)[0] = 0 if PY3 else b'\0'
with pytest.raises(TypeError):
io.BytesIO(b'1').readinto(buf)