Skip to content

Commit 4e6fa04

Browse files
Update dynd-python codebase to use new py_ref class.
1 parent ef33713 commit 4e6fa04

13 files changed

Lines changed: 378 additions & 356 deletions
Lines changed: 110 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#pragma once
22

3-
#include <dynd/callables/base_callable.hpp>
43
#include "kernels/assign_to_pyarrayobject_kernel.hpp"
4+
#include <dynd/callables/base_callable.hpp>
55

66
/**
77
* This sets up a ckernel to copy from a dynd array
@@ -22,127 +22,128 @@ class assign_to_pyarrayobject_callable : public dynd::nd::base_callable {
2222
return dst_tp;
2323
}
2424

25-
/*
26-
void instantiate(char *data, dynd::nd::kernel_builder *ckb, const dynd::ndt::type &dst_tp, const char *dst_arrmeta,
27-
intptr_t nsrc, const dynd::ndt::type *src_tp, const char *const *src_arrmeta,
28-
dynd::kernel_request_t kernreq, intptr_t nkwd, const dynd::nd::array *kwds,
29-
const std::map<std::string, dynd::ndt::type> &tp_vars)
30-
{
31-
PyObject *dst_obj = *reinterpret_cast<PyObject *const *>(dst_arrmeta);
32-
uintptr_t dst_alignment = reinterpret_cast<const uintptr_t *>(dst_arrmeta)[1];
25+
/*
26+
void instantiate(char *data, dynd::nd::kernel_builder *ckb, const dynd::ndt::type &dst_tp, const char *dst_arrmeta,
27+
intptr_t nsrc, const dynd::ndt::type *src_tp, const char *const *src_arrmeta,
28+
dynd::kernel_request_t kernreq, intptr_t nkwd, const dynd::nd::array *kwds,
29+
const std::map<std::string, dynd::ndt::type> &tp_vars)
30+
{
31+
PyObject *dst_obj = *reinterpret_cast<PyObject *const *>(dst_arrmeta);
32+
uintptr_t dst_alignment = reinterpret_cast<const uintptr_t *>(dst_arrmeta)[1];
3333
34-
PyArray_Descr *dtype = reinterpret_cast<PyArray_Descr *>(dst_obj);
35-
36-
// If there is no object type in the numpy type, get the dynd equivalent
37-
// type and use it to do the copying
38-
if (!PyDataType_FLAGCHK(dtype, NPY_ITEM_HASOBJECT)) {
39-
dynd::ndt::type dst_view_tp = pydynd::_type_from_numpy_dtype(dtype, dst_alignment);
40-
nd::array error_mode = assign_error_fractional;
41-
nd::assign->instantiate(node, NULL, ckb, dst_view_tp, NULL, 1, src_tp, src_arrmeta, kernreq, 1, &error_mode,
42-
std::map<std::string, ndt::type>());
43-
return;
44-
}
34+
PyArray_Descr *dtype = reinterpret_cast<PyArray_Descr *>(dst_obj);
4535
46-
if (PyDataType_ISOBJECT(dtype)) {
47-
dynd::nd::assign->instantiate(node, NULL, ckb, dynd::ndt::make_type<pyobject_type>(), NULL, nsrc, src_tp,
48-
src_arrmeta, kernreq, 0, NULL, tp_vars);
49-
return;
50-
}
51-
52-
if (PyDataType_HASFIELDS(dtype)) {
53-
if (src_tp[0].get_id() != dynd::struct_id && src_tp[0].get_id() != dynd::tuple_id) {
54-
std::stringstream ss;
55-
pydynd::pyobject_ownref dtype_str(PyObject_Str((PyObject *)dtype));
56-
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
57-
<< pydynd::pystring_as_string(dtype_str.get());
58-
throw std::invalid_argument(ss.str());
36+
// If there is no object type in the numpy type, get the dynd equivalent
37+
// type and use it to do the copying
38+
if (!PyDataType_FLAGCHK(dtype, NPY_ITEM_HASOBJECT)) {
39+
dynd::ndt::type dst_view_tp = pydynd::_type_from_numpy_dtype(dtype, dst_alignment);
40+
nd::array error_mode = assign_error_fractional;
41+
nd::assign->instantiate(node, NULL, ckb, dst_view_tp, NULL, 1, src_tp, src_arrmeta, kernreq, 1, &error_mode,
42+
std::map<std::string, ndt::type>());
43+
return;
5944
}
6045
61-
// Get the fields out of the numpy dtype
62-
std::vector<PyArray_Descr *> field_dtypes_orig;
63-
std::vector<std::string> field_names_orig;
64-
std::vector<size_t> field_offsets_orig;
65-
pydynd::extract_fields_from_numpy_struct(dtype, field_dtypes_orig, field_names_orig, field_offsets_orig);
66-
intptr_t field_count = field_dtypes_orig.size();
67-
if (field_count != src_tp[0].extended<dynd::ndt::tuple_type>()->get_field_count()) {
68-
std::stringstream ss;
69-
pydynd::pyobject_ownref dtype_str(PyObject_Str((PyObject *)dtype));
70-
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
71-
<< pydynd::pystring_as_string(dtype_str.get());
72-
throw std::invalid_argument(ss.str());
46+
if (PyDataType_ISOBJECT(dtype)) {
47+
dynd::nd::assign->instantiate(node, NULL, ckb, dynd::ndt::make_type<pyobject_type>(), NULL, nsrc, src_tp,
48+
src_arrmeta, kernreq, 0, NULL, tp_vars);
49+
return;
7350
}
7451
75-
// Permute the numpy fields to match with the dynd fields
76-
std::vector<PyArray_Descr *> field_dtypes;
77-
std::vector<size_t> field_offsets;
78-
if (src_tp[0].get_id() == dynd::struct_id) {
79-
field_dtypes.resize(field_count);
80-
field_offsets.resize(field_count);
81-
for (intptr_t i = 0; i < field_count; ++i) {
82-
intptr_t src_i = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_index(field_names_orig[i]);
83-
if (src_i >= 0) {
84-
field_dtypes[src_i] = field_dtypes_orig[i];
85-
field_offsets[src_i] = field_offsets_orig[i];
86-
}
87-
else {
88-
std::stringstream ss;
89-
pydynd::pyobject_ownref dtype_str(PyObject_Str((PyObject *)dtype));
90-
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
91-
<< pydynd::pystring_as_string(dtype_str.get());
92-
throw std::invalid_argument(ss.str());
52+
if (PyDataType_HASFIELDS(dtype)) {
53+
if (src_tp[0].get_id() != dynd::struct_id && src_tp[0].get_id() != dynd::tuple_id) {
54+
std::stringstream ss;
55+
pydynd::py_ref dtype_str = capture_if_not_null(PyObject_Str((PyObject *)dtype));
56+
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
57+
<< pydynd::pystring_as_string(dtype_str.get());
58+
throw std::invalid_argument(ss.str());
59+
}
60+
61+
// Get the fields out of the numpy dtype
62+
std::vector<PyArray_Descr *> field_dtypes_orig;
63+
std::vector<std::string> field_names_orig;
64+
std::vector<size_t> field_offsets_orig;
65+
pydynd::extract_fields_from_numpy_struct(dtype, field_dtypes_orig, field_names_orig, field_offsets_orig);
66+
intptr_t field_count = field_dtypes_orig.size();
67+
if (field_count != src_tp[0].extended<dynd::ndt::tuple_type>()->get_field_count()) {
68+
std::stringstream ss;
69+
pydynd::py_ref dtype_str = capture_if_not_null(PyObject_Str((PyObject *)dtype));
70+
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
71+
<< pydynd::pystring_as_string(dtype_str.get());
72+
throw std::invalid_argument(ss.str());
73+
}
74+
75+
// Permute the numpy fields to match with the dynd fields
76+
std::vector<PyArray_Descr *> field_dtypes;
77+
std::vector<size_t> field_offsets;
78+
if (src_tp[0].get_id() == dynd::struct_id) {
79+
field_dtypes.resize(field_count);
80+
field_offsets.resize(field_count);
81+
for (intptr_t i = 0; i < field_count; ++i) {
82+
intptr_t src_i = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_index(field_names_orig[i]);
83+
if (src_i >= 0) {
84+
field_dtypes[src_i] = field_dtypes_orig[i];
85+
field_offsets[src_i] = field_offsets_orig[i];
86+
}
87+
else {
88+
std::stringstream ss;
89+
pydynd::py_ref dtype_str = capture_if_not_null(PyObject_Str((PyObject *)dtype));
90+
ss << "Cannot assign from source dynd type " << src_tp[0] << " to numpy type "
91+
<< pydynd::pystring_as_string(dtype_str.get());
92+
throw std::invalid_argument(ss.str());
93+
}
9394
}
9495
}
95-
}
96-
else {
97-
// In the tuple case, use position instead of name
98-
field_dtypes.swap(field_dtypes_orig);
99-
field_offsets.swap(field_offsets_orig);
100-
}
96+
else {
97+
// In the tuple case, use position instead of name
98+
field_dtypes.swap(field_dtypes_orig);
99+
field_offsets.swap(field_offsets_orig);
100+
}
101101
102-
std::vector<dynd::ndt::type> dst_fields_tp(field_count, dynd::ndt::make_type<void>());
103-
std::vector<copy_to_numpy_arrmeta> dst_arrmeta_values(field_count);
104-
std::vector<const char *> dst_fields_arrmeta(field_count);
105-
for (intptr_t i = 0; i < field_count; ++i) {
106-
dst_arrmeta_values[i].dst_dtype = field_dtypes[i];
107-
dst_arrmeta_values[i].dst_alignment = dst_alignment | field_offsets[i];
108-
dst_fields_arrmeta[i] = reinterpret_cast<const char *>(&dst_arrmeta_values[i]);
109-
}
102+
std::vector<dynd::ndt::type> dst_fields_tp(field_count, dynd::ndt::make_type<void>());
103+
std::vector<copy_to_numpy_arrmeta> dst_arrmeta_values(field_count);
104+
std::vector<const char *> dst_fields_arrmeta(field_count);
105+
for (intptr_t i = 0; i < field_count; ++i) {
106+
dst_arrmeta_values[i].dst_dtype = field_dtypes[i];
107+
dst_arrmeta_values[i].dst_alignment = dst_alignment | field_offsets[i];
108+
dst_fields_arrmeta[i] = reinterpret_cast<const char *>(&dst_arrmeta_values[i]);
109+
}
110110
111-
const uintptr_t *src_arrmeta_offsets = src_tp[0].extended<dynd::ndt::tuple_type>()->get_arrmeta_offsets_raw();
112-
dynd::shortvector<const char *> src_fields_arrmeta(field_count);
113-
for (intptr_t i = 0; i != field_count; ++i) {
114-
src_fields_arrmeta[i] = src_arrmeta[0] + src_arrmeta_offsets[i];
115-
}
111+
const uintptr_t *src_arrmeta_offsets = src_tp[0].extended<dynd::ndt::tuple_type>()->get_arrmeta_offsets_raw();
112+
dynd::shortvector<const char *> src_fields_arrmeta(field_count);
113+
for (intptr_t i = 0; i != field_count; ++i) {
114+
src_fields_arrmeta[i] = src_arrmeta[0] + src_arrmeta_offsets[i];
115+
}
116116
117-
// Todo: Remove this
118-
dynd::nd::callable af = dynd::nd::make_callable<assign_to_pyarrayobject_callable>();
117+
// Todo: Remove this
118+
dynd::nd::callable af = dynd::nd::make_callable<assign_to_pyarrayobject_callable>();
119119
120-
const std::vector<ndt::type> &src_field_tp = src_tp[0].extended<dynd::ndt::tuple_type>()->get_field_types();
121-
const uintptr_t *src_data_offsets = src_tp[0].extended<dynd::ndt::tuple_type>()->get_data_offsets(src_arrmeta[0]);
120+
const std::vector<ndt::type> &src_field_tp = src_tp[0].extended<dynd::ndt::tuple_type>()->get_field_types();
121+
const uintptr_t *src_data_offsets =
122+
src_tp[0].extended<dynd::ndt::tuple_type>()->get_data_offsets(src_arrmeta[0]);
122123
123-
intptr_t self_offset = ckb->size();
124-
ckb->emplace_back<nd::tuple_unary_op_ck>(kernreq);
125-
nd::tuple_unary_op_ck *self = ckb->get_at<nd::tuple_unary_op_ck>(self_offset);
126-
self->m_fields.resize(field_count);
127-
for (intptr_t i = 0; i < field_count; ++i) {
128-
self = ckb->get_at<nd::tuple_unary_op_ck>(self_offset);
129-
nd::tuple_unary_op_item &field = self->m_fields[i];
130-
field.child_kernel_offset = ckb->size() - self_offset;
131-
field.dst_data_offset = field_offsets[i];
132-
field.src_data_offset = src_data_offsets[i];
133-
nd::array error_mode = ndt::traits<assign_error_mode>::na();
134-
af->instantiate(node, NULL, ckb, dst_fields_tp[i], dst_fields_arrmeta[i], 1, &src_field_tp[i],
135-
&src_fields_arrmeta[i], kernel_request_single, 1, &error_mode,
136-
std::map<std::string, ndt::type>());
124+
intptr_t self_offset = ckb->size();
125+
ckb->emplace_back<nd::tuple_unary_op_ck>(kernreq);
126+
nd::tuple_unary_op_ck *self = ckb->get_at<nd::tuple_unary_op_ck>(self_offset);
127+
self->m_fields.resize(field_count);
128+
for (intptr_t i = 0; i < field_count; ++i) {
129+
self = ckb->get_at<nd::tuple_unary_op_ck>(self_offset);
130+
nd::tuple_unary_op_item &field = self->m_fields[i];
131+
field.child_kernel_offset = ckb->size() - self_offset;
132+
field.dst_data_offset = field_offsets[i];
133+
field.src_data_offset = src_data_offsets[i];
134+
nd::array error_mode = ndt::traits<assign_error_mode>::na();
135+
af->instantiate(node, NULL, ckb, dst_fields_tp[i], dst_fields_arrmeta[i], 1, &src_field_tp[i],
136+
&src_fields_arrmeta[i], kernel_request_single, 1, &error_mode,
137+
std::map<std::string, ndt::type>());
138+
}
139+
return;
140+
}
141+
else {
142+
std::stringstream ss;
143+
ss << "TODO: implement assign from source dynd type " << src_tp[0] << " to numpy type "
144+
<< pydynd::pyobject_repr((PyObject *)dtype);
145+
throw std::invalid_argument(ss.str());
137146
}
138-
return;
139-
}
140-
else {
141-
std::stringstream ss;
142-
ss << "TODO: implement assign from source dynd type " << src_tp[0] << " to numpy type "
143-
<< pydynd::pyobject_repr((PyObject *)dtype);
144-
throw std::invalid_argument(ss.str());
145147
}
146-
}
147-
*/
148+
*/
148149
};

