ARROW-2380: [Python] Streamline conversions

* Correctly check for overflow when constructing string / binary objects
* Accept bytearrays in the same places as bytes objects
* Other small refactoring

cpp/src/arrow/python/builtin_convert.cc

@@ -42,26 +42,9 @@ namespace py {
 
 Status InvalidConversion(PyObject* obj, const std::string& expected_types,
                          std::ostream* out) {
-  OwnedRef type(PyObject_Type(obj));
-  RETURN_IF_PYERROR();
-  DCHECK_NE(type.obj(), nullptr);
-
-  OwnedRef type_name(PyObject_GetAttrString(type.obj(), "__name__"));
-  RETURN_IF_PYERROR();
-  DCHECK_NE(type_name.obj(), nullptr);
-
-  PyObjectStringify bytestring(type_name.obj());
-  RETURN_IF_PYERROR();
-
-  const char* bytes = bytestring.bytes;
-  DCHECK_NE(bytes, nullptr) << "bytes from type(...).__name__ were null";
-
-  Py_ssize_t size = bytestring.size;
-
-  std::string cpp_type_name(bytes, size);
-
-  (*out) << "Got Python object of type " << cpp_type_name
+  (*out) << "Got Python object of type " << Py_TYPE(obj)->tp_name
          << " but can only handle these types: " << expected_types;
+  // XXX streamline this?
   return Status::OK();
 }
 
@@ -100,7 +83,7 @@ class ScalarVisitor {
       ++date_count_;
     } else if (PyDateTime_CheckExact(obj)) {
       ++timestamp_count_;
-    } else if (PyBytes_Check(obj)) {
+    } else if (internal::IsPyBinary(obj)) {
       ++binary_count_;
     } else if (PyUnicode_Check(obj)) {
       ++unicode_count_;
@@ -449,16 +432,9 @@ class Int8Converter : public TypedConverterVisitor<Int8Builder, Int8Converter> {
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    const auto val = static_cast<int64_t>(PyLong_AsLongLong(obj));
-
-    if (ARROW_PREDICT_FALSE(val > std::numeric_limits<int8_t>::max() ||
-                            val < std::numeric_limits<int8_t>::min())) {
-      return Status::Invalid(
-          "Cannot coerce values to array type that would "
-          "lose data");
-    }
-    RETURN_IF_PYERROR();
-    return typed_builder_->Append(static_cast<int8_t>(val));
+    int8_t value;
+    RETURN_NOT_OK(internal::Int8FromPythonInt(obj, &value));
+    return typed_builder_->Append(value);
   }
 };
 
@@ -466,16 +442,9 @@ class Int16Converter : public TypedConverterVisitor<Int16Builder, Int16Converter
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    const auto val = static_cast<int64_t>(PyLong_AsLongLong(obj));
-
-    if (ARROW_PREDICT_FALSE(val > std::numeric_limits<int16_t>::max() ||
-                            val < std::numeric_limits<int16_t>::min())) {
-      return Status::Invalid(
-          "Cannot coerce values to array type that would "
-          "lose data");
-    }
-    RETURN_IF_PYERROR();
-    return typed_builder_->Append(static_cast<int16_t>(val));
+    int16_t value;
+    RETURN_NOT_OK(internal::Int16FromPythonInt(obj, &value));
+    return typed_builder_->Append(value);
   }
 };
 
@@ -483,16 +452,9 @@ class Int32Converter : public TypedConverterVisitor<Int32Builder, Int32Converter
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    const auto val = static_cast<int64_t>(PyLong_AsLongLong(obj));
-
-    if (ARROW_PREDICT_FALSE(val > std::numeric_limits<int32_t>::max() ||
-                            val < std::numeric_limits<int32_t>::min())) {
-      return Status::Invalid(
-          "Cannot coerce values to array type that would "
-          "lose data");
-    }
-    RETURN_IF_PYERROR();
-    return typed_builder_->Append(static_cast<int32_t>(val));
+    int32_t value;
+    RETURN_NOT_OK(internal::Int32FromPythonInt(obj, &value));
+    return typed_builder_->Append(value);
   }
 };
 
@@ -500,9 +462,9 @@ class Int64Converter : public TypedConverterVisitor<Int64Builder, Int64Converter
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    const auto val = static_cast<int64_t>(PyLong_AsLongLong(obj));
-    RETURN_IF_PYERROR();
-    return typed_builder_->Append(val);
+    int64_t value;
+    RETURN_NOT_OK(internal::Int64FromPythonInt(obj, &value));
+    return typed_builder_->Append(value);
   }
 };
 
@@ -510,15 +472,9 @@ class UInt8Converter : public TypedConverterVisitor<UInt8Builder, UInt8Converter
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    const auto val = static_cast<uint64_t>(PyLong_AsLongLong(obj));
-    RETURN_IF_PYERROR();
-
-    if (ARROW_PREDICT_FALSE(val > std::numeric_limits<uint8_t>::max())) {
-      return Status::Invalid(
-          "Cannot coerce values to array type that would "
-          "lose data");
-    }
-    return typed_builder_->Append(static_cast<uint8_t>(val));
+    uint8_t value;
+    RETURN_NOT_OK(internal::UInt8FromPythonInt(obj, &value));
+    return typed_builder_->Append(value);
   }
 };
 
@@ -526,15 +482,9 @@ class UInt16Converter : public TypedConverterVisitor<UInt16Builder, UInt16Conver
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    const auto val = static_cast<uint64_t>(PyLong_AsLongLong(obj));
-    RETURN_IF_PYERROR();
-
-    if (ARROW_PREDICT_FALSE(val > std::numeric_limits<uint16_t>::max())) {
-      return Status::Invalid(
-          "Cannot coerce values to array type that would "
-          "lose data");
-    }
-    return typed_builder_->Append(static_cast<uint16_t>(val));
+    uint16_t value;
+    RETURN_NOT_OK(internal::UInt16FromPythonInt(obj, &value));
+    return typed_builder_->Append(value);
   }
 };
 
@@ -542,15 +492,9 @@ class UInt32Converter : public TypedConverterVisitor<UInt32Builder, UInt32Conver
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    const auto val = static_cast<uint64_t>(PyLong_AsLongLong(obj));
-    RETURN_IF_PYERROR();
-
-    if (ARROW_PREDICT_FALSE(val > std::numeric_limits<uint32_t>::max())) {
-      return Status::Invalid(
-          "Cannot coerce values to array type that would "
-          "lose data");
-    }
-    return typed_builder_->Append(static_cast<uint32_t>(val));
+    uint32_t value;
+    RETURN_NOT_OK(internal::UInt32FromPythonInt(obj, &value));
+    return typed_builder_->Append(value);
   }
 };
 
