Skip to content

Commit 2e4b655

Browse files
robertnishihararobertnishihara
authored
Merge pull request #11 from pcmoritz/callbackerr
Propagate callback error through
2 parents c4c33bd + a428806 commit 2e4b655

File tree

4 files changed

+38
-33
lines changed

4 files changed

+38
-33
lines changed

‎python/src/pynumbuf/adapters/numpy.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ Status SerializeArray(PyArrayObject* array, SequenceBuilder& builder,
117117
PyObject* result = PyObject_CallObject(numbuf_serialize_callback, arglist);
118118
if (!result) {
119119
Py_XDECREF(arglist);
120-
return python_error_to_status();
120+
return Status::NotImplemented("python error"); // TODO(pcm): https://github.com/pcmoritz/numbuf/issues/10
121121
}
122122
builder.AppendDict(PyDict_Size(result));
123123
subdicts.push_back(result);

‎python/src/pynumbuf/adapters/python.cc‎

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -11,56 +11,49 @@ extern PyObject* numbuf_deserialize_callback;
1111

1212
namespace numbuf {
1313

14-
PyObject* get_value(ArrayPtr arr, int32_t index, int32_t type, PyObject* base) {
15-
PyObject* result;
14+
Status get_value(ArrayPtr arr, int32_t index, int32_t type, PyObject* base, PyObject** result) {
1615
switch (arr->type()->type) {
1716
case Type::BOOL:
18-
return PyBool_FromLong(std::static_pointer_cast<BooleanArray>(arr)->Value(index));
17+
*result = PyBool_FromLong(std::static_pointer_cast<BooleanArray>(arr)->Value(index));
18+
return Status::OK();
1919
case Type::INT64:
20-
return PyInt_FromLong(std::static_pointer_cast<Int64Array>(arr)->Value(index));
20+
*result = PyInt_FromLong(std::static_pointer_cast<Int64Array>(arr)->Value(index));
21+
return Status::OK();
2122
case Type::BINARY: {
2223
int32_t nchars;
2324
const uint8_t* str = std::static_pointer_cast<BinaryArray>(arr)->GetValue(index, &nchars);
24-
return PyString_FromStringAndSize(reinterpret_cast<const char*>(str), nchars);
25+
*result = PyString_FromStringAndSize(reinterpret_cast<const char*>(str), nchars);
26+
return Status::OK();
2527
}
2628
case Type::STRING: {
2729
int32_t nchars;
2830
const uint8_t* str = std::static_pointer_cast<StringArray>(arr)->GetValue(index, &nchars);
29-
return PyUnicode_FromStringAndSize(reinterpret_cast<const char*>(str), nchars);
31+
*result = PyUnicode_FromStringAndSize(reinterpret_cast<const char*>(str), nchars);
32+
return Status::OK();
3033
}
3134
case Type::FLOAT:
32-
return PyFloat_FromDouble(std::static_pointer_cast<FloatArray>(arr)->Value(index));
35+
*result = PyFloat_FromDouble(std::static_pointer_cast<FloatArray>(arr)->Value(index));
36+
return Status::OK();
3337
case Type::DOUBLE:
34-
return PyFloat_FromDouble(std::static_pointer_cast<DoubleArray>(arr)->Value(index));
38+
*result = PyFloat_FromDouble(std::static_pointer_cast<DoubleArray>(arr)->Value(index));
39+
return Status::OK();
3540
case Type::STRUCT: {
3641
auto s = std::static_pointer_cast<StructArray>(arr);
3742
auto l = std::static_pointer_cast<ListArray>(s->field(0));
3843
if (s->type()->child(0)->name == "list") {
39-
ARROW_CHECK_OK(DeserializeList(l->values(), l->value_offset(index), l->value_offset(index+1), base, &result));
44+
return DeserializeList(l->values(), l->value_offset(index), l->value_offset(index+1), base, result);
4045
} else if (s->type()->child(0)->name == "tuple") {
41-
ARROW_CHECK_OK(DeserializeTuple(l->values(), l->value_offset(index), l->value_offset(index+1), base, &result));
46+
return DeserializeTuple(l->values(), l->value_offset(index), l->value_offset(index+1), base, result);
4247
} else if (s->type()->child(0)->name == "dict") {
43-
ARROW_CHECK_OK(DeserializeDict(l->values(), l->value_offset(index), l->value_offset(index+1), base, &result));
48+
return DeserializeDict(l->values(), l->value_offset(index), l->value_offset(index+1), base, result);
4449
} else {
45-
ARROW_CHECK_OK(DeserializeArray(arr, index, base, &result));
50+
return DeserializeArray(arr, index, base, result);
4651
}
47-
return result;
4852
}
4953
default:
5054
DCHECK(false) << "union tag not recognized " << type;
5155
}
52-
return NULL;
53-
}
54-
55-
Status python_error_to_status() {
56-
PyObject *type, *value, *traceback;
57-
PyErr_Fetch(&type, &value, &traceback);
58-
char *err_message = PyString_AsString(value);
59-
std::stringstream ss;
60-
if (err_message) {
61-
ss << "Python error in callback: " << err_message;
62-
}
63-
return Status::NotImplemented(ss.str());
56+
return Status::OK();
6457
}
6558

6659
Status append(PyObject* elem, SequenceBuilder& builder,
@@ -123,7 +116,7 @@ Status append(PyObject* elem, SequenceBuilder& builder,
123116
PyObject* result = PyObject_CallObject(numbuf_serialize_callback, arglist);
124117
if (!result) {
125118
Py_XDECREF(arglist);
126-
return python_error_to_status();
119+
return Status::NotImplemented("python error"); // TODO(pcm): https://github.com/pcmoritz/numbuf/issues/10
127120
}
128121
builder.AppendDict(PyDict_Size(result));
129122
subdicts.push_back(result);
@@ -181,7 +174,9 @@ Status SerializeSequences(std::vector<PyObject*> sequences, std::shared_ptr<Arra
181174
int32_t offset = offsets->Value(i); \
182175
int8_t type = types->Value(i); \
183176
ArrayPtr arr = data->child(type); \
184-
SET_ITEM(result, i-start_idx, get_value(arr, offset, type, base)); \
177+
PyObject* value; \
178+
RETURN_NOT_OK(get_value(arr, offset, type, base, &value)); \
179+
SET_ITEM(result, i-start_idx, value); \
185180
} \
186181
} \
187182
*out = result; \
@@ -245,7 +240,7 @@ Status DeserializeDict(std::shared_ptr<Array> array, int32_t start_idx, int32_t
245240
result = PyObject_CallObject(numbuf_deserialize_callback, arglist);
246241
if (!result) {
247242
Py_XDECREF(arglist);
248-
return python_error_to_status();
243+
return Status::NotImplemented("python error"); // TODO(pcm): https://github.com/pcmoritz/numbuf/issues/10
249244
}
250245
Py_XDECREF(arglist);
251246
}

‎python/src/pynumbuf/adapters/python.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ arrow::Status DeserializeList(std::shared_ptr<arrow::Array> array, int32_t start
1717
arrow::Status DeserializeTuple(std::shared_ptr<arrow::Array> array, int32_t start_idx, int32_t stop_idx, PyObject* base, PyObject** out);
1818
arrow::Status DeserializeDict(std::shared_ptr<arrow::Array> array, int32_t start_idx, int32_t stop_idx, PyObject* base, PyObject** out);
1919

20-
arrow::Status python_error_to_status();
21-
2220
}
2321

2422
#endif

‎python/src/pynumbuf/numbuf.cc‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,11 @@ static PyObject* serialize_list(PyObject* self, PyObject* args) {
5353
if (PyList_Check(value)) {
5454
Status s = SerializeSequences(std::vector<PyObject*>({value}), &array);
5555
if (!s.ok()) {
56-
PyErr_SetString(NumbufError, s.ToString().c_str());
56+
// If this condition is true, there was an error in the callback that
57+
// needs to be passed through
58+
if (!PyErr_Occurred()) {
59+
PyErr_SetString(NumbufError, s.ToString().c_str());
60+
}
5761
return NULL;
5862
}
5963

@@ -131,7 +135,15 @@ static PyObject* deserialize_list(PyObject* self, PyObject* args) {
131135
return NULL;
132136
}
133137
PyObject* result;
134-
ARROW_CHECK_OK(DeserializeList((*data)->column(0), 0, (*data)->num_rows(), base, &result));
138+
Status s = DeserializeList((*data)->column(0), 0, (*data)->num_rows(), base, &result);
139+
if (!s.ok()) {
140+
// If this condition is true, there was an error in the callback that
141+
// needs to be passed through
142+
if (!PyErr_Occurred()) {
143+
PyErr_SetString(NumbufError, s.ToString().c_str());
144+
}
145+
return NULL;
146+
}
135147
return result;
136148
}
137149

0 commit comments

Comments
 (0)