dynd/include/callables/assign_to_pyobject_callable.hpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,11 +289,12 @@ namespace nd {
289289
ckb_offset = kb.size();
290290
self_ck->m_src_tp = src0_tp;
291291
self_ck->m_src_arrmeta = src_arrmeta[0];
292-
self_ck->m_field_names.reset(PyTuple_New(field_count));
292+
self_ck->m_field_names = capture_if_not_null(PyTuple_New(field_count));
293293
for (intptr_t i = 0; i < field_count; ++i) {
294294
const dynd::string &rawname = src0_tp.extended<dynd::ndt::struct_type>()->get_field_name(i);
295-
pydynd::pyobject_ownref name(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
296-
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, name.release());
295+
pydynd::py_ref name =
296+
capture_if_not_null(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
297+
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, name.get());
297298
}
298299
self_ck->m_copy_el_offsets.resize(field_count);
299300

@@ -333,10 +334,11 @@ namespace nd {
333334
intptr_t field_count = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_count();
334335
const dynd::ndt::type *field_types = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_types_raw();
335336
const uintptr_t *arrmeta_offsets = src_tp[0].extended<dynd::ndt::struct_type>()->get_arrmeta_offsets_raw();
336-
self_ck->m_field_names.reset(PyTuple_New(field_count));
337+
self_ck->m_field_names = capture_if_not_null(PyTuple_New(field_count));
337338
for (intptr_t i = 0; i < field_count; ++i) {
338339
const dynd::string &rawname = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_name(i);
339-
pydynd::pyobject_ownref name(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
340+
pydynd::py_ref name =
341+
capture_if_not_null(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
340342
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, name.release());
341343
}
342344
self_ck->m_copy_el_offsets.resize(field_count);

dynd/include/kernels/apply_pyobject_kernel.hpp

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
3737
if (Py_REFCNT(item) != 1 || pydynd::array_to_cpp_ref(item)->get_use_count() != 1) {
3838
std::stringstream ss;
3939
ss << "Python callback function ";
40-
pydynd::pyobject_ownref pyfunc_repr(PyObject_Repr(m_pyfunc));
40+
pydynd::py_ref pyfunc_repr = pydynd::capture_if_not_null(PyObject_Repr(m_pyfunc));
4141
ss << pydynd::pystring_as_string(pyfunc_repr.get());
4242
ss << ", called by dynd, held a reference to parameter ";
4343
ss << (i + 1) << " which contained temporary memory.";
@@ -70,7 +70,7 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
7070
const dynd::ndt::type &dst_tp = fpt->get_return_type();
7171
const std::vector<dynd::ndt::type> &src_tp = fpt->get_argument_types();
7272
// First set up the parameters in a tuple
73-
pydynd::pyobject_ownref args(PyTuple_New(nsrc));
73+
pydynd::py_ref args = pydynd::capture_if_not_null(PyTuple_New(nsrc));
7474
for (intptr_t i = 0; i != nsrc; ++i) {
7575
dynd::ndt::type tp = src_tp[i];
7676
dynd::nd::array n = dynd::nd::make_array(tp, const_cast<char *>(src[i]), dynd::nd::read_access_flag);
@@ -80,12 +80,15 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
8080
PyTuple_SET_ITEM(args.get(), i, pydynd::array_from_cpp(std::move(n)));
8181
}
8282
// Now call the function
83-
pydynd::pyobject_ownref res(PyObject_Call(m_pyfunc, args.get(), NULL));
84-
// Copy the result into the destination memory
85-
PyObject *child_obj = res.get();
86-
char *child_src = reinterpret_cast<char *>(&child_obj);
87-
get_child()->single(dst, &child_src);
88-
res.clear();
83+
PyObject *child_obj;
84+
char *child_src;
85+
{ // This scope exists to limit the lifetime of py_ref res.
86+
pydynd::py_ref res = pydynd::capture_if_not_null(PyObject_Call(m_pyfunc, args.get(), NULL));
87+
// Copy the result into the destination memory
88+
child_obj = res.get();
89+
child_src = reinterpret_cast<char *>(&child_obj);
90+
get_child()->single(dst, &child_src);
91+
}
8992
// Validate that the call didn't hang onto the ephemeral data
9093
// pointers we used. This is done after the dst assignment, because
9194
// the function result may have contained a reference to an argument.
@@ -99,7 +102,7 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
99102
const dynd::ndt::type &dst_tp = fpt->get_return_type();
100103
const std::vector<dynd::ndt::type> &src_tp = fpt->get_argument_types();
101104
// First set up the parameters in a tuple
102-
pydynd::pyobject_ownref args(PyTuple_New(nsrc));
105+
pydynd::py_ref args = pydynd::capture_if_not_null(PyTuple_New(nsrc));
103106
for (intptr_t i = 0; i != nsrc; ++i) {
104107
dynd::ndt::type tp = src_tp[i];
105108
dynd::nd::array n = dynd::nd::make_array(tp, const_cast<char *>(src[i]), dynd::nd::read_access_flag);
@@ -111,12 +114,15 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
111114
// Do the loop, reusing the args we created
112115
for (size_t j = 0; j != count; ++j) {
113116
// Call the function
114-
pydynd::pyobject_ownref res(PyObject_Call(m_pyfunc, args.get(), NULL));
115-
// Copy the result into the destination memory
116-
PyObject *child_obj = res.get();
117-
char *child_src = reinterpret_cast<char *>(&child_obj);
118-
get_child()->single(dst, &child_src);
119-
res.clear();
117+
PyObject *child_obj;
118+
char *child_src;
119+
{ // Scope to hold lifetime of py_ref res.
120+
pydynd::py_ref res = pydynd::capture_if_not_null(PyObject_Call(m_pyfunc, args.get(), NULL));
121+
// Copy the result into the destination memory
122+
child_obj = res.get();
123+
child_src = reinterpret_cast<char *>(&child_obj);
124+
get_child()->single(dst, &child_src);
125+
}
120126
// Validate that the call didn't hang onto the ephemeral data
121127
// pointers we used. This is done after the dst assignment, because
122128
// the function result may have contained a reference to an argument.
@@ -125,7 +131,7 @@ struct apply_pyobject_kernel : dynd::nd::base_strided_kernel<apply_pyobject_kern
125131
dst += dst_stride;
126132
for (intptr_t i = 0; i != nsrc; ++i) {
127133
const dynd::nd::array &n = pydynd::array_to_cpp_ref(PyTuple_GET_ITEM(args.get(), i));
128-
// n->set_data(n->get_data() + src_stride[i]);
134+
// n->set_data(n->get_data() + src_stride[i]);
129135
}
130136
}
131137
}

0 commit comments

Comments
 (0)