@@ -690,27 +634,7 @@ class BytesConverter : public TypedConverterVisitor<BinaryBuilder, BytesConverte
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    PyObject* bytes_obj;
-    const char* bytes;
-    Py_ssize_t length;
-    OwnedRef tmp;
-
-    if (PyUnicode_Check(obj)) {
-      tmp.reset(PyUnicode_AsUTF8String(obj));
-      RETURN_IF_PYERROR();
-      bytes_obj = tmp.obj();
-    } else if (PyBytes_Check(obj)) {
-      bytes_obj = obj;
-    } else {
-      std::stringstream ss;
-      ss << "Error converting to Binary type: ";
-      RETURN_NOT_OK(InvalidConversion(obj, "bytes", &ss));
-      return Status::Invalid(ss.str());
-    }
-    // No error checking
-    length = PyBytes_GET_SIZE(bytes_obj);
-    bytes = PyBytes_AS_STRING(bytes_obj);
-    return typed_builder_->Append(bytes, static_cast<int32_t>(length));
+    return internal::BuilderAppend(typed_builder_, obj);
   }
 };
 
@@ -719,27 +643,7 @@ class FixedWidthBytesConverter
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    PyObject* bytes_obj;
-    OwnedRef tmp;
-    Py_ssize_t expected_length =
-        std::dynamic_pointer_cast<FixedSizeBinaryType>(typed_builder_->type())
-            ->byte_width();
-    if (PyUnicode_Check(obj)) {
-      tmp.reset(PyUnicode_AsUTF8String(obj));
-      RETURN_IF_PYERROR();
-      bytes_obj = tmp.obj();
-    } else if (PyBytes_Check(obj)) {
-      bytes_obj = obj;
-    } else {
-      std::stringstream ss;
-      ss << "Error converting to FixedSizeBinary type: ";
-      RETURN_NOT_OK(InvalidConversion(obj, "bytes", &ss));
-      return Status::Invalid(ss.str());
-    }
-    // No error checking
-    RETURN_NOT_OK(CheckPythonBytesAreFixedLength(bytes_obj, expected_length));
-    return typed_builder_->Append(
-        reinterpret_cast<const uint8_t*>(PyBytes_AS_STRING(bytes_obj)));
+    return internal::BuilderAppend(typed_builder_, obj);
   }
 };
 
@@ -747,32 +651,7 @@ class UTF8Converter : public TypedConverterVisitor<StringBuilder, UTF8Converter>
  public:
   // Append a non-missing item
   Status AppendItem(PyObject* obj) {
-    PyObject* bytes_obj;
-    OwnedRef tmp;
-    const char* bytes;
-    Py_ssize_t length;
-
-    if (PyBytes_Check(obj)) {
-      tmp.reset(
-          PyUnicode_FromStringAndSize(PyBytes_AS_STRING(obj), PyBytes_GET_SIZE(obj)));
-      RETURN_IF_PYERROR();
-      bytes_obj = obj;
-    } else if (!PyUnicode_Check(obj)) {
-      OwnedRef repr(PyObject_Repr(obj));
-      PyObjectStringify stringified(repr.obj());
-      std::stringstream ss;
-      ss << "Non bytes/unicode value encountered: " << stringified.bytes;
-      return Status::Invalid(ss.str());
-    } else {
-      tmp.reset(PyUnicode_AsUTF8String(obj));
-      RETURN_IF_PYERROR();
-      bytes_obj = tmp.obj();
-    }
-
-    // No error checking
-    length = PyBytes_GET_SIZE(bytes_obj);
-    bytes = PyBytes_AS_STRING(bytes_obj);
-    return typed_builder_->Append(bytes, static_cast<int32_t>(length));
+    return internal::BuilderAppend(typed_builder_, obj, true /* check_valid */);
   }
 };
 
@@ -1032,16 +911,5 @@ Status ConvertPySequence(PyObject* obj, int64_t size,
   return ConvertPySequenceReal(obj, size, &type, pool, out);
 }
 
-Status CheckPythonBytesAreFixedLength(PyObject* obj, Py_ssize_t expected_length) {
-  const Py_ssize_t length = PyBytes_GET_SIZE(obj);
-  if (length != expected_length) {
-    std::stringstream ss;
-    ss << "Found byte string of length " << length << ", expected length is "
-       << expected_length;
-    return Status::Invalid(ss.str());
-  }
-  return Status::OK();
-}
-
 }  // namespace py
 }  // namespace arrow

cpp/src/arrow/python/builtin_convert.h

@@ -73,9 +73,6 @@ ARROW_EXPORT
 Status InvalidConversion(PyObject* obj, const std::string& expected_type_name,
                          std::ostream* out);
 
-ARROW_EXPORT Status CheckPythonBytesAreFixedLength(PyObject* obj,
-                                                   Py_ssize_t expected_length);
-
 }  // namespace py
 }  // namespace arrow
 

cpp/src/arrow/python/common.cc

@@ -25,6 +25,8 @@
 #include "arrow/status.h"
 #include "arrow/util/logging.h"
 
