-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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.
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
GH-44923: [MATLAB] Add IPC RecordBatchStreamReader
MATLAB class
#45068
base: main
Are you sure you want to change the base?
Conversation
…le that is not an Arrow IPC Stream file.
matlab/src/cpp/arrow/matlab/io/ipc/proxy/record_batch_stream_reader.cc
Outdated
Show resolved
Hide resolved
matlab/src/cpp/arrow/matlab/io/ipc/proxy/record_batch_stream_reader.cc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thanks for working on this :)
Co-authored-by: Sarah Gilmore <[email protected]>
Thanks for the review @sgilmore10! Sorry for all of the typos! |
function tf = hasnext(obj) | ||
tf = obj.Proxy.hasNextRecordBatch(); | ||
end | ||
|
||
function tf = done(obj) | ||
tf = ~obj.Proxy.hasNextRecordBatch(); | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with MATLAB but does MATLAB require those APIs for iterator (or something)?
If we don't require this, it may be better that we don't provide them... If we don't have them, we can simplify our implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good question.
There is no specific API for iteration in MATLAB (i.e. there is no for each
style iteration). So, we were trying to emulate this behavior using a "while has more data"-style loop. There is precedent for this pattern of iteration being used elsewhere in MATLAB.
Does that answer your question?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Yes.
One more question. Is while record_batch = reader.read()
-style loop used in MATLAB too? If the style is also used in MATLAB, we may want to recommend the style for the MATLAB bindings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, no - MATLAB does not support variable assignment like while record_batch = reader.read()
inside of a while
loop expression, either. If that syntax was supported, I agree it would be preferable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
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?
arrow.io.ipc.RecordBatchStreamReader
MATLAB class.Are these changes tested?
Yes.
arrow/matlab/test/arrow/io/ipc/tRecordBatchStreamReader.m
.Are there any user-facing changes?
Yes.
arrow.io.ipc.RecordBatchStreamReader
objects to readRecordBatch
objects incrementally from an Arrow IPC Stream file.Notes
RecordBatchStreamReader
MATLAB class #44923