This is an automated email from the ASF dual-hosted git repository.
thisisnic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new be3f4208eb GH-50378: [R] Reading a parquet with a Float16 column
yields incorrect value (#50451)
be3f4208eb is described below
commit be3f4208eb07e72495ce444f806d94601f07a62a
Author: Nic Crane <[email protected]>
AuthorDate: Thu Sep 3 15:16:45 2026 +0100
GH-50378: [R] Reading a parquet with a Float16 column yields incorrect
value (#50451)
### Rationale for this change
Wrong values reading Parquet files with float16s in them as we were
returning the raw bytes not the float value
### What changes are included in this PR?
Return the float value. Also enabling float16 in a lot of our tests.
### Are these changes tested?
Aye
### Are there any user-facing changes?
Nah
* GitHub Issue: #50378
Authored-by: Nic Crane <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
---
r/src/array_to_vector.cpp | 7 ++++++-
r/src/r_to_arrow.cpp | 5 +++--
r/tests/testthat/test-chunked-array.R | 14 +++++++++++++-
r/tests/testthat/test-dplyr-funcs-type.R | 13 ++++++++++++-
4 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/r/src/array_to_vector.cpp b/r/src/array_to_vector.cpp
index dc02c711d1..8b617e6709 100644
--- a/r/src/array_to_vector.cpp
+++ b/r/src/array_to_vector.cpp
@@ -23,6 +23,7 @@
#include <arrow/table.h>
#include <arrow/util/bitmap_reader.h>
#include <arrow/util/bitmap_writer.h>
+#include <arrow/util/float16.h>
#include <arrow/util/int_util.h>
#include <type_traits>
@@ -224,7 +225,11 @@ class Converter_Double : public Converter {
}
auto p_data = REAL(data) + start;
auto ingest_one = [&](R_xlen_t i) {
- p_data[i] = static_cast<value_type>(p_values[i]);
+ if constexpr (std::is_same_v<Type, HalfFloatType>) {
+ p_data[i] = arrow::util::Float16::FromBits(p_values[i]).ToDouble();
+ } else {
+ p_data[i] = static_cast<value_type>(p_values[i]);
+ }
return Status::OK();
};
auto null_one = [&](R_xlen_t i) {
diff --git a/r/src/r_to_arrow.cpp b/r/src/r_to_arrow.cpp
index cbe404aba9..9a26a2c3df 100644
--- a/r/src/r_to_arrow.cpp
+++ b/r/src/r_to_arrow.cpp
@@ -29,6 +29,7 @@
#include <arrow/util/bitmap_writer.h>
#include <arrow/util/checked_cast.h>
#include <arrow/util/converter.h>
+#include <arrow/util/float16.h>
#include <arrow/util/logging.h>
#include "./r_task_group.h"
@@ -390,12 +391,12 @@ struct RConvert {
return static_cast<float>(from);
}
- // ---- convert to half float: not implemented
+ // ---- convert to half float
template <typename Type, typename From>
static enable_if_t<std::is_same<Type, const HalfFloatType>::value,
Result<typename Type::c_type>>
Convert(Type*, From from) {
- return Status::Invalid("Cannot convert to Half Float");
+ return arrow::util::Float16(static_cast<double>(from)).bits();
}
};
diff --git a/r/tests/testthat/test-chunked-array.R
b/r/tests/testthat/test-chunked-array.R
index bcadaa889f..e5fcfefe90 100644
--- a/r/tests/testthat/test-chunked-array.R
+++ b/r/tests/testthat/test-chunked-array.R
@@ -17,7 +17,7 @@
int_types <- c(int8(), int16(), int32(), int64())
uint_types <- c(uint8(), uint16(), uint32(), uint64())
-float_types <- c(float32(), float64()) # float16() not really supported in C++
yet
+float_types <- c(float16(), float32(), float64())
all_numeric_types <- c(int_types, uint_types, float_types)
expect_chunked_roundtrip <- function(x, type) {
@@ -549,3 +549,15 @@ test_that("as_chunked_array() works for Array", {
chunked_array(Array$create(1:6, type = float64()))
)
})
+
+test_that("float16 values roundtrip to R correctly", {
+ # Values exactly representable in half-float, so the roundtrip is exact.
+ # Regression test for GH-50378 (values were previously decoded as raw uint16
bits)
+ x <- c(1, 2, 3.5, NA, -0.25, 1024, Inf)
+ a <- chunked_array(x[1:4], x[5:7], type = float16())
+ expect_type_equal(a$type, float16())
+ expect_identical(a$num_chunks, 2L)
+ expect_as_vector(a, x)
+ expect_as_vector(a$chunk(1), x[5:7])
+ expect_as_vector(a$Slice(1), x[-1])
+})
diff --git a/r/tests/testthat/test-dplyr-funcs-type.R
b/r/tests/testthat/test-dplyr-funcs-type.R
index e2adf1de3e..928a0e3d24 100644
--- a/r/tests/testthat/test-dplyr-funcs-type.R
+++ b/r/tests/testthat/test-dplyr-funcs-type.R
@@ -31,7 +31,7 @@ test_that("explicit type conversions with cast()", {
int_types <- c(int8(), int16(), int32(), int64())
uint_types <- c(uint8(), uint16(), uint32(), uint64())
- float_types <- c(float32(), float64())
+ float_types <- c(float16(), float32(), float64())
types <- c(
int_types,
@@ -1064,3 +1064,14 @@ test_that("format() for unsupported types returns the
input as string", {
collect()
)
})
+
+test_that("cast to float16 roundtrips values correctly", {
+ # Values exactly representable in half-float, so the roundtrip is exact.
+ # Regression test for GH-50378 (values were previously decoded as raw uint16
bits)
+ df <- tibble::tibble(x = c(1, 2, 3.5, -0.25, 1024, NA))
+ result <- df |>
+ arrow_table() |>
+ mutate(x = cast(x, float16())) |>
+ collect()
+ expect_identical(result$x, df$x)
+})
