GH-37290: [MATLAB] Add `arrow.array.Time32Array` class #37315

sgilmore10 · 2023-08-22T18:08:48Z

Rationale for this change

Now that the arrow.type.Time32Type class has been added to the MATLAB Interface (#37250), we can add the arrow.array.Time32Array class.

What changes are included in this PR?

Added a new class arrow.array.Time32Array. It has the following read-only properties: Type and Length. It has the following methods: fromMATLAB(), toMATLAB(), and duration(). For reference, duration arrays in MATLAB represent lengths of time in fixed-length units. Here's a link to the documentation.
Added a new class called arrow.type.traits.Time32Traits.

Example Usage

>> times = seconds([100 120 NaN 200])

times = 

  1×4 duration array

   100 sec   120 sec   NaN sec   200 sec

>> time32Array = arrow.array.Time32Array.fromMATLAB(times)

time32Array = 

[
  00:01:40,
  00:02:00,
  null,
  00:03:20
]

>> roundtrip = toMATLAB(time32Array)

roundtrip = 

  4×1 duration array

   100 sec
   120 sec
   NaN sec
   200 sec

You can also specify the TimeUnit to use via a name-value pair. TimeUnit can either be "Second" (default) or "Milisecond".

>> times = seconds([100 120 NaN 200]);
>> time32Array = arrow.array.Time32Array.fromMATLAB(times, TimeUnit="Millisecond")

time32Array = 

[
  00:01:40.000,
  00:02:00.000,
  null,
  00:03:20.000
]

Are these changes tested?

Yes. Added two new test classes: tTime32Array.m and tTime32Traits.m.

Are there any user-facing changes?

Yes, users can now create arrow.array.Time32Arrays from MATLAB durations via the static fromMATLAB method on arrow.array.Time32Array.

Future Directions

Add arrow.array.Time64Array
Add arrow.type.Date32Type and arrow.array.Date32Array
Add arrow.type.Date64Type and arrow.array.Date64Array

Closes: [MATLAB] Add arrow.array.Time32Array class #37290

kevingurney

Looks good overall - thank you!

matlab/src/cpp/arrow/matlab/array/proxy/time32_array.cc

matlab/src/matlab/+arrow/+array/Time32Array.m

matlab/test/arrow/array/tTime32Array.m

kou

+1

conbench-apache-arrow · 2023-08-23T18:46:55Z

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 0781f13.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

…dBatch` errors if `index` refers to an `arrow.array.Time32Array` column (#37347) ### Rationale for this change The `column(index)` method of `arrow.tabular.RecordBatch` errors if `index` refers to an `arrow.array.Time32Array` column. ```matlab >> string = arrow.array(["A", "B", "C"]); >> time32 = arrow.array.Time32Array.fromMATLAB(seconds(1:3)); >> rb = arrow.tabular.RecordBatch.fromArrays(string, time32, ColumnNames=["String", "Time32"]) rb = String: [ "A", "B", "C" ] Time32: [ 00:00:01, 00:00:02, 00:00:03 ] >> time32Column = rb.column(2) Error using . Unsupported DataType: time32[s] Error in arrow.tabular.RecordBatch/column (line 63) [proxyID, typeID] = obj.Proxy.getColumnByIndex(args); ``` `column(index)` is throwing this error because the case for `Time32` was not added to `arrow::matlab::array::proxy::wrap(arrowArray)` in #37315. `column(index)` calls this function to create proxy objects around existing arrow arrays. Adding the case for `Time32` will resolve this bug. ### What changes are included in this PR? 1. Updated `arrow::Result<proxy::Array> wrap(const std::shared_ptr<arrow::Array>& array)` to handle wrapping `arrow::Time32Array` instances within `proxy::Time32Array`s. 2. Added a new test utility called `arrow.internal.test.tabular.createAllSupportedArrayTypes`, which returns two cell arrays: `arrowArrays` and `matlabData`. The `arrowArrays` cell array contains one instance of each concrete subclass of `arrow.array.Array`. The `matlabData` cell array contains the MATLAB arrays used to generate each array in `arrowArrays`. This utility can be used to create an `arrow.array.RecordBatch` that contains all the arrow array types that are supported by the MATLAB interface. ### Are these changes tested? Yes. Updated the `fromArrays` test cases in `tRecordBatch` to verify the `column(index)` method of `arrow.type.RecordBatch` supports extracting columns of any arrow type (supported by the MATLAB Interface). ### Are there any user-facing changes? Yes. Fixed a bug in `arrow.tabular.RecordBatch`. ### Future Directions 1. #37345 * Closes: #37340 Authored-by: Sarah Gilmore <[email protected]> Signed-off-by: Kevin Gurney <[email protected]>

…37315) ### Rationale for this change Now that the `arrow.type.Time32Type` class has been added to the MATLAB Interface (apache#37250), we can add the `arrow.array.Time32Array` class. ### What changes are included in this PR? 1. Added a new class `arrow.array.Time32Array`. It has the following read-only properties: `Type` and `Length`. It has the following methods: `fromMATLAB()`, `toMATLAB()`, and `duration()`. For reference, `duration` arrays in MATLAB represent lengths of time in fixed-length units. Here's a [link](https://www.mathworks.com/help/matlab/ref/duration.html) to the documentation. 2. Added a new class called `arrow.type.traits.Time32Traits`. **Example Usage** ```matlab >> times = seconds([100 120 NaN 200]) times = 1×4 duration array 100 sec 120 sec NaN sec 200 sec >> time32Array = arrow.array.Time32Array.fromMATLAB(times) time32Array = [ 00:01:40, 00:02:00, null, 00:03:20 ] >> roundtrip = toMATLAB(time32Array) roundtrip = 4×1 duration array 100 sec 120 sec NaN sec 200 sec ``` You can also specify the `TimeUnit` to use via a name-value pair. `TimeUnit` can either be `"Second"` (default) or `"Milisecond"`. ```matlab >> times = seconds([100 120 NaN 200]); >> time32Array = arrow.array.Time32Array.fromMATLAB(times, TimeUnit="Millisecond") time32Array = [ 00:01:40.000, 00:02:00.000, null, 00:03:20.000 ] ``` ### Are these changes tested? Yes. Added two new test classes: `tTime32Array.m` and `tTime32Traits.m`. ### Are there any user-facing changes? Yes, users can now create `arrow.array.Time32Array`s from MATLAB `duration`s via the static `fromMATLAB` method on `arrow.array.Time32Array`. ### Future Directions 1. Add `arrow.array.Time64Array` 2. Add `arrow.type.Date32Type` and `arrow.array.Date32Array` 4. Add `arrow.type.Date64Type` and `arrow.array.Date64Array` * Closes: apache#37290 Authored-by: Sarah Gilmore <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>

….RecordBatch` errors if `index` refers to an `arrow.array.Time32Array` column (apache#37347) ### Rationale for this change The `column(index)` method of `arrow.tabular.RecordBatch` errors if `index` refers to an `arrow.array.Time32Array` column. ```matlab >> string = arrow.array(["A", "B", "C"]); >> time32 = arrow.array.Time32Array.fromMATLAB(seconds(1:3)); >> rb = arrow.tabular.RecordBatch.fromArrays(string, time32, ColumnNames=["String", "Time32"]) rb = String: [ "A", "B", "C" ] Time32: [ 00:00:01, 00:00:02, 00:00:03 ] >> time32Column = rb.column(2) Error using . Unsupported DataType: time32[s] Error in arrow.tabular.RecordBatch/column (line 63) [proxyID, typeID] = obj.Proxy.getColumnByIndex(args); ``` `column(index)` is throwing this error because the case for `Time32` was not added to `arrow::matlab::array::proxy::wrap(arrowArray)` in apache#37315. `column(index)` calls this function to create proxy objects around existing arrow arrays. Adding the case for `Time32` will resolve this bug. ### What changes are included in this PR? 1. Updated `arrow::Result<proxy::Array> wrap(const std::shared_ptr<arrow::Array>& array)` to handle wrapping `arrow::Time32Array` instances within `proxy::Time32Array`s. 2. Added a new test utility called `arrow.internal.test.tabular.createAllSupportedArrayTypes`, which returns two cell arrays: `arrowArrays` and `matlabData`. The `arrowArrays` cell array contains one instance of each concrete subclass of `arrow.array.Array`. The `matlabData` cell array contains the MATLAB arrays used to generate each array in `arrowArrays`. This utility can be used to create an `arrow.array.RecordBatch` that contains all the arrow array types that are supported by the MATLAB interface. ### Are these changes tested? Yes. Updated the `fromArrays` test cases in `tRecordBatch` to verify the `column(index)` method of `arrow.type.RecordBatch` supports extracting columns of any arrow type (supported by the MATLAB Interface). ### Are there any user-facing changes? Yes. Fixed a bug in `arrow.tabular.RecordBatch`. ### Future Directions 1. apache#37345 * Closes: apache#37340 Authored-by: Sarah Gilmore <[email protected]> Signed-off-by: Kevin Gurney <[email protected]>

sgilmore10 added 10 commits August 21, 2023 17:02

Add proxy Time32Array class

64ecb71

Register Time32Array proxy

eac0e29

Add TIME32 case to switch statement in type::proxy::wrap()

fa01bbb

Add Time32Traits class

02680a2

Add case for arrow.type.ID.Time32 to arrow.type.traits.traits function

75ef71c

Fix typo

5296b84

Add arrow.type.Time32Array class

1b90017

Fix MatlabClassName in Time32Traits

afb0965

Add unit tests for Time32Traits

dde9713

Add unit tests for Time32Array

950e169

sgilmore10 marked this pull request as ready for review August 22, 2023 18:08

sgilmore10 requested review from kevingurney and kou as code owners August 22, 2023 18:08

github-actions bot added Component: MATLAB awaiting review Awaiting review labels Aug 22, 2023

kevingurney requested changes Aug 22, 2023

View reviewed changes

github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Aug 22, 2023

sgilmore10 added 7 commits August 22, 2023 16:01

Rename timestamp* vars to time32* vars

43b22ee

Fix comment

e3c355a

Rename convertDurationToNumber to convertDurationToTicks

2ccab68

Rename variable 'time' to 'ticks'

1b715e7

Use verifyInstanceOf

a849a44

Fix typos in comments

56552a6

Rephrase comment in test

80d33d5

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Aug 22, 2023

kou approved these changes Aug 23, 2023

View reviewed changes

kou merged commit 0781f13 into apache:main Aug 23, 2023
8 of 9 checks passed

kou removed the awaiting change review Awaiting change review label Aug 23, 2023

github-actions bot added the awaiting merge Awaiting merge label Aug 23, 2023

sgilmore10 mentioned this pull request Aug 23, 2023

[MATLAB] The column(index) method of arrow.tabular.RecordBatch errors if index refers to an arrow.array.Time32Array column #37340

Closed

sgilmore10 mentioned this pull request Aug 23, 2023

GH-37340: [MATLAB] The column(index) method of arrow.tabular.RecordBatch errors if index refers to an arrow.array.Time32Array column #37347

Merged

kevingurney deleted the GH-37290 branch August 30, 2023 00:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-37290: [MATLAB] Add `arrow.array.Time32Array` class #37315

GH-37290: [MATLAB] Add `arrow.array.Time32Array` class #37315

sgilmore10 commented Aug 22, 2023 •

edited

Loading

kevingurney left a comment

kou left a comment

conbench-apache-arrow bot commented Aug 23, 2023

GH-37290: [MATLAB] Add arrow.array.Time32Array class #37315

GH-37290: [MATLAB] Add arrow.array.Time32Array class #37315

Conversation

sgilmore10 commented Aug 22, 2023 • edited Loading

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Future Directions

kevingurney left a comment

Choose a reason for hiding this comment

kou left a comment

Choose a reason for hiding this comment

conbench-apache-arrow bot commented Aug 23, 2023

GH-37290: [MATLAB] Add `arrow.array.Time32Array` class #37315

GH-37290: [MATLAB] Add `arrow.array.Time32Array` class #37315

sgilmore10 commented Aug 22, 2023 •

edited

Loading