+#include "arrow/python/helpers.h"
+
 namespace arrow {
 namespace py {
 
@@ -87,19 +89,15 @@ Status CheckPyError(StatusCode code) {
     PyObject* exc_value = nullptr;
     PyObject* traceback = nullptr;
 
+    PyErr_Fetch(&exc_type, &exc_value, &traceback);
+    PyErr_NormalizeException(&exc_type, &exc_value, &traceback);
+
     OwnedRef exc_type_ref(exc_type);
     OwnedRef exc_value_ref(exc_value);
     OwnedRef traceback_ref(traceback);
 
-    PyErr_Fetch(&exc_type, &exc_value, &traceback);
-
-    PyErr_NormalizeException(&exc_type, &exc_value, &traceback);
-
-    OwnedRef exc_value_str(PyObject_Str(exc_value));
-    PyObjectStringify stringified(exc_value_str.obj());
-    std::string message(stringified.bytes);
-
-    PyErr_Clear();
+    std::string message;
+    RETURN_NOT_OK(internal::PyObject_StdStringStr(exc_value, &message));
     return Status(code, message);
   }
   return Status::OK();

cpp/src/arrow/python/common.h

@@ -19,7 +19,9 @@
 #define ARROW_PYTHON_COMMON_H
 
 #include <memory>
+#include <sstream>
 #include <string>
+#include <utility>
 
 #include "arrow/python/config.h"
 
@@ -33,6 +35,15 @@ class MemoryPool;
 
 namespace py {
 
+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());
+
+#define PY_RETURN_IF_ERROR(CODE) RETURN_NOT_OK(CheckPyError(CODE));
+
 class ARROW_EXPORT PyAcquireGIL {
  public:
   PyAcquireGIL() : acquired_gil_(false) { acquire(); }
@@ -70,6 +81,11 @@ class ARROW_EXPORT OwnedRef {
   OwnedRef(OwnedRef&& other) : OwnedRef(other.detach()) {}
   explicit OwnedRef(PyObject* obj) : obj_(obj) {}
 
+  OwnedRef& operator=(OwnedRef&& other) {
+    obj_ = other.detach();
+    return *this;
+  }
+
   ~OwnedRef() { reset(); }
 
   void reset(PyObject* obj) {
@@ -89,6 +105,8 @@ class ARROW_EXPORT OwnedRef {
 
   PyObject** ref() { return &obj_; }
 
+  operator bool() const { return obj_ != NULLPTR; }
+
  private:
   ARROW_DISALLOW_COPY_AND_ASSIGN(OwnedRef);
 
@@ -110,37 +128,72 @@ class ARROW_EXPORT OwnedRefNoGIL : public OwnedRef {
   }
 };
 
-struct ARROW_EXPORT PyObjectStringify {
-  OwnedRef tmp_obj;
+// A temporary conversion of a Python object to a bytes area.
+struct ARROW_EXPORT PyBytesView {
   const char* bytes;
   Py_ssize_t size;
 
-  explicit PyObjectStringify(PyObject* obj) {
-    PyObject* bytes_obj;
+  PyBytesView() : bytes(nullptr), size(0), ref(nullptr) {}
+
+  // View the given Python object as binary-like, i.e. bytes
+  static Status FromBinary(PyObject* obj, PyBytesView* out) {
+    return FromBinary(obj, out, "a bytes object");
+  }
+
+  // View the given Python object as string-like, i.e. str or (utf8) bytes
+  // XXX make this non-static?
+  static Status FromString(PyObject* obj, PyBytesView* out, bool check_valid = false) {
     if (PyUnicode_Check(obj)) {
-      bytes_obj = PyUnicode_AsUTF8String(obj);
-      tmp_obj.reset(bytes_obj);
-      bytes = PyBytes_AsString(bytes_obj);
-      size = PyBytes_GET_SIZE(bytes_obj);
-    } else if (PyBytes_Check(obj)) {
-      bytes = PyBytes_AsString(obj);
-      size = PyBytes_GET_SIZE(obj);
+#if PY_MAJOR_VERSION >= 3
+      Py_ssize_t size;
+      // The utf-8 representation is cached on the unicode object
+      const char* data = PyUnicode_AsUTF8AndSize(obj, &size);
+      RETURN_IF_PYERROR();
+      *out = PyBytesView(data, size);
+      return Status::OK();
+#else
+      PyObject* converted = PyUnicode_AsUTF8String(obj);
+      RETURN_IF_PYERROR();
+      *out = PyBytesView(PyBytes_AS_STRING(converted), PyBytes_GET_SIZE(converted),
+                         converted);
+      return Status::OK();
+#endif
     } else {
-      bytes = NULLPTR;
-      size = -1;
+      PyBytesView view;
+      RETURN_NOT_OK(FromBinary(obj, &view, "a string or bytes object"));
+      if (check_valid) {
+        // Check the bytes are valid utf-8
+        OwnedRef decoded(PyUnicode_FromStringAndSize(view.bytes, view.size));
+        RETURN_IF_PYERROR();
+      }
+      *out = std::move(view);
+      return Status::OK();
     }
   }
+
+ protected:
+  PyBytesView(const char* b, Py_ssize_t s, PyObject* obj = nullptr)
+      : bytes(b), size(s), ref(obj) {}
+
+  static Status FromBinary(PyObject* obj, PyBytesView* out,
+                           const std::string& expected_msg) {
+    if (PyBytes_Check(obj)) {
+      *out = PyBytesView(PyBytes_AS_STRING(obj), PyBytes_GET_SIZE(obj));
+      return Status::OK();
+    } else if (PyByteArray_Check(obj)) {
+      *out = PyBytesView(PyByteArray_AS_STRING(obj), PyByteArray_GET_SIZE(obj));
+      return Status::OK();
+    } else {
+      std::stringstream ss;
+      ss << "Expected " << expected_msg << ", got a '" << Py_TYPE(obj)->tp_name
+         << "' object";
+      return Status::TypeError(ss.str());
+    }
+  }
+
+  OwnedRef ref;
 };
 
-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());
-
-#define PY_RETURN_IF_ERROR(CODE) RETURN_NOT_OK(CheckPyError(CODE));
-
 // Return the common PyArrow memory pool
 ARROW_EXPORT void set_default_memory_pool(MemoryPool* pool);
 ARROW_EXPORT MemoryPool* get_memory_pool();

cpp/src/arrow/python/helpers.cc

@@ -78,6 +78,50 @@ Status PyFloat_AsHalf(PyObject* obj, npy_half* out) {
 
 namespace internal {
 
+std::string PyBytes_AsStdString(PyObject* obj) {
+  DCHECK(PyBytes_Check(obj));
+  return std::string(PyBytes_AS_STRING(obj), PyBytes_GET_SIZE(obj));
+}
+
+Status PyUnicode_AsStdString(PyObject* obj, std::string* out) {
+  DCHECK(PyUnicode_Check(obj));
+#if PY_MAJOR_VERSION >= 3
+  Py_ssize_t size;
+  // The utf-8 representation is cached on the unicode object
+  const char* data = PyUnicode_AsUTF8AndSize(obj, &size);
+  RETURN_IF_PYERROR();
+  *out = std::string(data, size);
+  return Status::OK();
+#else
+  OwnedRef bytes_ref(PyUnicode_AsUTF8String(obj));
+  RETURN_IF_PYERROR();
+  *out = PyBytes_AsStdString(bytes_ref.obj());
+  return Status::OK();
+#endif
+}
+
+std::string PyObject_StdStringRepr(PyObject* obj) {
+  OwnedRef unicode_ref(PyObject_Repr(obj));
+  OwnedRef bytes_ref;
+
+  if (unicode_ref) {
+    bytes_ref.reset(
+        PyUnicode_AsEncodedString(unicode_ref.obj(), "utf8", "backslashreplace"));
+  }
+  if (!bytes_ref) {
+    PyErr_Clear();
+    std::stringstream ss;
+    ss << "<object of type '" << Py_TYPE(obj)->tp_name << "' repr() failed>";
+    return ss.str();
+  }
+  return PyBytes_AsStdString(bytes_ref.obj());
+}
+
+Status PyObject_StdStringStr(PyObject* obj, std::string* out) {
+  OwnedRef unicode_ref(PyObject_Str(obj));
+  return PyUnicode_AsStdString(unicode_ref.obj(), out);
+}
+
 Status ImportModule(const std::string& module_name, OwnedRef* ref) {
   PyObject* module = PyImport_ImportModule(module_name.c_str());
   RETURN_IF_PYERROR();
@@ -97,6 +141,195 @@ Status ImportFromModule(const OwnedRef& module, const std::string& name, OwnedRe
   return Status::OK();
 }
 
+Status BuilderAppend(BinaryBuilder* builder, PyObject* obj, bool* is_full) {
+  PyBytesView view;
+  // XXX For some reason, we must accept unicode objects here
+  RETURN_NOT_OK(PyBytesView::FromString(obj, &view));
+  int32_t length;
+  RETURN_NOT_OK(CastSize(view.size, &length));
+  // Did we reach the builder size limit?
+  if (ARROW_PREDICT_FALSE(builder->value_data_length() + length > kBinaryMemoryLimit)) {
+    if (is_full) {
+      *is_full = true;
+      return Status::OK();
+    } else {
+      return Status::Invalid("Maximum array size reached (2GB)");
+    }
+  }
+  RETURN_NOT_OK(builder->Append(view.bytes, length));
+  if (is_full) {
+    *is_full = false;
+  }
+  return Status::OK();
+}
+
+Status BuilderAppend(FixedSizeBinaryBuilder* builder, PyObject* obj, bool* is_full) {
+  PyBytesView view;
+  // XXX For some reason, we must accept unicode objects here
+  RETURN_NOT_OK(PyBytesView::FromString(obj, &view));
+  const auto expected_length =
+      static_cast<const FixedSizeBinaryType&>(*builder->type()).byte_width();
+  if (ARROW_PREDICT_FALSE(view.size != expected_length)) {
+    std::stringstream ss;
+    ss << "Got bytestring of length " << view.size << " (expected " << expected_length
+       << ")";
+    return Status::Invalid(ss.str());
+  }
+  // Did we reach the builder size limit?
+  if (ARROW_PREDICT_FALSE(builder->value_data_length() + view.size >
+                          kBinaryMemoryLimit)) {
+    if (is_full) {
+      *is_full = true;
+      return Status::OK();
+    } else {
+      return Status::Invalid("Maximum array size reached (2GB)");
+    }
+  }
+  RETURN_NOT_OK(builder->Append(view.bytes));
+  if (is_full) {
+    *is_full = false;
+  }
+  return Status::OK();
+}
+
+Status BuilderAppend(StringBuilder* builder, PyObject* obj, bool check_valid,
+                     bool* is_full) {
+  PyBytesView view;
+  RETURN_NOT_OK(PyBytesView::FromString(obj, &view, check_valid));
+  int32_t length;
+  RETURN_NOT_OK(CastSize(view.size, &length));
+  // Did we reach the builder size limit?
+  if (ARROW_PREDICT_FALSE(builder->value_data_length() + length > kBinaryMemoryLimit)) {
+    if (is_full) {
+      *is_full = true;
+      return Status::OK();
+    } else {
+      return Status::Invalid("Maximum array size reached (2GB)");
+    }
+  }
+  RETURN_NOT_OK(builder->Append(view.bytes, length));
+  if (is_full) {
+    *is_full = false;
+  }
+  return Status::OK();
+}
+
+namespace {
+
+template <typename UInt>
+Status UnsignedIntFromPython(PyObject* obj, UInt* out) {
+  static_assert(std::is_unsigned<UInt>::value, "requires unsigned integer type");
+  static_assert(sizeof(UInt) <= sizeof(unsigned long long),  // NOLINT
+                "integer type larger than unsigned long long");
+
+  OwnedRef ref;
+  // PyLong_AsUnsignedLong() and PyLong_AsUnsignedLongLong() don't handle
+  // conversion from non-ints (e.g. np.uint64), so do it ourselves
+  if (!PyLong_Check(obj)) {
+    ref.reset(PyNumber_Long(obj));
+    if (!ref) {
+      RETURN_IF_PYERROR();
+    }
+    obj = ref.obj();
+  }
+  if (sizeof(UInt) > sizeof(unsigned long)) {  // NOLINT
+    const auto value = PyLong_AsUnsignedLongLong(obj);
+    if (ARROW_PREDICT_FALSE(value == static_cast<decltype(value)>(-1))) {
+      RETURN_IF_PYERROR();
+    }
+    if (ARROW_PREDICT_FALSE(value > std::numeric_limits<UInt>::max())) {
+      std::stringstream ss;
+      ss << "value too large to fit in " << typeid(UInt).name();
+      return Status::Invalid(ss.str());
+    }
+    *out = static_cast<UInt>(value);
+  } else {
+    const auto value = PyLong_AsUnsignedLong(obj);
+    if (ARROW_PREDICT_FALSE(value == static_cast<decltype(value)>(-1))) {
+      RETURN_IF_PYERROR();
+    }
+    if (ARROW_PREDICT_FALSE(value > std::numeric_limits<UInt>::max())) {
+      std::stringstream ss;
+      ss << "value too large to fit in " << typeid(UInt).name();
+      return Status::Invalid(ss.str());
+    }
+    *out = static_cast<UInt>(value);
+  }
+  return Status::OK();
+}
+
+template <typename Int>
+Status SignedIntFromPython(PyObject* obj, Int* out) {
+  static_assert(std::is_signed<Int>::value, "requires signed integer type");
+  static_assert(sizeof(Int) <= sizeof(long long),  // NOLINT
+                "integer type larger than long long");
+
+  if (sizeof(Int) > sizeof(long)) {  // NOLINT
+    const auto value = PyLong_AsLongLong(obj);
+    if (ARROW_PREDICT_FALSE(value == -1)) {
+      RETURN_IF_PYERROR();
+    }
+    if (ARROW_PREDICT_FALSE(value < std::numeric_limits<Int>::min() ||
+                            value > std::numeric_limits<Int>::max())) {
+      std::stringstream ss;
+      ss << "value too large to fit in " << typeid(Int).name();
+      return Status::Invalid(ss.str());
+    }
+    *out = static_cast<Int>(value);
+  } else {
+    const auto value = PyLong_AsLong(obj);
+    if (ARROW_PREDICT_FALSE(value == -1)) {
+      RETURN_IF_PYERROR();
+    }
+    if (ARROW_PREDICT_FALSE(value < std::numeric_limits<Int>::min() ||
+                            value > std::numeric_limits<Int>::max())) {
+      std::stringstream ss;
+      ss << "value too large to fit in " << typeid(Int).name();
+      return Status::Invalid(ss.str());
+    }
+    *out = static_cast<Int>(value);
+  }
+  return Status::OK();
+}
+
+}  // namespace
+
+Status UInt8FromPythonInt(PyObject* obj, uint8_t* out) {
+  return UnsignedIntFromPython(obj, out);
+}
+
+Status UInt16FromPythonInt(PyObject* obj, uint16_t* out) {
+  return UnsignedIntFromPython(obj, out);
+}
+
+Status UInt32FromPythonInt(PyObject* obj, uint32_t* out) {
+  return UnsignedIntFromPython(obj, out);
+}
+
+Status UInt64FromPythonInt(PyObject* obj, uint64_t* out) {
+  return UnsignedIntFromPython(obj, out);
+}
+
+Status Int8FromPythonInt(PyObject* obj, int8_t* out) {
+  return SignedIntFromPython(obj, out);
+}
+
+Status Int16FromPythonInt(PyObject* obj, int16_t* out) {
+  return SignedIntFromPython(obj, out);
+}
+
+Status Int32FromPythonInt(PyObject* obj, int32_t* out) {
+  return SignedIntFromPython(obj, out);
+}
+
+Status Int64FromPythonInt(PyObject* obj, int64_t* out) {
+  return SignedIntFromPython(obj, out);
+}
+
+//
+// Python Decimal support
+//
+
 Status ImportDecimalType(OwnedRef* decimal_type) {
   OwnedRef decimal_module;
   RETURN_NOT_OK(ImportModule("decimal", &decimal_module));
@@ -106,20 +339,7 @@ Status ImportDecimalType(OwnedRef* decimal_type) {
 
 Status PythonDecimalToString(PyObject* python_decimal, std::string* out) {
   // Call Python's str(decimal_object)
-  OwnedRef str_obj(PyObject_Str(python_decimal));
-  RETURN_IF_PYERROR();
-
-  PyObjectStringify str(str_obj.obj());
-  RETURN_IF_PYERROR();
-
-  const char* bytes = str.bytes;
-  DCHECK_NE(bytes, nullptr);
-
-  Py_ssize_t size = str.size;
-
-  std::string c_string(bytes, size);
-  *out = c_string;
-  return Status::OK();
+  return PyObject_StdStringStr(python_decimal, out);
 }
 
 // \brief Infer the precision and scale of a Python decimal.Decimal instance
@@ -219,31 +439,6 @@ Status DecimalFromPythonDecimal(PyObject* python_decimal, const DecimalType& arr
   return Status::OK();
 }
 
-bool IsPyInteger(PyObject* obj) {
-#if PYARROW_IS_PY2
-  return PyLong_Check(obj) || PyInt_Check(obj);
-#else
-  return PyLong_Check(obj);
-#endif
-}
-
-Status UInt64FromPythonInt(PyObject* obj, uint64_t* out) {
-  OwnedRef ref;
-  // PyLong_AsUnsignedLongLong() doesn't handle conversion from non-ints
-  // (e.g. np.uint64), so do it ourselves
-  if (!PyLong_Check(obj)) {
-    ref.reset(PyNumber_Long(obj));
-    RETURN_IF_PYERROR();
-    obj = ref.obj();
-  }
-  auto result = static_cast<uint64_t>(PyLong_AsUnsignedLongLong(obj));
-  if (result == static_cast<uint64_t>(-1)) {
-    RETURN_IF_PYERROR();
-  }
-  *out = static_cast<uint64_t>(result);
-  return Status::OK();
-}
-
 bool PyDecimal_Check(PyObject* obj) {
   static OwnedRef decimal_type;
   if (!decimal_type.obj()) {

cpp/src/arrow/python/helpers.h

@@ -20,6 +20,7 @@
 
 #include "arrow/python/platform.h"
 
+#include <limits>
 #include <memory>
 #include <string>
 #include <utility>
@@ -63,6 +64,72 @@ Status ImportModule(const std::string& module_name, OwnedRef* ref);
 // module
 Status ImportFromModule(const OwnedRef& module, const std::string& name, OwnedRef* ref);
 
+// \brief Check whether obj is an integer, independent of Python versions.
+inline bool IsPyInteger(PyObject* obj) {
+#if PYARROW_IS_PY2
+  return PyLong_Check(obj) || PyInt_Check(obj);
+#else
+  return PyLong_Check(obj);
+#endif
+}
+
+// \brief Use pandas missing value semantics to check if a value is null
+bool PandasObjectIsNull(PyObject* obj);
+
+// \brief Check whether obj is nan
+bool PyFloat_IsNaN(PyObject* obj);
+
+inline bool IsPyBinary(PyObject* obj) {
+  return PyBytes_Check(obj) || PyByteArray_Check(obj);
+}
+
+// \brief Convert a Python integer into a C integer
+// \param[in] obj A Python integer
+// \param[out] out A pointer to a C integer to hold the result of the conversion
+// \return The status of the operation
+Status Int8FromPythonInt(PyObject* obj, int8_t* out);
+Status Int16FromPythonInt(PyObject* obj, int16_t* out);
+Status Int32FromPythonInt(PyObject* obj, int32_t* out);
+Status Int64FromPythonInt(PyObject* obj, int64_t* out);
+Status UInt8FromPythonInt(PyObject* obj, uint8_t* out);
+Status UInt16FromPythonInt(PyObject* obj, uint16_t* out);
+Status UInt32FromPythonInt(PyObject* obj, uint32_t* out);
+Status UInt64FromPythonInt(PyObject* obj, uint64_t* out);
+
+// \brief Convert a Python unicode string to a std::string
+Status PyUnicode_AsStdString(PyObject* obj, std::string* out);
+
+// \brief Convert a Python bytes object to a std::string
+std::string PyBytes_AsStdString(PyObject* obj);
+
+// \brief Call str() on the given object and return the result as a std::string
+Status PyObject_StdStringStr(PyObject* obj, std::string* out);
+
+// \brief Return the repr() of the given object (always succeeds)
+std::string PyObject_StdStringRepr(PyObject* obj);
+
+// \brief Cast the given size to int32_t, with error checking
+inline Status CastSize(Py_ssize_t size, int32_t* out,
+                       const std::string& error_msg = "Maximum size exceeded (2GB)") {
+  // size is assumed to be positive
+  if (size > std::numeric_limits<int32_t>::max()) {
+    return Status::Invalid(error_msg);
+  }
+  *out = static_cast<int32_t>(size);
+  return Status::OK();
+}
+
+Status BuilderAppend(StringBuilder* builder, PyObject* obj, bool check_valid = false,
+                     bool* is_full = nullptr);
+Status BuilderAppend(BinaryBuilder* builder, PyObject* obj, bool* is_full = nullptr);
+Status BuilderAppend(FixedSizeBinaryBuilder* builder, PyObject* obj,
+                     bool* is_full = nullptr);
+
+//
+// Decimal helpers
+// XXX decimal.h?
+//
+
 // \brief Import
 Status ImportDecimalType(OwnedRef* decimal_type);
 
@@ -87,15 +154,6 @@ PyObject* DecimalFromString(PyObject* decimal_constructor,
 Status DecimalFromPythonDecimal(PyObject* python_decimal, const DecimalType& arrow_type,
                                 Decimal128* out);
 
-// \brief Check whether obj is an integer, independent of Python versions.
-bool IsPyInteger(PyObject* obj);
-
-// \brief Use pandas missing value semantics to check if a value is null
-bool PandasObjectIsNull(PyObject* obj);
-
-// \brief Check whether obj is nan
-bool PyFloat_IsNaN(PyObject* obj);
-
 // \brief Check whether obj is an instance of Decimal
 bool PyDecimal_Check(PyObject* obj);
 
@@ -103,12 +161,6 @@ bool PyDecimal_Check(PyObject* obj);
 // is not a Decimal instance
 bool PyDecimal_ISNAN(PyObject* obj);
 
-// \brief Convert a Python integer into an unsigned 64-bit integer
-// \param[in] obj A Python integer
-// \param[out] out A pointer to a C uint64_t to hold the result of the conversion
-// \return The status of the operation
-Status UInt64FromPythonInt(PyObject* obj, uint64_t* out);
-
 // \brief Helper class to track and update the precision and scale of a decimal
 class DecimalMetadata {
  public:

cpp/src/arrow/python/numpy_to_arrow.cc

@@ -65,14 +65,6 @@ using internal::NumPyTypeSize;
 
 namespace {
 
-inline bool PyObject_is_string(PyObject* obj) {
-#if PY_MAJOR_VERSION >= 3
-  return PyUnicode_Check(obj) || PyBytes_Check(obj);
-#else
-  return PyString_Check(obj) || PyUnicode_Check(obj);
-#endif
-}
-
 inline bool PyObject_is_integer(PyObject* obj) {
   return !PyBool_Check(obj) && PyArray_IsIntegerScalar(obj);
 }
@@ -204,8 +196,6 @@ int64_t MaskToBitmap(PyArrayObject* mask, int64_t length, uint8_t* bitmap) {
 static Status AppendObjectBinaries(PyArrayObject* arr, PyArrayObject* mask,
                                    int64_t offset, BinaryBuilder* builder,
                                    int64_t* end_offset) {
-  PyObject* obj;
-
   Ndarray1DIndexer<PyObject*> objects(arr);
   Ndarray1DIndexer<uint8_t> mask_values;
 
@@ -216,30 +206,15 @@ static Status AppendObjectBinaries(PyArrayObject* arr, PyArrayObject* mask,
   }
 
   for (; offset < objects.size(); ++offset) {
-    OwnedRef tmp_obj;
-    obj = objects[offset];
+    PyObject* obj = objects[offset];
     if ((have_mask && mask_values[offset]) || internal::PandasObjectIsNull(obj)) {
       RETURN_NOT_OK(builder->AppendNull());
       continue;
-    } else if (PyBytes_Check(obj)) {
-      const int32_t length = static_cast<int32_t>(PyBytes_GET_SIZE(obj));
-      if (ARROW_PREDICT_FALSE(builder->value_data_length() + length >
-                              kBinaryMemoryLimit)) {
-        break;
-      }
-      RETURN_NOT_OK(builder->Append(PyBytes_AS_STRING(obj), length));
-    } else if (PyByteArray_Check(obj)) {
-      const int32_t length = static_cast<int32_t>(PyByteArray_GET_SIZE(obj));
-      if (ARROW_PREDICT_FALSE(builder->value_data_length() + length >
-                              kBinaryMemoryLimit)) {
-        break;
-      }
-      RETURN_NOT_OK(builder->Append(PyByteArray_AS_STRING(obj), length));
-    } else {
-      std::stringstream ss;
-      ss << "Error converting from Python objects to bytes: ";
-      RETURN_NOT_OK(InvalidConversion(obj, "str, bytes, bytearray", &ss));
-      return Status::Invalid(ss.str());
+    }
+    bool is_full;
+    RETURN_NOT_OK(internal::BuilderAppend(builder, obj, &is_full));
+    if (is_full) {
+      break;
     }
   }
 
@@ -275,27 +250,16 @@ static Status AppendObjectStrings(PyArrayObject* arr, PyArrayObject* mask, int64
     if ((have_mask && mask_values[offset]) || internal::PandasObjectIsNull(obj)) {
       RETURN_NOT_OK(builder->AppendNull());
       continue;
-    } else if (PyUnicode_Check(obj)) {
-      obj = PyUnicode_AsUTF8String(obj);
-      if (obj == NULL) {
-        PyErr_Clear();
-        return Status::Invalid("failed converting unicode to UTF8");
-      }
-      tmp_obj.reset(obj);
-    } else if (PyBytes_Check(obj)) {
-      *have_bytes = true;
-    } else {
-      std::stringstream ss;
-      ss << "Error converting from Python objects to String/UTF8: ";
-      RETURN_NOT_OK(InvalidConversion(obj, "str, bytes", &ss));
-      return Status::Invalid(ss.str());
     }
-
-    const int32_t length = static_cast<int32_t>(PyBytes_GET_SIZE(obj));
-    if (ARROW_PREDICT_FALSE(builder->value_data_length() + length > kBinaryMemoryLimit)) {
+    if (internal::IsPyBinary(obj)) {
+      *have_bytes = true;
+    }
+    bool is_full;
+    RETURN_NOT_OK(
+        internal::BuilderAppend(builder, obj, false /* check_valid */, &is_full));
+    if (is_full) {
       break;
     }
-    RETURN_NOT_OK(builder->Append(PyBytes_AS_STRING(obj), length));
   }
 
   // If we consumed the whole array, this will be the length of arr
@@ -324,28 +288,12 @@ static Status AppendObjectFixedWidthBytes(PyArrayObject* arr, PyArrayObject* mas
     if ((have_mask && mask_values[offset]) || internal::PandasObjectIsNull(obj)) {
       RETURN_NOT_OK(builder->AppendNull());
       continue;
-    } else if (PyUnicode_Check(obj)) {
-      obj = PyUnicode_AsUTF8String(obj);
-      if (obj == NULL) {
-        PyErr_Clear();
-        return Status::Invalid("failed converting unicode to UTF8");
-      }
-
-      tmp_obj.reset(obj);
-    } else if (!PyBytes_Check(obj)) {
-      std::stringstream ss;
-      ss << "Error converting from Python objects to FixedSizeBinary: ";
-      RETURN_NOT_OK(InvalidConversion(obj, "str, bytes", &ss));
-      return Status::Invalid(ss.str());
     }
-
-    RETURN_NOT_OK(CheckPythonBytesAreFixedLength(obj, byte_width));
-    if (ARROW_PREDICT_FALSE(builder->value_data_length() + byte_width >
-                            kBinaryMemoryLimit)) {
+    bool is_full;
+    RETURN_NOT_OK(internal::BuilderAppend(builder, obj, &is_full));
+    if (is_full) {
       break;
     }
-    RETURN_NOT_OK(
-        builder->Append(reinterpret_cast<const uint8_t*>(PyBytes_AS_STRING(obj))));
   }
 
   // If we consumed the whole array, this will be the length of arr
@@ -1102,7 +1050,9 @@ Status NumPyConverter::ConvertObjectsInfer() {
     PyObject* obj = objects[i];
     if (internal::PandasObjectIsNull(obj)) {
       continue;
-    } else if (PyObject_is_string(obj)) {
+    } else if (PyUnicode_Check(obj) || internal::IsPyBinary(obj)) {
+      // The exact Arrow type (Binary or String) will be decided based on
+      // Python object types
       return ConvertObjectStrings();
     } else if (PyFloat_Check(obj)) {
       return ConvertObjectFloats();
@@ -1119,8 +1069,6 @@ Status NumPyConverter::ConvertObjectsInfer() {
       return ConvertTimes();
     } else if (PyObject_IsInstance(obj, decimal_type_.obj()) == 1) {
       return ConvertDecimals();
-    } else if (PyByteArray_Check(obj)) {
-      return ConvertObjectBytes();
     } else if (PyList_Check(obj)) {
       std::shared_ptr<DataType> inferred_type;
       RETURN_NOT_OK(InferArrowType(obj, &inferred_type));

cpp/src/arrow/python/python_to_arrow.cc

@@ -84,7 +84,9 @@ class SequenceBuilder {
     if (*tag == -1) {
       *tag = num_tags_++;
     }
-    RETURN_NOT_OK(offsets_.Append(static_cast<int32_t>(offset)));
+    int32_t offset32;
+    RETURN_NOT_OK(internal::CastSize(offset, &offset32));
+    RETURN_NOT_OK(offsets_.Append(offset32));
     RETURN_NOT_OK(types_.Append(*tag));
     return nones_.AppendToBitmap(true);
   }
@@ -173,26 +175,34 @@ class SequenceBuilder {
   /// \param size
   /// The size of the sublist
   Status AppendList(Py_ssize_t size) {
+    int32_t offset;
+    RETURN_NOT_OK(internal::CastSize(list_offsets_.back() + size, &offset));
     RETURN_NOT_OK(Update(list_offsets_.size() - 1, &list_tag_));
-    list_offsets_.push_back(list_offsets_.back() + static_cast<int32_t>(size));
+    list_offsets_.push_back(offset);
     return Status::OK();
   }
 
   Status AppendTuple(Py_ssize_t size) {
+    int32_t offset;
+    RETURN_NOT_OK(internal::CastSize(tuple_offsets_.back() + size, &offset));
     RETURN_NOT_OK(Update(tuple_offsets_.size() - 1, &tuple_tag_));
-    tuple_offsets_.push_back(tuple_offsets_.back() + static_cast<int32_t>(size));
+    tuple_offsets_.push_back(offset);
     return Status::OK();
   }
 
   Status AppendDict(Py_ssize_t size) {
+    int32_t offset;
+    RETURN_NOT_OK(internal::CastSize(dict_offsets_.back() + size, &offset));
     RETURN_NOT_OK(Update(dict_offsets_.size() - 1, &dict_tag_));
-    dict_offsets_.push_back(dict_offsets_.back() + static_cast<int32_t>(size));
+    dict_offsets_.push_back(offset);
     return Status::OK();
   }
 
   Status AppendSet(Py_ssize_t size) {
+    int32_t offset;
+    RETURN_NOT_OK(internal::CastSize(set_offsets_.back() + size, &offset));
     RETURN_NOT_OK(Update(set_offsets_.size() - 1, &set_tag_));
-    set_offsets_.push_back(set_offsets_.back() + static_cast<int32_t>(size));
+    set_offsets_.push_back(offset);
     return Status::OK();
   }
 
@@ -365,17 +375,8 @@ Status CallCustomCallback(PyObject* context, PyObject* method_name, PyObject* el
   *result = NULL;
   if (context == Py_None) {
     std::stringstream ss;
-    OwnedRef repr(PyObject_Repr(elem));
-    RETURN_IF_PYERROR();
-#if PY_MAJOR_VERSION >= 3
-    OwnedRef ascii(PyUnicode_AsASCIIString(repr.obj()));
-    RETURN_IF_PYERROR();
-    ss << "error while calling callback on " << PyBytes_AsString(ascii.obj())
+    ss << "error while calling callback on " << internal::PyObject_StdStringRepr(elem)
        << ": handler not registered";
-#else
-    ss << "error while calling callback on " << PyString_AsString(repr.obj())
-       << ": handler not registered";
-#endif
     return Status::SerializationError(ss.str());
   } else {
     *result = PyObject_CallMethodObjArgs(context, method_name, elem, NULL);
@@ -483,24 +484,15 @@ Status Append(PyObject* context, PyObject* elem, SequenceBuilder* builder,
 #endif
   } else if (PyBytes_Check(elem)) {
     auto data = reinterpret_cast<uint8_t*>(PyBytes_AS_STRING(elem));
-    const int64_t size = static_cast<int64_t>(PyBytes_GET_SIZE(elem));
-    if (size > std::numeric_limits<int32_t>::max()) {
-      return Status::Invalid("Cannot writes bytes over 2GB");
-    }
-    RETURN_NOT_OK(builder->AppendBytes(data, static_cast<int32_t>(size)));
+    int32_t size;
+    RETURN_NOT_OK(internal::CastSize(PyBytes_GET_SIZE(elem), &size));
+    RETURN_NOT_OK(builder->AppendBytes(data, size));
   } else if (PyUnicode_Check(elem)) {
-    Py_ssize_t size;
-#if PY_MAJOR_VERSION >= 3
-    char* data = PyUnicode_AsUTF8AndSize(elem, &size);
-#else
-    OwnedRef str(PyUnicode_AsUTF8String(elem));
-    char* data = PyString_AS_STRING(str.obj());
-    size = PyString_GET_SIZE(str.obj());
-#endif
-    if (size > std::numeric_limits<int32_t>::max()) {
-      return Status::Invalid("Cannot writes bytes over 2GB");
-    }
-    RETURN_NOT_OK(builder->AppendString(data, static_cast<int32_t>(size)));
+    PyBytesView view;
+    RETURN_NOT_OK(PyBytesView::FromString(elem, &view));
+    int32_t size;
+    RETURN_NOT_OK(internal::CastSize(view.size, &size));
+    RETURN_NOT_OK(builder->AppendString(view.bytes, size));
   } else if (PyList_CheckExact(elem)) {
     RETURN_NOT_OK(builder->AppendList(PyList_Size(elem)));
     sublists->push_back(elem);

python/pyarrow/tests/test_convert_builtin.py

@@ -215,6 +215,26 @@ def test_sequence_numpy_integer_inferred(seq, np_scalar_pa_type):
     assert arr.to_pylist() == expected
 
 
+@pytest.mark.parametrize("bits", [8, 16, 32, 64])
+def test_signed_integer_overflow(bits):
+    ty = getattr(pa, "int%d" % bits)()
+    # XXX ideally would raise OverflowError
+    with pytest.raises((ValueError, pa.ArrowException)):
+        pa.array([2 ** (bits - 1)], ty)
+    with pytest.raises((ValueError, pa.ArrowException)):
+        pa.array([-2 ** (bits - 1) - 1], ty)
+
+
+@pytest.mark.parametrize("bits", [8, 16, 32, 64])
+def test_unsigned_integer_overflow(bits):
+    ty = getattr(pa, "uint%d" % bits)()
+    # XXX ideally would raise OverflowError
+    with pytest.raises((ValueError, pa.ArrowException)):
+        pa.array([2 ** bits], ty)
+    with pytest.raises((ValueError, pa.ArrowException)):
+        pa.array([-1], ty)
+
+
 def test_garbage_collection():
     import gc
 
@@ -260,12 +280,14 @@ def test_sequence_bytes():
     u1 = b'ma\xc3\xb1ana'
     data = [b'foo',
             u1.decode('utf-8'),  # unicode gets encoded,
+            bytearray(b'bar'),
             None]
-    arr = pa.array(data)
-    assert len(arr) == 3
-    assert arr.null_count == 1
-    assert arr.type == pa.binary()
-    assert arr.to_pylist() == [b'foo', u1, None]
+    for ty in [None, pa.binary()]:
+        arr = pa.array(data, type=ty)
+        assert len(arr) == 4
+        assert arr.null_count == 1
+        assert arr.type == pa.binary()
+        assert arr.to_pylist() == [b'foo', u1, b'bar', None]
 
 
 def test_sequence_utf8_to_unicode():
@@ -281,12 +303,12 @@ def test_sequence_utf8_to_unicode():
 
 
 def test_sequence_fixed_size_bytes():
-    data = [b'foof', None, b'barb', b'2346']
+    data = [b'foof', None, bytearray(b'barb'), b'2346']
     arr = pa.array(data, type=pa.binary(4))
     assert len(arr) == 4
     assert arr.null_count == 1
     assert arr.type == pa.binary(4)
-    assert arr.to_pylist() == data
+    assert arr.to_pylist() == [b'foof', None, b'barb', b'2346']
 
 
 def test_fixed_size_bytes_does_not_accept_varying_lengths():
@@ -473,7 +495,7 @@ def test_sequence_mixed_types_with_specified_type_fails():
     data = ['-10', '-5', {'a': 1}, '0', '5', '10']
 
     type = pa.string()
-    with pytest.raises(pa.ArrowInvalid):
+    with pytest.raises(TypeError):
         pa.array(data, type=type)
 
 

python/pyarrow/tests/test_convert_pandas.py

@@ -1065,13 +1065,13 @@ class TestConvertStringLikeTypes(object):
         _check_pandas_roundtrip(df, expected_schema=schema)
 
     def test_bytes_to_binary(self):
-        values = [u('qux'), b'foo', None, 'bar', 'qux', np.nan]
+        values = [u'qux', b'foo', None, bytearray(b'barz'), 'qux', np.nan]
         df = pd.DataFrame({'strings': values})
 
         table = pa.Table.from_pandas(df)
         assert table[0].type == pa.binary()
 
-        values2 = [b'qux', b'foo', None, b'bar', b'qux', np.nan]
+        values2 = [b'qux', b'foo', None, b'barz', b'qux', np.nan]
         expected = pd.DataFrame({'strings': values2})
         _check_pandas_roundtrip(df, expected)
 
@@ -1093,7 +1093,7 @@ class TestConvertStringLikeTypes(object):
         assert table[0].data.num_chunks == 2
 
     def test_fixed_size_bytes(self):
-        values = [b'foo', None, b'bar', None, None, b'hey']
+        values = [b'foo', None, bytearray(b'bar'), None, None, b'hey']
         df = pd.DataFrame({'strings': values})
         schema = pa.schema([pa.field('strings', pa.binary(3))])
         table = pa.Table.from_pandas(df, schema=schema)

python/pyarrow/tests/test_feather.py

@@ -465,7 +465,7 @@ class TestFeatherReader(unittest.TestCase):
 
         # non-strings
         df = pd.DataFrame({'a': ['a', 1, 2.0]})
-        self._assert_error_on_write(df, ValueError)
+        self._assert_error_on_write(df, TypeError)
 
     @pytest.mark.slow
     def test_large_dataframe(self):