jorisvandenbossche commented on code in PR #40365:
URL: https://github.com/apache/arrow/pull/40365#discussion_r1515742514
##########
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:
For testing purposes, I think it might be better to very explicitly create
the values as they are stored in the tensor, instead of implicitly reshuffling
them (which is essentially what would happen if you present the data as a Nd
json array, but then combined that with non-trivial permutation or strides).
IMO that would make the tests only harder to interpret, so I would go for the
simpler option (i.e. current PR)
--
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]
