pitrou commented on code in PR #46618:
URL: https://github.com/apache/arrow/pull/46618#discussion_r2111621299
##########
python/pyarrow/src/arrow/python/python_to_arrow.cc:
##########
@@ -227,7 +227,16 @@ class PyValue {
static Result<uint16_t> Convert(const HalfFloatType*, const O&, I obj) {
uint16_t value;
- RETURN_NOT_OK(PyFloat_AsHalf(obj, &value));
+ if (internal::PyFloatScalar_Check(obj)) {
+ RETURN_NOT_OK(PyFloat_AsHalf(obj, &value));
+ } else if (internal::PyIntScalar_Check(obj)) {
+ float float_val{};
+ RETURN_NOT_OK(internal::IntegerScalarToFloat32Safe(obj, &float_val));
+ arrow::util::Float16 half_val =
arrow::util::Float16::FromFloat(float_val);
Review Comment:
You can probably make this a little less wordy.
```suggestion
const auto half_val = arrow::util::Float16::FromFloat(float_val);
```
##########
python/pyarrow/src/arrow/python/helpers.h:
##########
@@ -44,10 +44,14 @@ class OwnedRef;
ARROW_PYTHON_EXPORT std::shared_ptr<DataType> GetPrimitiveType(Type::type
type);
// \brief Construct a np.float16 object from a npy_half value.
+ARROW_DEPRECATED("Deprecated in 21.0.0. Will be removed in 23.0.0")
ARROW_PYTHON_EXPORT PyObject* PyHalf_FromHalf(npy_half value);
-// \brief Convert a Python object to a npy_half value.
-ARROW_PYTHON_EXPORT Status PyFloat_AsHalf(PyObject* obj, npy_half* out);
+// \brief Construct a Python float object from a uint16_t value.
+ARROW_PYTHON_EXPORT PyObject* PyFloat_FromHalf(uint16_t value);
+
+// \brief Convert a Python object to a uint16_t value.
Review Comment:
```suggestion
// \brief Convert a Python object to a half-float uint16_t value.
```
##########
python/pyarrow/src/arrow/python/helpers.h:
##########
@@ -44,10 +44,14 @@ class OwnedRef;
ARROW_PYTHON_EXPORT std::shared_ptr<DataType> GetPrimitiveType(Type::type
type);
// \brief Construct a np.float16 object from a npy_half value.
+ARROW_DEPRECATED("Deprecated in 21.0.0. Will be removed in 23.0.0")
ARROW_PYTHON_EXPORT PyObject* PyHalf_FromHalf(npy_half value);
-// \brief Convert a Python object to a npy_half value.
-ARROW_PYTHON_EXPORT Status PyFloat_AsHalf(PyObject* obj, npy_half* out);
+// \brief Construct a Python float object from a uint16_t value.
+ARROW_PYTHON_EXPORT PyObject* PyFloat_FromHalf(uint16_t value);
+
+// \brief Convert a Python object to a uint16_t value.
+ARROW_PYTHON_EXPORT Status PyFloat_AsHalf(PyObject* obj, uint16_t* out);
Review Comment:
Can this return `Result<uint16_t>` instead?
##########
python/pyarrow/src/arrow/python/helpers.cc:
##########
@@ -81,14 +83,23 @@ PyObject* PyHalf_FromHalf(npy_half value) {
return result;
}
-Status PyFloat_AsHalf(PyObject* obj, npy_half* out) {
- if (PyArray_IsScalar(obj, Half)) {
+PyObject* PyFloat_FromHalf(uint16_t value) {
+ // Convert the uint16_t Float16 value to a PyFloat object
+ arrow::util::Float16 half_val = arrow::util::Float16::FromBits(value);
+ PyObject* result = PyFloat_FromDouble(half_val.ToDouble());
+ return result;
+}
+
+Status PyFloat_AsHalf(PyObject* obj, uint16_t* out) {
+ if (PyFloat_Check(obj)) {
+ // Use Arrow's Float16 implementation instead of NumPy
+ float float_val = static_cast<float>(PyFloat_AsDouble(obj));
+ arrow::util::Float16 half_val = arrow::util::Float16::FromFloat(float_val);
+ *out = half_val.bits();
+ } else if (has_numpy() && PyArray_IsScalar(obj, Half)) {
*out = PyArrayScalar_VAL(obj, Half);
- return Status::OK();
- } else {
- // XXX: cannot use npy_double_to_half() without linking with Numpy
- return Status::TypeError("Expected np.float16 instance");
}
Review Comment:
No error return otherwise?
##########
python/pyarrow/src/arrow/python/helpers.h:
##########
@@ -44,10 +44,14 @@ class OwnedRef;
ARROW_PYTHON_EXPORT std::shared_ptr<DataType> GetPrimitiveType(Type::type
type);
// \brief Construct a np.float16 object from a npy_half value.
+ARROW_DEPRECATED("Deprecated in 21.0.0. Will be removed in 23.0.0")
Review Comment:
Do we care about deprecating it? These should be considered internal APIs
IMHO.
Besides, there seem to be [no other public
uses](https://grep.app/search?q=PyHalf_FromHalf).
##########
python/pyarrow/tests/test_array.py:
##########
@@ -1664,6 +1664,16 @@ def test_floating_point_truncate_unsafe():
_check_cast_case(case, safe=False)
+def test_half_float_array_from_python():
+ # GH-46611
+ arr = pa.array([1.0, 2.0, 3, None], type=pa.float16())
+ assert arr.type == pa.float16()
+ assert arr.to_pylist() == [1.0, 2.0, 3.0, None]
+ msg1 = "Could not convert 'a' with type str: tried to convert to float16"
+ with pytest.raises(pa.ArrowInvalid, match=msg1):
+ arr = pa.array(['a', 3, None], type=pa.float16())
Review Comment:
Is the conversion from integer to float16 tested somewhere?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
