jorisvandenbossche commented on code in PR #40803:
URL: https://github.com/apache/arrow/pull/40803#discussion_r1540664261
##########
cpp/src/arrow/record_batch.cc:
##########
@@ -334,42 +347,51 @@ Result<std::shared_ptr<Tensor>>
RecordBatch::ToTensor(MemoryPool* pool) const {
result_type = result_field->type();
}
+ // Check if result_type is signed or unsigned integer and null_to_nan is set
to true
+ // Then all columns should be promoted to float type
+ if (is_integer(result_type->id()) && null_to_nan) {
+ ARROW_ASSIGN_OR_RAISE(
+ result_field,
+ result_field->MergeWith(field(result_field->name(), float16()),
options));
Review Comment:
I was going to suggest it would be good to add a test that covers the fact
that we need to use float32 here, and not accidentally use float16 (i.e. you
changed this now, but the tests were not failing because of it or you didn't
update any test).
But, for the case of (int8, float16), that's actually not supported anyway,
as you already raise an error for that combo earlier in the code because of not
supporting float16 with mixed types
--
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]
