[MATLAB] Implement featherwrite in terms of arrow.internal.io.feather.Writer
#37045
Comments
|
take |
kevingurney
pushed a commit
that referenced
this issue
Aug 7, 2023
…io.feather.Writer (#37047) ### Rationale for this change Now that #37043 is merged, we can re-implement `featherwrite` in terms of the new `arrow.internal.io.feather.Writer` class. Once this change is made, we can delete the legacy build infrastructure and featherwrite MEX code. ### What changes are included in this PR? 1. Re-implemented `featherwrite` using `arrow.internal.io.feather.Writer`. ### Are these changes tested? 1. Yes, the existing tests in `tfeather.m` cover these changes. 2. I had to update some of the expected error message IDs in `tfeather.m` because the new implementation throws errors with different IDs. 3. `featherwrite` used to export the real part of MATLAB complex numeric arrays. The new version of `featherwrite` now errors if the input table contains complex data because feather/Arrow itself does not support complex numeric data. We think this is the right decision. Writing out only the real part is lossy. ### Are there any user-facing changes? Yes, `featherwrite` no longer supports writing complex numeric arrays. ### Future Directions 1. Once this PR is merged, we will remove the legacy build infrastructure and MEX code. * Closes: #37045 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
This was referenced Aug 15, 2023
kevingurney
pushed a commit
that referenced
this issue
Aug 15, 2023
### Rationale for this change Now that `featherread` and `featherwrite` have been re-implemented in terms of the new APIs (`matlab.internal.io.feather.Reader` in #37046 and `matlab.io.internal.feather.Writer` in #37045), we can start removing the MEX code. To avoid submitting a large pull request, we should break the work down into several smaller, incremental steps. The first step should involve deleting the outdated test class `tArrowCppCall.m`. This test class will no longer be relevant once the MEX source code is removed. ### What changes are included in this PR? 1. Deleted the outdated test class `tArrowCppCall.m` ### Are these changes tested? No tests necessary. ### Are there any user-facing changes? No. ### Future Directions 1. Move the test cases in `test/tfeathermex.m` to `test/arrow/internal/io/feather/tRoundtrip.m`. These test cases directly call the MEX functions, so we will re-write the tests to utilize the `arrow.internal.io.feather.Reader` and `arrow.internal.io.feather.Writer` classes directly. 2. Delete `test/arrow/util/createVariablesAndMetadataStruct.m` 3. Delete `test/arrow/util/createTable.m` 4. Delete `test/arrow/util/featherMEXRoundTrip.m` 5. Delete `test/arrow/util/featherRoundTrip.m` 6. Remove MEX source code. * Closes: #37181 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this issue
Nov 13, 2023
…ernal.io.feather.Writer (apache#37047) ### Rationale for this change Now that apache#37043 is merged, we can re-implement `featherwrite` in terms of the new `arrow.internal.io.feather.Writer` class. Once this change is made, we can delete the legacy build infrastructure and featherwrite MEX code. ### What changes are included in this PR? 1. Re-implemented `featherwrite` using `arrow.internal.io.feather.Writer`. ### Are these changes tested? 1. Yes, the existing tests in `tfeather.m` cover these changes. 2. I had to update some of the expected error message IDs in `tfeather.m` because the new implementation throws errors with different IDs. 3. `featherwrite` used to export the real part of MATLAB complex numeric arrays. The new version of `featherwrite` now errors if the input table contains complex data because feather/Arrow itself does not support complex numeric data. We think this is the right decision. Writing out only the real part is lossy. ### Are there any user-facing changes? Yes, `featherwrite` no longer supports writing complex numeric arrays. ### Future Directions 1. Once this PR is merged, we will remove the legacy build infrastructure and MEX code. * Closes: apache#37045 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this issue
Nov 13, 2023
…apache#37185) ### Rationale for this change Now that `featherread` and `featherwrite` have been re-implemented in terms of the new APIs (`matlab.internal.io.feather.Reader` in apache#37046 and `matlab.io.internal.feather.Writer` in apache#37045), we can start removing the MEX code. To avoid submitting a large pull request, we should break the work down into several smaller, incremental steps. The first step should involve deleting the outdated test class `tArrowCppCall.m`. This test class will no longer be relevant once the MEX source code is removed. ### What changes are included in this PR? 1. Deleted the outdated test class `tArrowCppCall.m` ### Are these changes tested? No tests necessary. ### Are there any user-facing changes? No. ### Future Directions 1. Move the test cases in `test/tfeathermex.m` to `test/arrow/internal/io/feather/tRoundtrip.m`. These test cases directly call the MEX functions, so we will re-write the tests to utilize the `arrow.internal.io.feather.Reader` and `arrow.internal.io.feather.Writer` classes directly. 2. Delete `test/arrow/util/createVariablesAndMetadataStruct.m` 3. Delete `test/arrow/util/createTable.m` 4. Delete `test/arrow/util/featherMEXRoundTrip.m` 5. Delete `test/arrow/util/featherRoundTrip.m` 6. Remove MEX source code. * Closes: apache#37181 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the enhancement requested
Now that #37043 is merged, we can re-implement
featherwritein terms of the newarrow.internal.io.feather.Writerclass. Once this change is made, we can delete the legacy build infrastructure and featherwrite MEX code.Component(s)
MATLAB
The text was updated successfully, but these errors were encountered: