Skip to content
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

Add format_descriptor<> & npy_format_descriptor<> PyObject * specializations. #4674

Merged
merged 24 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5168c13
Add `npy_format_descriptor<PyObject *>` to enable `py::array_t<PyObje…
May 17, 2023
d53a796
resolve clang-tidy warning
May 17, 2023
5bea2a8
Use existing constructor instead of adding a static method. Thanks @S…
May 17, 2023
50eaa3a
Add `format_descriptor<PyObject *>`
May 17, 2023
82ce80f
Add test_format_descriptor_format
May 18, 2023
20b9baf
Ensure the Eigen `type_caster`s do not segfault when loading arrays w…
May 18, 2023
0640eb3
Use `static_assert()` `!std::is_pointer<>` to replace runtime guards.
May 18, 2023
ddb625e
Add comments to explain how to check for ref-count bugs. (NO code cha…
May 18, 2023
03dafde
Make the "Pointer types ... are not supported" message Eigen-specific…
May 18, 2023
28492ed
Change "format_descriptor_format" implementation as suggested by @Lal…
May 18, 2023
1593ebc
resolve clang-tidy warning
May 18, 2023
3f04188
Account for np.float128, np.complex256 not being available on Windows…
May 18, 2023
38aa697
Fully address i|q|l ambiguity (hopefully).
May 18, 2023
7f124bb
Remove the new `np.format_parser()`-based test, it's much more distra…
May 19, 2023
d432ce7
Use bi.itemsize to disambiguate "l" or "L"
May 19, 2023
18e1bd2
Use `py::detail::compare_buffer_info<T>::compare()` to validate the `…
May 19, 2023
029b157
Add `buffer_info::compare<T>` to make `detail::compare_buffer_info<T>…
May 19, 2023
d9e3bd3
silence clang-tidy warning
May 19, 2023
e9a289c
pytest-compatible access to np.float128, np.complex256
May 19, 2023
8abe0e9
Revert "pytest-compatible access to np.float128, np.complex256"
May 19, 2023
b09e75b
Use `sizeof(long double) == sizeof(double)` instead of `std::is_same<>`
May 19, 2023
ba7063e
Report skipped `long double` tests.
May 19, 2023
a4d61b4
Change the name of the new `buffer_info` member function to `item_typ…
May 19, 2023
ef34d29
Change `item_type_is_equivalent_to<>()` from `static` function to mem…
May 20, 2023
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
26 changes: 20 additions & 6 deletions include/pybind11/numpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,16 @@ class dtype : public object {
return detail::npy_format_descriptor<typename std::remove_cv<T>::type>::dtype();
}

/// Return dtype for the given typenum (one of the NPY_TYPES).
/// https://numpy.org/devdocs/reference/c-api/array.html#c.PyArray_DescrFromType
static dtype from_typenum(int typenum) {
auto *ptr = detail::npy_api::get().PyArray_DescrFromType_(typenum);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought dtype already have an explicit int ctor for this. Probably should have made it a static method in retrospect, but it's already there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out, changed. The explicit ctor is fine IMO, I just didn't look around at all.

if (!ptr) {
throw error_already_set();
}
return reinterpret_steal<dtype>(ptr);
}

/// Size of the data type in bytes.
ssize_t itemsize() const { return detail::array_descriptor_proxy(m_ptr)->elsize; }

Expand Down Expand Up @@ -1283,12 +1293,16 @@ struct npy_format_descriptor<
public:
static constexpr int value = values[detail::is_fmt_numeric<T>::index];

static pybind11::dtype dtype() {
if (auto *ptr = npy_api::get().PyArray_DescrFromType_(value)) {
return reinterpret_steal<pybind11::dtype>(ptr);
}
pybind11_fail("Unsupported buffer format!");
}
static pybind11::dtype dtype() { return pybind11::dtype::from_typenum(value); }
};

template <typename T>
struct npy_format_descriptor<T, enable_if_t<is_same_ignoring_cvref<T, PyObject *>::value>> {
static constexpr auto name = const_name("object");

static constexpr int value = npy_api::NPY_OBJECT_;

static pybind11::dtype dtype() { return pybind11::dtype::from_typenum(value); }
};

