You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kevingurney (via GitHub)" <gi...@apache.org> on 2023/08/30 19:14:56 UTC
[GitHub] [arrow] kevingurney opened a new pull request, #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
kevingurney opened a new pull request, #37475:
URL: https://github.com/apache/arrow/pull/37475
### Rationale for this change
Currently, `arrow.tabular.Schema` supports indexing by `Field` name. However, `arrow.tabular.RecordBatch` does not.
This pull request adds the ability to index columns in a `RecordBatch` by `Field` name.
### What changes are included in this PR?
1. Added support for indexing columns in a `RecordBatch` by `Field` name via the `column` method.
**Example**
```matlab
>> recordBatch = arrow.tabular.RecordBatch.fromArrays(...
arrow.array([1, 2, 3]), ...
arrow.array(["A", "B", "C"]), ...
arrow.array([true, false, true]), ...
ColumnNames=["A", "B", "C"] ...
)
recordBatch =
A: [
1,
2,
3
]
B: [
"A",
"B",
"C"
]
C: [
true,
false,
true
]
>> recordBatch.column("B")
ans =
[
"A",
"B",
"C"
]
>> recordBatch.column("C")
ans =
[
true,
false,
true
]
```
2. Removed comments about vectorizing `field` method of `Schema` and `column` method of `RecordBatch`. After further consideration, we believe it would make more sense to only allow these methods to accept scalar inputs. We could revisit support for vectorization if we overload the parenthesis operator (e.g. `recordBatch(rows, columns)`) in the future to return another `RecordBatch`/`Schema` that only includes the selected columns/fields.
3. Fixed typo in `tSchema.m`.
### Are these changes tested?
Yes.
1. Added tests for indexing by column name using the `column` method to `tRecordBatch.m`.
### Are there any user-facing changes?
Yes.
1. Users can now index `RecordBatch` columns by name using the syntax `column(name)`.
### Future Directions
1. Consider overloading parentheses-based indexing on `RecordBatch` and `Schema`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney commented on pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #37475:
URL: https://github.com/apache/arrow/pull/37475#issuecomment-1699774120
+1
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310735584
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
Review Comment:
can be accessed using a field name -> can be accessed by name
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
+ % can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "", "C"] ...
+ );
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("");
+ testCase.verifyEqual(actual, expected)
+ end
+
+ function GetColumnByNameWithWhitespace(testCase)
+ % Verify that a column whose Field name contains only whitespace
+ % characters can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=[" ", " ", " "] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function ErrorIfColumnNameDoesNotExist(testCase)
+ % Verify that an error is thrown when trying to access a column
+ % with a Field name that is not part of the Schema of the RecordBatch.
Review Comment:
field name -> name
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310727987
##########
matlab/src/matlab/+arrow/+tabular/RecordBatch.m:
##########
@@ -54,13 +54,17 @@
function arrowArray = column(obj, idx)
import arrow.internal.validate.*
- idx = index.numeric(idx, "int32");
- % TODO: Consider vectorizing column() in the future to support
- % extracting multiple columns at once.
- validateattributes(idx, "int32", "scalar");
-
- args = struct(Index=idx);
- [proxyID, typeID] = obj.Proxy.getColumnByIndex(args);
+ idx = index.numericOrString(idx, "int32");
+
+ if isnumeric(idx)
+ validateattributes(idx, "int32", "scalar");
+ args = struct(Index=idx);
+ [proxyID, typeID] = obj.Proxy.getColumnByIndex(args);
+ else
+ validateattributes(idx, "string", "scalar");
Review Comment:
Same comment as above. You could use an `arguments` block.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310754835
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
+ % can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "", "C"] ...
+ );
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("");
+ testCase.verifyEqual(actual, expected)
+ end
+
+ function GetColumnByNameWithWhitespace(testCase)
+ % Verify that a column whose Field name contains only whitespace
Review Comment:
Fixed.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310729344
##########
matlab/src/matlab/+arrow/+tabular/Schema.m:
##########
@@ -48,14 +48,10 @@
idx = index.numericOrString(idx, "int32");
if isnumeric(idx)
- % TODO: Consider vectorizing field() to support extracting
Review Comment:
Could use an `arguments` block here as well.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310727987
##########
matlab/src/matlab/+arrow/+tabular/RecordBatch.m:
##########
@@ -54,13 +54,17 @@
function arrowArray = column(obj, idx)
import arrow.internal.validate.*
- idx = index.numeric(idx, "int32");
- % TODO: Consider vectorizing column() in the future to support
- % extracting multiple columns at once.
- validateattributes(idx, "int32", "scalar");
-
- args = struct(Index=idx);
- [proxyID, typeID] = obj.Proxy.getColumnByIndex(args);
+ idx = index.numericOrString(idx, "int32");
+
+ if isnumeric(idx)
+ validateattributes(idx, "int32", "scalar");
+ args = struct(Index=idx);
+ [proxyID, typeID] = obj.Proxy.getColumnByIndex(args);
+ else
+ validateattributes(idx, "string", "scalar");
Review Comment:
Same comment as above. You could use an `arguments` block.
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
+ % can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "", "C"] ...
+ );
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("");
+ testCase.verifyEqual(actual, expected)
+ end
+
+ function GetColumnByNameWithWhitespace(testCase)
+ % Verify that a column whose Field name contains only whitespace
+ % characters can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=[" ", " ", " "] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function ErrorIfColumnNameDoesNotExist(testCase)
+ % Verify that an error is thrown when trying to access a column
+ % with a Field name that is not part of the Schema of the RecordBatch.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ % Matching should be case sensitive.
+ name = "a";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = "aA";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = "D";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = "";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = " ";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+ end
+
+ function ErrorIfAmbiguousColumnName(testCase)
+ % Verify that an error is thrown when trying to access a column
+ % with a name that is ambiguous / occurs more than once in the
+ % Schema of the RecordBatch.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ arrow.array([days(1), days(2), days(3)]), ...
+ ColumnNames=["A", "A", "B", "B"] ...
+ );
+
+ name = "A";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = "B";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+ end
+
+ function GetColumnByNameWithChar(testCase)
+ % Verify that the column method works when supplied a char
+ % vector as input.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["", "B", "123"] ...
+ );
+
+ % Should match the first column whose name is the
+ % empty string ("").
+ name = char.empty(0, 0);
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(name);
+ testCase.verifyEqual(actual, expected);
+
+ name = char.empty(0, 1);
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(name);
+ testCase.verifyEqual(actual, expected);
+
+ name = char.empty(1, 0);
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(name);
+ testCase.verifyEqual(actual, expected);
+
+ % Should match the second column whose name is "B".
+ name = 'B';
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column(name);
+ testCase.verifyEqual(actual, expected);
+
+ % Should match the third field whose name is "123".
Review Comment:
field -> column
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310732596
##########
matlab/src/matlab/+arrow/+tabular/RecordBatch.m:
##########
@@ -54,13 +54,17 @@
function arrowArray = column(obj, idx)
import arrow.internal.validate.*
- idx = index.numeric(idx, "int32");
- % TODO: Consider vectorizing column() in the future to support
- % extracting multiple columns at once.
- validateattributes(idx, "int32", "scalar");
-
- args = struct(Index=idx);
- [proxyID, typeID] = obj.Proxy.getColumnByIndex(args);
+ idx = index.numericOrString(idx, "int32");
+
+ if isnumeric(idx)
+ validateattributes(idx, "int32", "scalar");
Review Comment:
On second thought, we should enhance the `index.numericOrString` function to accept a `AcceptNonScalar` name-value pair that defaults to `true`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310754513
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
Review Comment:
Fixed.
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
Review Comment:
Fixed.
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
+ % can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "", "C"] ...
+ );
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("");
+ testCase.verifyEqual(actual, expected)
+ end
+
+ function GetColumnByNameWithWhitespace(testCase)
+ % Verify that a column whose Field name contains only whitespace
+ % characters can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=[" ", " ", " "] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function ErrorIfColumnNameDoesNotExist(testCase)
+ % Verify that an error is thrown when trying to access a column
+ % with a Field name that is not part of the Schema of the RecordBatch.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ % Matching should be case sensitive.
+ name = "a";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = "aA";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = "D";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = "";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = " ";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+ end
+
+ function ErrorIfAmbiguousColumnName(testCase)
+ % Verify that an error is thrown when trying to access a column
+ % with a name that is ambiguous / occurs more than once in the
+ % Schema of the RecordBatch.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ arrow.array([days(1), days(2), days(3)]), ...
+ ColumnNames=["A", "A", "B", "B"] ...
+ );
+
+ name = "A";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+
+ name = "B";
+ testCase.verifyError(@() recordBatch.column(name), "arrow:tabular:schema:AmbiguousFieldName");
+ end
+
+ function GetColumnByNameWithChar(testCase)
+ % Verify that the column method works when supplied a char
+ % vector as input.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["", "B", "123"] ...
+ );
+
+ % Should match the first column whose name is the
+ % empty string ("").
+ name = char.empty(0, 0);
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(name);
+ testCase.verifyEqual(actual, expected);
+
+ name = char.empty(0, 1);
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(name);
+ testCase.verifyEqual(actual, expected);
+
+ name = char.empty(1, 0);
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(name);
+ testCase.verifyEqual(actual, expected);
+
+ % Should match the second column whose name is "B".
+ name = 'B';
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column(name);
+ testCase.verifyEqual(actual, expected);
+
+ % Should match the third field whose name is "123".
Review Comment:
Fixed.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #37475:
URL: https://github.com/apache/arrow/pull/37475#issuecomment-1703695000
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 798132ce65ee4d7fb902e737e2e7832bf58faaaa.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/16436444272) has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310754384
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
+ % can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "", "C"] ...
+ );
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("");
+ testCase.verifyEqual(actual, expected)
+ end
+
+ function GetColumnByNameWithWhitespace(testCase)
+ % Verify that a column whose Field name contains only whitespace
+ % characters can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=[" ", " ", " "] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function ErrorIfColumnNameDoesNotExist(testCase)
+ % Verify that an error is thrown when trying to access a column
+ % with a Field name that is not part of the Schema of the RecordBatch.
Review Comment:
Fixed.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310734344
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
+ % can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "", "C"] ...
+ );
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("");
+ testCase.verifyEqual(actual, expected)
+ end
+
+ function GetColumnByNameWithWhitespace(testCase)
+ % Verify that a column whose Field name contains only whitespace
+ % characters can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=[" ", " ", " "] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function ErrorIfColumnNameDoesNotExist(testCase)
+ % Verify that an error is thrown when trying to access a column
+ % with a Field name that is not part of the Schema of the RecordBatch.
Review Comment:
`Field` -> `field`
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310727723
##########
matlab/src/matlab/+arrow/+tabular/RecordBatch.m:
##########
@@ -54,13 +54,17 @@
function arrowArray = column(obj, idx)
import arrow.internal.validate.*
- idx = index.numeric(idx, "int32");
- % TODO: Consider vectorizing column() in the future to support
- % extracting multiple columns at once.
- validateattributes(idx, "int32", "scalar");
-
- args = struct(Index=idx);
- [proxyID, typeID] = obj.Proxy.getColumnByIndex(args);
+ idx = index.numericOrString(idx, "int32");
+
+ if isnumeric(idx)
+ validateattributes(idx, "int32", "scalar");
Review Comment:
You can remove this `validateattributes` call if you include an `arguments` block:
```matlab
function arrowArray = column(obj, idx)
arguments
obj
idx(1, 1)
end
...
end
```
That way you don't need to re-validate the type of `idx`
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney merged pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney merged PR #37475:
URL: https://github.com/apache/arrow/pull/37475
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310729344
##########
matlab/src/matlab/+arrow/+tabular/Schema.m:
##########
@@ -48,14 +48,10 @@
idx = index.numericOrString(idx, "int32");
if isnumeric(idx)
- % TODO: Consider vectorizing field() to support extracting
Review Comment:
Could use an `arguments` block here as well.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310734344
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
+ % can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "", "C"] ...
+ );
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("");
+ testCase.verifyEqual(actual, expected)
+ end
+
+ function GetColumnByNameWithWhitespace(testCase)
+ % Verify that a column whose Field name contains only whitespace
+ % characters can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=[" ", " ", " "] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column(" ");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function ErrorIfColumnNameDoesNotExist(testCase)
+ % Verify that an error is thrown when trying to access a column
+ % with a Field name that is not part of the Schema of the RecordBatch.
Review Comment:
"Field" -> "field"
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310737133
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
+ % can be accessed using the column() method.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "", "C"] ...
+ );
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("");
+ testCase.verifyEqual(actual, expected)
+ end
+
+ function GetColumnByNameWithWhitespace(testCase)
+ % Verify that a column whose Field name contains only whitespace
Review Comment:
field name -> name
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310735263
##########
matlab/test/arrow/tabular/tRecordBatch.m:
##########
@@ -223,6 +223,169 @@ function SchemaNoSetter(tc)
"MATLAB:class:SetProhibited");
end
+ function GetColumnByName(testCase)
+ % Verify that columns can be accessed using a field name.
+ recordBatch = arrow.tabular.RecordBatch.fromArrays(...
+ arrow.array([1, 2, 3]), ...
+ arrow.array(["A", "B", "C"]), ...
+ arrow.array([true, false, true]), ...
+ ColumnNames=["A", "B", "C"] ...
+ );
+
+ expected = arrow.array([1, 2, 3]);
+ actual = recordBatch.column("A");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array(["A", "B", "C"]);
+ actual = recordBatch.column("B");
+ testCase.verifyEqual(actual, expected);
+
+ expected = arrow.array([true, false, true]);
+ actual = recordBatch.column("C");
+ testCase.verifyEqual(actual, expected);
+ end
+
+ function GetColumnByNameWithEmptyString(testCase)
+ % Verify that a column whose Field name is the empty string ("")
Review Comment:
Field name -> name
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney commented on a diff in pull request #37475: GH-37473: [MATLAB] Add support for indexing `RecordBatch` columns by `Field` name
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #37475:
URL: https://github.com/apache/arrow/pull/37475#discussion_r1310760769
##########
matlab/src/matlab/+arrow/+tabular/RecordBatch.m:
##########
@@ -54,13 +54,17 @@
function arrowArray = column(obj, idx)
import arrow.internal.validate.*
- idx = index.numeric(idx, "int32");
- % TODO: Consider vectorizing column() in the future to support
- % extracting multiple columns at once.
- validateattributes(idx, "int32", "scalar");
-
- args = struct(Index=idx);
- [proxyID, typeID] = obj.Proxy.getColumnByIndex(args);
+ idx = index.numericOrString(idx, "int32");
+
+ if isnumeric(idx)
+ validateattributes(idx, "int32", "scalar");
Review Comment:
That's a good point. I've captured this in #37477.
##########
matlab/src/matlab/+arrow/+tabular/RecordBatch.m:
##########
@@ -54,13 +54,17 @@
function arrowArray = column(obj, idx)
import arrow.internal.validate.*
- idx = index.numeric(idx, "int32");
- % TODO: Consider vectorizing column() in the future to support
- % extracting multiple columns at once.
- validateattributes(idx, "int32", "scalar");
-
- args = struct(Index=idx);
- [proxyID, typeID] = obj.Proxy.getColumnByIndex(args);
+ idx = index.numericOrString(idx, "int32");
+
+ if isnumeric(idx)
+ validateattributes(idx, "int32", "scalar");
Review Comment:
Marking this as resolved.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org