AlenkaF commented on code in PR #40365:
URL: https://github.com/apache/arrow/pull/40365#discussion_r1517481549
##########
cpp/src/arrow/record_batch_test.cc:
##########
@@ -705,17 +705,12 @@ TEST_F(TestRecordBatch, ToTensorSupportedNaN) {
std::vector<int64_t> shape = {9, 2};
const int64_t f32_size = sizeof(float);
std::vector<int64_t> f_strides = {f32_size, f32_size * shape[0]};
- std::vector<float> f_values = {
- static_cast<float>(NAN), 2, 3, 4, 5, 6, 7, 8, 9, 10, 20, 30, 40,
- static_cast<float>(NAN), 60, 70, 80, 90};
- auto data = Buffer::Wrap(f_values);
-
- std::shared_ptr<Tensor> tensor_expected;
- ASSERT_OK_AND_ASSIGN(tensor_expected, Tensor::Make(float32(), data, shape,
f_strides));
+ std::shared_ptr<Tensor> tensor_expected = TensorFromJSON(
+ float32(), shape,
+ "[NaN, 2, 3, 4, 5, 6, 7, 8, 9, 10, 20, 30, 40, NaN, 60, 70, 80, 90]",
f_strides);
Review Comment:
I would agree with you @bkietz on the topic of the nested Arrow arrays. But
in the case of Arrow Tensors the difference between logical and physical layout
already makes things hard to visualise and Nd json would make it even more
complicated. I propose to leave it as is with explicitly creating values as
they are stored. If we later see the benefit of having Nd json where shape is
inferred, I am happy to do a follow-up adding a second, "RowMajor
TensorFromJSON" for example (to make things a bit clearer).
Thank you @rok for the info about `ComputeStrides` helper! Will use it and
add permutation as a possible input also.
--
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]
