From a428806788cb8de345cc0c279dbcb59cf9e92227 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Sun, 4 Sep 2016 13:08:34 -0700 Subject: [PATCH] propagate callback error --- python/src/pynumbuf/adapters/numpy.cc | 2 +- python/src/pynumbuf/adapters/python.cc | 51 ++++++++++++-------------- python/src/pynumbuf/adapters/python.h | 2 - python/src/pynumbuf/numbuf.cc | 16 +++++++- 4 files changed, 38 insertions(+), 33 deletions(-) diff --git a/python/src/pynumbuf/adapters/numpy.cc b/python/src/pynumbuf/adapters/numpy.cc index 92319f181..e3e1fa366 100644 --- a/python/src/pynumbuf/adapters/numpy.cc +++ b/python/src/pynumbuf/adapters/numpy.cc @@ -117,7 +117,7 @@ Status SerializeArray(PyArrayObject* array, SequenceBuilder& builder, PyObject* result = PyObject_CallObject(numbuf_serialize_callback, arglist); if (!result) { Py_XDECREF(arglist); - return python_error_to_status(); + return Status::NotImplemented("python error"); // TODO(pcm): https://github.com/pcmoritz/numbuf/issues/10 } builder.AppendDict(PyDict_Size(result)); subdicts.push_back(result); diff --git a/python/src/pynumbuf/adapters/python.cc b/python/src/pynumbuf/adapters/python.cc index 173767a1a..5b71865ff 100644 --- a/python/src/pynumbuf/adapters/python.cc +++ b/python/src/pynumbuf/adapters/python.cc @@ -11,56 +11,49 @@ extern PyObject* numbuf_deserialize_callback; namespace numbuf { -PyObject* get_value(ArrayPtr arr, int32_t index, int32_t type, PyObject* base) { - PyObject* result; +Status get_value(ArrayPtr arr, int32_t index, int32_t type, PyObject* base, PyObject** result) { switch (arr->type()->type) { case Type::BOOL: - return PyBool_FromLong(std::static_pointer_cast(arr)->Value(index)); + *result = PyBool_FromLong(std::static_pointer_cast(arr)->Value(index)); + return Status::OK(); case Type::INT64: - return PyInt_FromLong(std::static_pointer_cast(arr)->Value(index)); + *result = PyInt_FromLong(std::static_pointer_cast(arr)->Value(index)); + return Status::OK(); case Type::BINARY: { int32_t nchars; const uint8_t* str = std::static_pointer_cast(arr)->GetValue(index, &nchars); - return PyString_FromStringAndSize(reinterpret_cast(str), nchars); + *result = PyString_FromStringAndSize(reinterpret_cast(str), nchars); + return Status::OK(); } case Type::STRING: { int32_t nchars; const uint8_t* str = std::static_pointer_cast(arr)->GetValue(index, &nchars); - return PyUnicode_FromStringAndSize(reinterpret_cast(str), nchars); + *result = PyUnicode_FromStringAndSize(reinterpret_cast(str), nchars); + return Status::OK(); } case Type::FLOAT: - return PyFloat_FromDouble(std::static_pointer_cast(arr)->Value(index)); + *result = PyFloat_FromDouble(std::static_pointer_cast(arr)->Value(index)); + return Status::OK(); case Type::DOUBLE: - return PyFloat_FromDouble(std::static_pointer_cast(arr)->Value(index)); + *result = PyFloat_FromDouble(std::static_pointer_cast(arr)->Value(index)); + return Status::OK(); case Type::STRUCT: { auto s = std::static_pointer_cast(arr); auto l = std::static_pointer_cast(s->field(0)); if (s->type()->child(0)->name == "list") { - ARROW_CHECK_OK(DeserializeList(l->values(), l->value_offset(index), l->value_offset(index+1), base, &result)); + return DeserializeList(l->values(), l->value_offset(index), l->value_offset(index+1), base, result); } else if (s->type()->child(0)->name == "tuple") { - ARROW_CHECK_OK(DeserializeTuple(l->values(), l->value_offset(index), l->value_offset(index+1), base, &result)); + return DeserializeTuple(l->values(), l->value_offset(index), l->value_offset(index+1), base, result); } else if (s->type()->child(0)->name == "dict") { - ARROW_CHECK_OK(DeserializeDict(l->values(), l->value_offset(index), l->value_offset(index+1), base, &result)); + return DeserializeDict(l->values(), l->value_offset(index), l->value_offset(index+1), base, result); } else { - ARROW_CHECK_OK(DeserializeArray(arr, index, base, &result)); + return DeserializeArray(arr, index, base, result); } - return result; } default: DCHECK(false) << "union tag not recognized " << type; } - return NULL; -} - -Status python_error_to_status() { - PyObject *type, *value, *traceback; - PyErr_Fetch(&type, &value, &traceback); - char *err_message = PyString_AsString(value); - std::stringstream ss; - if (err_message) { - ss << "Python error in callback: " << err_message; - } - return Status::NotImplemented(ss.str()); + return Status::OK(); } Status append(PyObject* elem, SequenceBuilder& builder, @@ -123,7 +116,7 @@ Status append(PyObject* elem, SequenceBuilder& builder, PyObject* result = PyObject_CallObject(numbuf_serialize_callback, arglist); if (!result) { Py_XDECREF(arglist); - return python_error_to_status(); + return Status::NotImplemented("python error"); // TODO(pcm): https://github.com/pcmoritz/numbuf/issues/10 } builder.AppendDict(PyDict_Size(result)); subdicts.push_back(result); @@ -181,7 +174,9 @@ Status SerializeSequences(std::vector sequences, std::shared_ptrValue(i); \ int8_t type = types->Value(i); \ ArrayPtr arr = data->child(type); \ - SET_ITEM(result, i-start_idx, get_value(arr, offset, type, base)); \ + PyObject* value; \ + RETURN_NOT_OK(get_value(arr, offset, type, base, &value)); \ + SET_ITEM(result, i-start_idx, value); \ } \ } \ *out = result; \ @@ -245,7 +240,7 @@ Status DeserializeDict(std::shared_ptr array, int32_t start_idx, int32_t result = PyObject_CallObject(numbuf_deserialize_callback, arglist); if (!result) { Py_XDECREF(arglist); - return python_error_to_status(); + return Status::NotImplemented("python error"); // TODO(pcm): https://github.com/pcmoritz/numbuf/issues/10 } Py_XDECREF(arglist); } diff --git a/python/src/pynumbuf/adapters/python.h b/python/src/pynumbuf/adapters/python.h index 13ee629d3..1f05e923f 100644 --- a/python/src/pynumbuf/adapters/python.h +++ b/python/src/pynumbuf/adapters/python.h @@ -17,8 +17,6 @@ arrow::Status DeserializeList(std::shared_ptr array, int32_t start arrow::Status DeserializeTuple(std::shared_ptr array, int32_t start_idx, int32_t stop_idx, PyObject* base, PyObject** out); arrow::Status DeserializeDict(std::shared_ptr array, int32_t start_idx, int32_t stop_idx, PyObject* base, PyObject** out); -arrow::Status python_error_to_status(); - } #endif diff --git a/python/src/pynumbuf/numbuf.cc b/python/src/pynumbuf/numbuf.cc index d89913d6e..de8722a48 100644 --- a/python/src/pynumbuf/numbuf.cc +++ b/python/src/pynumbuf/numbuf.cc @@ -53,7 +53,11 @@ static PyObject* serialize_list(PyObject* self, PyObject* args) { if (PyList_Check(value)) { Status s = SerializeSequences(std::vector({value}), &array); if (!s.ok()) { - PyErr_SetString(NumbufError, s.ToString().c_str()); + // If this condition is true, there was an error in the callback that + // needs to be passed through + if (!PyErr_Occurred()) { + PyErr_SetString(NumbufError, s.ToString().c_str()); + } return NULL; } @@ -131,7 +135,15 @@ static PyObject* deserialize_list(PyObject* self, PyObject* args) { return NULL; } PyObject* result; - ARROW_CHECK_OK(DeserializeList((*data)->column(0), 0, (*data)->num_rows(), base, &result)); + Status s = DeserializeList((*data)->column(0), 0, (*data)->num_rows(), base, &result); + if (!s.ok()) { + // If this condition is true, there was an error in the callback that + // needs to be passed through + if (!PyErr_Occurred()) { + PyErr_SetString(NumbufError, s.ToString().c_str()); + } + return NULL; + } return result; }