thisisnic commented on code in PR #50451:
URL: https://github.com/apache/arrow/pull/50451#discussion_r3559277635
##########
r/src/r_to_arrow.cpp:
##########
@@ -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");
Review Comment:
That bit there is enabling the R to Arrow direction, but the original issue
was hitting the Arrow to R direction. My assumption is that we chose not to
implement it because it's not really useful for most R users to have it? Like
float16s are weird because you can't represent as many numbers, e.g.
```r
> arrow_array(c(1.1, 2.2, 3.3), float16())
Array
<halffloat>
[
1.099609375,
2.19921875,
3.30078125
]
```
And actually, they don't roundtrip nicely at all
```r
> arrow_array(c(1.1, 2.2, 3.3), float16())$as_vector()
[1] 1.099609 2.199219 3.300781
```
Though you could say 1.1 was never a float16 in the first place. Maybe I
should just ditch the R to Arrow stuff; it felt like making a more complete PR,
but we might just be handing users a footgun, as I can image finding it easy to
go "this will take up less storage, let's use it".
--
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]
