You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/02/11 22:22:51 UTC

[arrow] branch master updated: ARROW-4457: [Python] Allow creating Decimal array from Python ints

This is an automated email from the ASF dual-hosted git repository.

wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/master by this push:
     new 1d72a89  ARROW-4457: [Python] Allow creating Decimal array from Python ints
1d72a89 is described below

commit 1d72a89d49a98878086442d0567655aead544075
Author: Antoine Pitrou <an...@python.org>
AuthorDate: Mon Feb 11 16:22:42 2019 -0600

    ARROW-4457: [Python] Allow creating Decimal array from Python ints
    
    This is a regression apparently introduced in 0.11.0
    
    Author: Antoine Pitrou <an...@python.org>
    
    Closes #3612 from pitrou/ARROW-4457-decimal-from-pyint and squashes the following commits:
    
    5d0e767e <Antoine Pitrou> ARROW-4457:  Allow creating Decimal array from Python ints
---
 cpp/src/arrow/python/decimal.cc              | 42 ++++++++++++++++++++++------
 cpp/src/arrow/python/decimal.h               |  8 ++++++
 cpp/src/arrow/python/python-test.cc          | 14 ++++++++++
 cpp/src/arrow/python/python_to_arrow.cc      | 12 ++------
 python/pyarrow/tests/test_convert_builtin.py |  8 ++++++
 5 files changed, 67 insertions(+), 17 deletions(-)

diff --git a/cpp/src/arrow/python/decimal.cc b/cpp/src/arrow/python/decimal.cc
index 8db7c01..4b6932d 100644
--- a/cpp/src/arrow/python/decimal.cc
+++ b/cpp/src/arrow/python/decimal.cc
@@ -107,19 +107,15 @@ PyObject* DecimalFromString(PyObject* decimal_constructor,
                                string_size);
 }
 
