niyue commented on PR #13041: URL: https://github.com/apache/arrow/pull/13041#issuecomment-1124612500
> By the way, this PR addresses `RecordBatchFileReader` but not `RecordBatchReader`. Do you plan to do that as well? I tried adding the pyarrow API for `RecordBatchReader` locally previously, but I found if I want to test the new pyarrow API for `RecordBatchReader`, I probably have to add the corresponding API implementation in C++ `RecordBatchStreamReader` so that I can unit test it in a meaningful way since it seems only `RecordBatchStreamReader` and `RecordBatchFileReader` provides the capability to read record batch custom metadata (is this correct?). And since this PR is more about adding new pyarrow API for existing C++ capability, I would like to make non necessary C++ API change less in this PR. 1) if there is other way to unit test pyarrow `RecordBatchReader.read_next_batch_with_metadata` API besides implementing it in `RecordBatchStreamReader`? 2) do you recommend to put all these changes together in single PR or keep them as separated PRs? I initially submitted this issue as a pyarrow enhancement PR, but I am okay to include more API changes if it is preferred. -- 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-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