#define PYBIND11_DECL_CHAR_FMT \
Expand Down
26 changes: 26 additions & 0 deletions tests/test_numpy_array.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -523,4 +523,30 @@ TEST_SUBMODULE(numpy_array, sm) {
sm.def("test_fmt_desc_const_double", [](const py::array_t<const double> &) {});

sm.def("round_trip_float", [](double d) { return d; });

sm.def("pass_array_pyobject_ptr_return_sum_str_values",
[](const py::array_t<PyObject *> &objs) {
std::string sum_str_values;
for (auto &obj : objs) {
sum_str_values += py::str(obj.attr("value"));
}
return sum_str_values;
});

sm.def("pass_array_pyobject_ptr_return_as_list",
[](const py::array_t<PyObject *> &objs) -> py::list { return objs; });

sm.def("return_array_pyobject_ptr_cpp_loop", [](const py::list &objs) {
py::size_t arr_size = py::len(objs);
py::array_t<PyObject *> arr_from_list(static_cast<py::ssize_t>(arr_size));
PyObject **data = arr_from_list.mutable_data();
for (py::size_t i = 0; i < arr_size; i++) {
assert(data[i] == nullptr);
data[i] = py::cast<PyObject *>(objs[i].attr("value"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a silly question, but does this appropriately increase the reference count?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a silly question, this was something I was struggling with quite a bit:

// Note that `cast<PyObject *>(obj)` increments the reference count of `obj`.
// This is necessary for the case that `obj` is a temporary, and could
// not possibly be different, given
// 1. the established convention that the passed `handle` is borrowed, and
// 2. we don't want to force all generic code using `cast<T>()` to special-case
// handling of `T` = `PyObject *` (to increment the reference count there).
// It is the responsibility of the caller to ensure that the reference count
// is decremented.
template <typename T,
typename Handle,
detail::enable_if_t<detail::is_same_ignoring_cvref<T, PyObject *>::value
&& detail::is_same_ignoring_cvref<Handle, handle>::value,
int>
= 0>
T cast(Handle &&handle) {
return handle.inc_ref().ptr();
}

}
return arr_from_list;
});

sm.def("return_array_pyobject_ptr_from_list",
[](const py::list &objs) -> py::array_t<PyObject *> { return objs; });
}
58 changes: 58 additions & 0 deletions tests/test_numpy_array.py
Original file line number Diff line number Diff line change
Expand Up @@ -595,3 +595,61 @@ def test_round_trip_float():
arr = np.zeros((), np.float64)
arr[()] = 37.2
assert m.round_trip_float(arr) == 37.2


# For use as a temporary user-defined object, to maximize sensitivity of the tests below.
class PyValueHolder:
def __init__(self, value):
self.value = value


def WrapWithPyValueHolder(*values):
return [PyValueHolder(v) for v in values]


def UnwrapPyValueHolder(vhs):
return [vh.value for vh in vhs]


def test_pass_array_pyobject_ptr_return_sum_str_values_ndarray():
# Intentionally all temporaries, do not change.
assert (
m.pass_array_pyobject_ptr_return_sum_str_values(
np.array(WrapWithPyValueHolder(-3, "four", 5.0), dtype=object)
)
== "-3four5.0"
)


def test_pass_array_pyobject_ptr_return_sum_str_values_list():
# Intentionally all temporaries, do not change.
assert (
m.pass_array_pyobject_ptr_return_sum_str_values(
WrapWithPyValueHolder(2, "three", -4.0)
)
== "2three-4.0"
)


def test_pass_array_pyobject_ptr_return_as_list():
# Intentionally all temporaries, do not change.
assert UnwrapPyValueHolder(
m.pass_array_pyobject_ptr_return_as_list(
np.array(WrapWithPyValueHolder(-1, "two", 3.0), dtype=object)
)
) == [-1, "two", 3.0]


@pytest.mark.parametrize(
("return_array_pyobject_ptr", "unwrap"),
[
(m.return_array_pyobject_ptr_cpp_loop, list),
(m.return_array_pyobject_ptr_from_list, UnwrapPyValueHolder),
],
)
def test_return_array_pyobject_ptr_cpp_loop(return_array_pyobject_ptr, unwrap):
# Intentionally all temporaries, do not change.
arr_from_list = return_array_pyobject_ptr(WrapWithPyValueHolder(6, "seven", -8.0))
assert isinstance(arr_from_list, np.ndarray)
assert arr_from_list.dtype == np.dtype("O")
assert unwrap(arr_from_list) == [6, "seven", -8.0]