-Status DecimalFromPythonDecimal(PyObject* python_decimal, const DecimalType& arrow_type,
-                                Decimal128* out) {
-  DCHECK_NE(python_decimal, NULLPTR);
-  DCHECK_NE(out, NULLPTR);
-
-  std::string string;
-  RETURN_NOT_OK(PythonDecimalToString(python_decimal, &string));
+namespace {
 
+Status DecimalFromStdString(const std::string& decimal_string,
+                            const DecimalType& arrow_type, Decimal128* out) {
   int32_t inferred_precision;
   int32_t inferred_scale;
 
   RETURN_NOT_OK(
-      Decimal128::FromString(string, out, &inferred_precision, &inferred_scale));
+      Decimal128::FromString(decimal_string, out, &inferred_precision, &inferred_scale));
 
   const int32_t precision = arrow_type.precision();
   const int32_t scale = arrow_type.scale();
@@ -137,6 +133,36 @@ Status DecimalFromPythonDecimal(PyObject* python_decimal, const DecimalType& arr
   return Status::OK();
 }
 
+}  // namespace
+
+Status DecimalFromPythonDecimal(PyObject* python_decimal, const DecimalType& arrow_type,
+                                Decimal128* out) {
+  DCHECK_NE(python_decimal, NULLPTR);
+  DCHECK_NE(out, NULLPTR);
+
+  std::string string;
+  RETURN_NOT_OK(PythonDecimalToString(python_decimal, &string));
+  return DecimalFromStdString(string, arrow_type, out);
+}
+
+Status DecimalFromPyObject(PyObject* obj, const DecimalType& arrow_type,
+                           Decimal128* out) {
+  DCHECK_NE(obj, NULLPTR);
+  DCHECK_NE(out, NULLPTR);
+
+  if (IsPyInteger(obj)) {
+    // TODO: add a fast path for small-ish ints
+    std::string string;
+    RETURN_NOT_OK(PyObject_StdStringStr(obj, &string));
+    return DecimalFromStdString(string, arrow_type, out);
+  } else if (PyDecimal_Check(obj)) {
+    return DecimalFromPythonDecimal(obj, arrow_type, out);
+  } else {
+    return Status::TypeError("int or Decimal object expected, got ",
+                             Py_TYPE(obj)->tp_name);
+  }
+}
+
 bool PyDecimal_Check(PyObject* obj) {
   static OwnedRef decimal_type;
   if (!decimal_type.obj()) {
diff --git a/cpp/src/arrow/python/decimal.h b/cpp/src/arrow/python/decimal.h
index 8072795..0477be8 100644
--- a/cpp/src/arrow/python/decimal.h
+++ b/cpp/src/arrow/python/decimal.h
@@ -65,6 +65,14 @@ ARROW_PYTHON_EXPORT
 Status DecimalFromPythonDecimal(PyObject* python_decimal, const DecimalType& arrow_type,
                                 Decimal128* out);
 
+// \brief Convert a Python object to an Arrow Decimal128 object
+// \param[in] python_decimal A Python int or decimal.Decimal instance
+// \param[in] arrow_type An instance of arrow::DecimalType
+// \param[out] out A pointer to a Decimal128
+// \return The status of the operation
+ARROW_PYTHON_EXPORT
+Status DecimalFromPyObject(PyObject* obj, const DecimalType& arrow_type, Decimal128* out);
+
 // \brief Check whether obj is an instance of Decimal
 ARROW_PYTHON_EXPORT
 bool PyDecimal_Check(PyObject* obj);
diff --git a/cpp/src/arrow/python/python-test.cc b/cpp/src/arrow/python/python-test.cc
index b749a91..ad1ae01 100644
--- a/cpp/src/arrow/python/python-test.cc
+++ b/cpp/src/arrow/python/python-test.cc
@@ -296,6 +296,11 @@ TEST_F(DecimalTest, FromPythonDecimalRescaleTruncateable) {
   ASSERT_OK(
       internal::DecimalFromPythonDecimal(python_decimal.obj(), decimal_type, &value));
   ASSERT_EQ(100, value.low_bits());
+  ASSERT_EQ(0, value.high_bits());
+
+  ASSERT_OK(internal::DecimalFromPyObject(python_decimal.obj(), decimal_type, &value));
+  ASSERT_EQ(100, value.low_bits());
+  ASSERT_EQ(0, value.high_bits());
 }
 
 TEST_F(DecimalTest, FromPythonNegativeDecimalRescale) {
@@ -308,6 +313,15 @@ TEST_F(DecimalTest, FromPythonNegativeDecimalRescale) {
   ASSERT_EQ(-1000000000, value);
 }
 
+TEST_F(DecimalTest, FromPythonInteger) {
+  Decimal128 value;
+  OwnedRef python_long(PyLong_FromLong(42));
+  auto type = ::arrow::decimal(10, 2);
+  const auto& decimal_type = checked_cast<const DecimalType&>(*type);
+  ASSERT_OK(internal::DecimalFromPyObject(python_long.obj(), decimal_type, &value));
+  ASSERT_EQ(4200, value);
+}
+
 TEST_F(DecimalTest, TestOverflowFails) {
   Decimal128 value;
   OwnedRef python_decimal(
diff --git a/cpp/src/arrow/python/python_to_arrow.cc b/cpp/src/arrow/python/python_to_arrow.cc
index f5e6a57..2d07f0e 100644
--- a/cpp/src/arrow/python/python_to_arrow.cc
+++ b/cpp/src/arrow/python/python_to_arrow.cc
@@ -821,15 +821,9 @@ class DecimalConverter : public TypedConverter<arrow::Decimal128Type, DecimalCon
   }
 
   Status AppendItem(PyObject* obj) {
-    if (internal::PyDecimal_Check(obj)) {
-      Decimal128 value;
-      RETURN_NOT_OK(internal::DecimalFromPythonDecimal(obj, *decimal_type_, &value));
-      return typed_builder_->Append(value);
-    } else {
-      // PyObject_IsInstance could error and set an exception
-      RETURN_IF_PYERROR();
-      return internal::InvalidValue(obj, "converting to Decimal128");
-    }
+    Decimal128 value;
+    RETURN_NOT_OK(internal::DecimalFromPyObject(obj, *decimal_type_, &value));
+    return typed_builder_->Append(value);
   }
 
  private:
diff --git a/python/pyarrow/tests/test_convert_builtin.py b/python/pyarrow/tests/test_convert_builtin.py
index 40a468d..a3d29be 100644
--- a/python/pyarrow/tests/test_convert_builtin.py
+++ b/python/pyarrow/tests/test_convert_builtin.py
@@ -852,6 +852,14 @@ def test_sequence_decimal_large_integer():
     assert arr.to_pylist() == data
 
 
+def test_sequence_decimal_from_integers():
+    data = [0, 1, -39402950693754869342983]
+    expected = [decimal.Decimal(x) for x in data]
+    type = pa.decimal128(precision=28, scale=5)
+    arr = pa.array(data, type=type)
+    assert arr.to_pylist() == expected
+
+
 def test_range_types():
     arr1 = pa.array(range(3))
     arr2 = pa.array((0, 1, 2))