kevingurney commented on code in PR #38274:
URL: https://github.com/apache/arrow/pull/38274#discussion_r1370349586
##########
matlab/src/matlab/+arrow/recordBatch.m:
##########
@@ -16,7 +16,7 @@
% permissions and limitations under the License.
function rb = recordBatch(T)
Review Comment:
Since you renamed the first input argument to `matlabTable` in the
`arguments` block, you will also need to rename `T` to `matlabTable`.
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -494,9 +494,23 @@ function TestIsEqualFalse(testCase)
% Call isequal with more than two arguments
testCase.verifyFalse(isequal(rb1, rb2, rb3, rb4));
end
-
-
+ function testArrowRecordBatchNoArgs()
+ % Call arrow.recordBatch with no arguments
+ actual = arrow.recordBatch();
+
+ % Create an empty expected RecordBatch
+ schema = arrow.schema();
+ expected = arrow.tabular.RecordBatch(schema, {});
Review Comment:
The `arrow.tabular.RecordBatch` constructor does not support any syntax that
accepts an `arrow.tabular.Schema` and an empty `cell` array (i.e. `{}`).
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -494,9 +494,23 @@ function TestIsEqualFalse(testCase)
% Call isequal with more than two arguments
testCase.verifyFalse(isequal(rb1, rb2, rb3, rb4));
end
-
-
+ function testArrowRecordBatchNoArgs()
+ % Call arrow.recordBatch with no arguments
Review Comment:
The indentation of the code in this test if off.
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -494,9 +494,23 @@ function TestIsEqualFalse(testCase)
% Call isequal with more than two arguments
testCase.verifyFalse(isequal(rb1, rb2, rb3, rb4));
end
-
-
+ function testArrowRecordBatchNoArgs()
+ % Call arrow.recordBatch with no arguments
+ actual = arrow.recordBatch();
+
+ % Create an empty expected RecordBatch
+ schema = arrow.schema();
+ expected = arrow.tabular.RecordBatch(schema, {});
+
+ % Use isequal to check if the actual and expected are equal
+ if isequal(expected, actual)
Review Comment:
You should be able to just use `testCase.verifyEqual(actual, expected)` here.
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -494,9 +494,23 @@ function TestIsEqualFalse(testCase)
% Call isequal with more than two arguments
testCase.verifyFalse(isequal(rb1, rb2, rb3, rb4));
end
-
-
+ function testArrowRecordBatchNoArgs()
Review Comment:
Could you please use `UpperCamelCase` for the test name to be consistent
with the rest of the test cases in this file? i.e. `TestArrowRecordBatchNoArgs`
instead of `testArrowRecordBatchNoArgs`.
##########
matlab/src/matlab/+arrow/recordBatch.m:
##########
@@ -16,7 +16,7 @@
% permissions and limitations under the License.
function rb = recordBatch(T)
Review Comment:
Also, the rest of the code in this function needs to be updated to use
`matlabTable` instead of `T`. For example:
```matlab
columnNames = string(T.Properties.VariableNames);
```
would become:
```matlab
columnNames = string(matlabTable.Properties.VariableNames);
```
--
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]
