arrow-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From w...@apache.org
Subject arrow git commit: ARROW-1450: [Python] Raise proper error if custom serialization handler fails
Date Sun, 03 Sep 2017 19:11:07 GMT
Repository: arrow
Updated Branches:
  refs/heads/master ebca1af3b -> 016c45aa2


ARROW-1450: [Python] Raise proper error if custom serialization handler fails

This fixes the serialize and deserialize methods to throw the right exceptions.

Author: Philipp Moritz <pcmoritz@gmail.com>

Closes #1029 from pcmoritz/pyarrow-serialization-error and squashes the following commits:

249b38ed [Philipp Moritz] formatting
2d872a4c [Philipp Moritz] fixes
caad76d1 [Philipp Moritz] add test for deserialization (needs fixing)
57772124 [Philipp Moritz] fix python serialization error propagation


Project: http://git-wip-us.apache.org/repos/asf/arrow/repo
Commit: http://git-wip-us.apache.org/repos/asf/arrow/commit/016c45aa
Tree: http://git-wip-us.apache.org/repos/asf/arrow/tree/016c45aa
Diff: http://git-wip-us.apache.org/repos/asf/arrow/diff/016c45aa

Branch: refs/heads/master
Commit: 016c45aa2031eebe118763b8465cf19a45781313
Parents: ebca1af
Author: Philipp Moritz <pcmoritz@gmail.com>
Authored: Sun Sep 3 15:11:00 2017 -0400
Committer: Wes McKinney <wes.mckinney@twosigma.com>
Committed: Sun Sep 3 15:11:00 2017 -0400

----------------------------------------------------------------------
 cpp/src/arrow/python/common.cc             |  9 +++++++++
 cpp/src/arrow/python/common.h              |  2 ++
 cpp/src/arrow/python/python_to_arrow.cc    |  2 +-
 cpp/src/arrow/status.h                     |  3 +++
 python/pyarrow/error.pxi                   |  3 +++
 python/pyarrow/includes/common.pxd         |  1 +
 python/pyarrow/serialization.pxi           |  4 +++-
 python/pyarrow/tests/test_serialization.py | 23 +++++++++++++++++++++++
 8 files changed, 45 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/arrow/blob/016c45aa/cpp/src/arrow/python/common.cc
----------------------------------------------------------------------
diff --git a/cpp/src/arrow/python/common.cc b/cpp/src/arrow/python/common.cc
index a248db3..53bd57b 100644
--- a/cpp/src/arrow/python/common.cc
+++ b/cpp/src/arrow/python/common.cc
@@ -81,5 +81,14 @@ Status CheckPyError(StatusCode code) {
   return Status::OK();
 }
 
+Status PassPyError() {
+  if (PyErr_Occurred()) {
+    // Do not call PyErr_Clear, the assumption is that someone further
+    // up the call stack will want to deal with the Python error.
+    return Status(StatusCode::PythonError, "");
+  }
+  return Status::OK();
+}
+
 }  // namespace py
 }  // namespace arrow

