New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GH-44923: [MATLAB] Add IPC `RecordBatchStreamReader` MATLAB class #45068

Merged

kevingurney merged 20 commits into apache:main from mathworks:GH-44923

Dec 23, 2024

Member

kevingurney commented Dec 18, 2024 •

edited

Loading

Rationale for this change

To enable support for the IPC Streaming format in the MATLAB interface, we should add a RecordBatchStreamReader class.

This is a followup to #44922

What changes are included in this PR?

Added a new arrow.io.ipc.RecordBatchStreamReader MATLAB class.

Are these changes tested?

Yes.

Added new MATLAB test suite arrow/matlab/test/arrow/io/ipc/tRecordBatchStreamReader.m.

Are there any user-facing changes?

Yes.

Users can now create arrow.io.ipc.RecordBatchStreamReader objects to read RecordBatch objects incrementally from an Arrow IPC Stream file.

Notes

Thank you @sgilmore10 for your help with this pull request!

GitHub Issue: [MATLAB] Add IPC RecordBatchStreamReader MATLAB class #44923

kevingurney added 9 commits

December 4, 2024 15:19


          Register RecordBatchStreamReader proxy with proxy factory.

6db726a


          Partial implementation for C++ RecordBatchStreamReader class.


          Fix compilation errors.

75902fe


          Basic MATLAB implementation of RecordBatchStreamReader class.

55dee07


          Check for NULL record batch in hasNextRecordBatch.

a2d5673


          Handle end of stream case when calling readRecordBatch.

8b5574f


          Implement readTable, hasnext, and done methods.

fab1063


          Add basic MATLAB tests for RecordBatchStreamReader class.

432eae8


          Add test which verifies that an error is thrown if provided with a fi…

c2a9d2e

…le that is not an Arrow IPC Stream file.

github-actions bot added Component: MATLAB awaiting committer review labels

kevingurney added 2 commits

December 18, 2024 14:36


          Fix formatting in factory.cc based on CI failures.

63f6314


          Fix formatting in record_batch_stream_reader.cc based on CI failures.

481a095

sgilmore10 reviewed

View reviewed changes

matlab/src/cpp/arrow/matlab/io/ipc/proxy/record_batch_stream_reader.cc Outdated Show resolved Hide resolved

sgilmore10 reviewed

View reviewed changes

matlab/src/cpp/arrow/matlab/io/ipc/proxy/record_batch_stream_reader.cc Outdated Show resolved Hide resolved

sgilmore10 reviewed

View reviewed changes

matlab/test/arrow/io/ipc/tRecordBatchStreamReader.m Outdated Show resolved Hide resolved

sgilmore10 reviewed

View reviewed changes

matlab/test/arrow/io/ipc/tRecordBatchStreamReader.m Outdated Show resolved Hide resolved

sgilmore10 reviewed

View reviewed changes

matlab/test/arrow/io/ipc/tRecordBatchStreamReader.m Outdated Show resolved Hide resolved

sgilmore10 reviewed

View reviewed changes

matlab/test/arrow/io/ipc/tRecordBatchStreamReader.m Outdated Show resolved Hide resolved

sgilmore10 reviewed

View reviewed changes

matlab/test/arrow/io/ipc/tRecordBatchStreamReader.m Outdated Show resolved Hide resolved

sgilmore10 requested changes

View reviewed changes

Member

sgilmore10 left a comment

Looks good! Thanks for working on this :)

github-actions bot added awaiting changes and removed awaiting committer review labels


          Add error ID constant for arrow:io:ipc:EndOfStream.

9531e70

github-actions bot added awaiting change review awaiting changes and removed awaiting changes awaiting change review labels


          Update test case name and comment for testing for arrow:io:ipc:EndOfS…

2c49160

…tream error.

github-actions bot removed the awaiting changes label

github-actions bot added awaiting change review awaiting changes and removed awaiting change review labels


          Fix incorrect comment for hasnext method test case.

4c6af41

Co-authored-by: Sarah Gilmore <[email protected]>

github-actions bot added awaiting change review awaiting changes and removed awaiting changes awaiting change review labels

kevingurney added 2 commits

December 18, 2024 15:32


          Fix incorrect comments for done method test case.

380708e


          Fix incorrect error message ID being used when unable to open file fo…

2fccd18

…r read.

github-actions bot added awaiting change review and removed awaiting changes labels


          Alphabetize headers.

8f23362

Member Author

kevingurney commented Dec 18, 2024

Thanks for the review @sgilmore10! Sorry for all of the typos!

kevingurney marked this pull request as ready for review

December 18, 2024 20:35

kevingurney requested a review from kou as a code owner

December 18, 2024 20:35

kevingurney added 2 commits

December 18, 2024 15:51


          Fix linting errors.

1915da4


          Fix typo.

d3d5255

sgilmore10 approved these changes

View reviewed changes

kou reviewed

View reviewed changes

matlab/src/matlab/+arrow/+io/+ipc/RecordBatchStreamReader.m Show resolved Hide resolved

github-actions bot added awaiting changes and removed awaiting change review labels

kou approved these changes

View reviewed changes

Member

kou left a comment

+1

github-actions bot added awaiting merge and removed awaiting changes labels

Member Author

kevingurney commented Dec 23, 2024

+1

kevingurney merged commit 8b75373 into apache:main

16 checks passed

kevingurney removed the awaiting merge label

kevingurney deleted the GH-44923 branch

December 23, 2024 20:53

kevingurney mentioned this pull request

[MATLAB] Add IPC RecordBatchStreamReader MATLAB class #44923

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: MATLAB