http://git-wip-us.apache.org/repos/asf/arrow/blob/016c45aa/cpp/src/arrow/python/common.h
----------------------------------------------------------------------
diff --git a/cpp/src/arrow/python/common.h b/cpp/src/arrow/python/common.h
index e315771..e3fe2ef 100644
--- a/cpp/src/arrow/python/common.h
+++ b/cpp/src/arrow/python/common.h
@@ -145,6 +145,8 @@ struct ARROW_EXPORT PyObjectStringify {
 
 Status CheckPyError(StatusCode code = StatusCode::UnknownError);
 
+Status PassPyError();
+
 // TODO(wesm): We can just let errors pass through. To be explored later
 #define RETURN_IF_PYERROR() RETURN_NOT_OK(CheckPyError());
 

http://git-wip-us.apache.org/repos/asf/arrow/blob/016c45aa/cpp/src/arrow/python/python_to_arrow.cc
----------------------------------------------------------------------
diff --git a/cpp/src/arrow/python/python_to_arrow.cc b/cpp/src/arrow/python/python_to_arrow.cc
index c5cfd6e..d823bad 100644
--- a/cpp/src/arrow/python/python_to_arrow.cc
+++ b/cpp/src/arrow/python/python_to_arrow.cc
@@ -337,7 +337,7 @@ Status CallCustomCallback(PyObject* context, PyObject* method_name, PyObject*
el
     return Status::SerializationError(ss.str());
   } else {
     *result = PyObject_CallMethodObjArgs(context, method_name, elem, NULL);
-    RETURN_IF_PYERROR();
+    return PassPyError();
   }
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/arrow/blob/016c45aa/cpp/src/arrow/status.h
----------------------------------------------------------------------
diff --git a/cpp/src/arrow/status.h b/cpp/src/arrow/status.h
index 7a39d0f..ece83ac 100644
--- a/cpp/src/arrow/status.h
+++ b/cpp/src/arrow/status.h
@@ -72,6 +72,7 @@ enum class StatusCode : char {
   UnknownError = 9,
   NotImplemented = 10,
   SerializationError = 11,
+  PythonError = 12,
   PlasmaObjectExists = 20,
   PlasmaObjectNonexistent = 21,
   PlasmaStoreFull = 22
@@ -154,6 +155,8 @@ class ARROW_EXPORT Status {
   bool IsNotImplemented() const { return code() == StatusCode::NotImplemented; }
   // An object could not be serialized or deserialized.
   bool IsSerializationError() const { return code() == StatusCode::SerializationError; }
+  // An error is propagated from a nested Python function.
+  bool IsPythonError() const { return code() == StatusCode::PythonError; }
   // An object with this object ID already exists in the plasma store.
   bool IsPlasmaObjectExists() const { return code() == StatusCode::PlasmaObjectExists; }
   // An object was requested that doesn't exist in the plasma store.

http://git-wip-us.apache.org/repos/asf/arrow/blob/016c45aa/python/pyarrow/error.pxi
----------------------------------------------------------------------
diff --git a/python/pyarrow/error.pxi b/python/pyarrow/error.pxi
index 2a21302..dfdfcd7 100644
--- a/python/pyarrow/error.pxi
+++ b/python/pyarrow/error.pxi
@@ -68,6 +68,9 @@ cdef int check_status(const CStatus& status) nogil except -1:
     if status.ok():
         return 0
 
+    if status.IsPythonError():
+        return -1
+
     with gil:
         message = frombytes(status.message())
         if status.IsInvalid():

http://git-wip-us.apache.org/repos/asf/arrow/blob/016c45aa/python/pyarrow/includes/common.pxd
----------------------------------------------------------------------
diff --git a/python/pyarrow/includes/common.pxd b/python/pyarrow/includes/common.pxd
index 1bd840c..f323fea 100644
--- a/python/pyarrow/includes/common.pxd
+++ b/python/pyarrow/includes/common.pxd
@@ -52,6 +52,7 @@ cdef extern from "arrow/api.h" namespace "arrow" nogil:
         c_bool IsNotImplemented()
         c_bool IsTypeError()
         c_bool IsSerializationError()
+        c_bool IsPythonError()
         c_bool IsPlasmaObjectExists()
         c_bool IsPlasmaObjectNonexistent()
         c_bool IsPlasmaStoreFull()

http://git-wip-us.apache.org/repos/asf/arrow/blob/016c45aa/python/pyarrow/serialization.pxi
----------------------------------------------------------------------
diff --git a/python/pyarrow/serialization.pxi b/python/pyarrow/serialization.pxi
index f38845e..aa1a6a4 100644
--- a/python/pyarrow/serialization.pxi
+++ b/python/pyarrow/serialization.pxi
@@ -118,7 +118,9 @@ cdef class SerializationContext:
         else:
             assert type_id not in self.types_to_pickle
             if type_id not in self.whitelisted_types:
-                raise "error"
+                msg = "Type ID " + str(type_id) + " not registered in " \
+                      "deserialization callback"
+                raise DeserializationCallbackError(msg, type_id)
             type_ = self.whitelisted_types[type_id]
             if type_id in self.custom_deserializers:
                 obj = self.custom_deserializers[type_id](

http://git-wip-us.apache.org/repos/asf/arrow/blob/016c45aa/python/pyarrow/tests/test_serialization.py
----------------------------------------------------------------------
diff --git a/python/pyarrow/tests/test_serialization.py b/python/pyarrow/tests/test_serialization.py
index 4e98bd5..5526ac6 100644
--- a/python/pyarrow/tests/test_serialization.py
+++ b/python/pyarrow/tests/test_serialization.py
@@ -255,3 +255,26 @@ def test_custom_serialization(large_memory_map):
     with pa.memory_map(large_memory_map, mode="r+") as mmap:
         for obj in CUSTOM_OBJECTS:
             serialization_roundtrip(obj, mmap)
+
+def test_serialization_callback_error():
+
+    class TempClass(object):
+            pass
+
+    # Pass a SerializationContext into serialize, but TempClass
+    # is not registered
+    serialization_context = pa.SerializationContext()
+    val = TempClass()
+    with pytest.raises(pa.SerializationCallbackError) as err:
+        serialized_object = pa.serialize(val, serialization_context)
+    assert err.value.example_object == val
+
+    serialization_context.register_type(TempClass, 20*b"\x00")
+    serialized_object = pa.serialize(TempClass(), serialization_context)
+    deserialization_context = pa.SerializationContext()
+    
+    # Pass a Serialization Context into deserialize, but TempClass
+    # is not registered
+    with pytest.raises(pa.DeserializationCallbackError) as err:
+        serialized_object.deserialize(deserialization_context)
+    assert err.value.type_id == 20*b"\x00"


Mime
View raw message