You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kevingurney (via GitHub)" <gi...@apache.org> on 2023/08/07 20:04:00 UTC
[GitHub] [arrow] kevingurney opened a new pull request, #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
kevingurney opened a new pull request, #37044:
URL: https://github.com/apache/arrow/pull/37044
### Rationale for this change
Now that we've have the basic building blocks for tabular IO in the MATLAB Interface (Array, Schema, RecordBatch), we can implement a Feather V1 reader in terms of the new APIs.
This is a follow up to #37043, where a new Feather V1 internal `Writer` object was added.
### What changes are included in this PR?
1. Added a new class called arrow.internal.io.feather.Reader which can be used to read Feather V1 files. It has one public property named `Filename` and one public method named `read`.
**Example Usage:**
```matlab
>> T = array2table(rand(3))
T =
3x3 table
Var1 Var2 Var3
_______ ________ _______
0.79221 0.035712 0.67874
0.95949 0.84913 0.75774
0.65574 0.93399 0.74313
>> filename = "test.feather";
>> featherwrite(filename, T)
>> reader = arrow.internal.io.feather.Reader(filename)
reader =
Reader with properties:
Filename: "test.feather"
>> T = reader.read()
T =
3x3 table
Var1 Var2 Var3
_______ ________ _______
0.79221 0.035712 0.67874
0.95949 0.84913 0.75774
0.65574 0.93399 0.74313
```
### Are these changes tested?
Yes.
1. Added `Reader` to `feather/tRoundTrip.m`.
### Are there any user-facing changes?
No.
These are only internal objects right now.
### Future Directions
1. Re-implement `featherread` in terms of the new `Reader` object.
2. Remove legacy feather code and infrastructure.
### Notes
1. For conciseness, I renamed the C++ Proxy class `FeatherWriter` to `Writer` since it is already inside of a `feather` namespace / "package".
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37044:
URL: https://github.com/apache/arrow/pull/37044#discussion_r1286338497
##########
matlab/src/cpp/arrow/matlab/io/feather/proxy/reader.cc:
##########
@@ -0,0 +1,98 @@
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "libmexclass/proxy/ProxyManager.h"
+
+#include "arrow/matlab/error/error.h"
+#include "arrow/matlab/io/feather/proxy/reader.h"
+#include "arrow/matlab/tabular/proxy/record_batch.h"
+
+#include "arrow/util/utf8.h"
+
+#include "arrow/result.h"
+
+#include "arrow/io/file.h"
+#include "arrow/ipc/feather.h"
+#include "arrow/table.h"
+
+namespace arrow::matlab::io::feather::proxy {
+
+ Reader::Reader(const std::string& filename) : filename{filename} {
+ REGISTER_METHOD(Reader, read);
+ REGISTER_METHOD(Reader, getFilename);
+ }
+
+ libmexclass::proxy::MakeResult Reader::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using ReaderProxy = arrow::matlab::io::feather::proxy::Reader;
+
+ mda::StructArray args = constructor_arguments[0];
+ const mda::StringArray filename_utf16_mda = args[0]["Filename"];
+ const auto filename_utf16 = std::u16string(filename_utf16_mda[0]);
Review Comment:
Can use `&` here to avoid a copy.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney merged pull request #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney merged PR #37044:
URL: https://github.com/apache/arrow/pull/37044
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney commented on a diff in pull request #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #37044:
URL: https://github.com/apache/arrow/pull/37044#discussion_r1286342626
##########
matlab/src/matlab/+arrow/+internal/+io/+feather/Reader.m:
##########
@@ -0,0 +1,52 @@
+% Licensed to the Apache Software Foundation (ASF) under one or more
+% contributor license agreements. See the NOTICE file distributed with
+% this work for additional information regarding copyright ownership.
+% The ASF licenses this file to you under the Apache License, Version
+% 2.0 (the "License"); you may not use this file except in compliance
+% with the License. You may obtain a copy of the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS,
+% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+% implied. See the License for the specific language governing
+% permissions and limitations under the License.
+
+classdef Reader
+%READER An internal Reader object for reading Feather files.
+
+ properties (GetAccess=public, SetAccess=private, Hidden)
+ Proxy
+ end
+
+ properties (Dependent, SetAccess=private, GetAccess=public)
+ % Name of the file to read.
+ Filename
+ end
+
+ methods
+
+ function obj = Reader(filename)
+ filename = convertCharsToStrings(filename);
Review Comment:
Good point.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney commented on a diff in pull request #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #37044:
URL: https://github.com/apache/arrow/pull/37044#discussion_r1286344535
##########
matlab/src/matlab/+arrow/+internal/+io/+feather/Reader.m:
##########
@@ -0,0 +1,52 @@
+% Licensed to the Apache Software Foundation (ASF) under one or more
+% contributor license agreements. See the NOTICE file distributed with
+% this work for additional information regarding copyright ownership.
+% The ASF licenses this file to you under the Apache License, Version
+% 2.0 (the "License"); you may not use this file except in compliance
+% with the License. You may obtain a copy of the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS,
+% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+% implied. See the License for the specific language governing
+% permissions and limitations under the License.
+
+classdef Reader
+%READER An internal Reader object for reading Feather files.
+
+ properties (GetAccess=public, SetAccess=private, Hidden)
+ Proxy
+ end
+
+ properties (Dependent, SetAccess=private, GetAccess=public)
+ % Name of the file to read.
+ Filename
+ end
+
+ methods
+
+ function obj = Reader(filename)
+ filename = convertCharsToStrings(filename);
Review Comment:
Fixed.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney commented on pull request #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #37044:
URL: https://github.com/apache/arrow/pull/37044#issuecomment-1668523720
+1
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37044:
URL: https://github.com/apache/arrow/pull/37044#discussion_r1286340384
##########
matlab/src/matlab/+arrow/+internal/+io/+feather/Reader.m:
##########
@@ -0,0 +1,52 @@
+% Licensed to the Apache Software Foundation (ASF) under one or more
+% contributor license agreements. See the NOTICE file distributed with
+% this work for additional information regarding copyright ownership.
+% The ASF licenses this file to you under the Apache License, Version
+% 2.0 (the "License"); you may not use this file except in compliance
+% with the License. You may obtain a copy of the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS,
+% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+% implied. See the License for the specific language governing
+% permissions and limitations under the License.
+
+classdef Reader
+%READER An internal Reader object for reading Feather files.
+
+ properties (GetAccess=public, SetAccess=private, Hidden)
+ Proxy
+ end
+
+ properties (Dependent, SetAccess=private, GetAccess=public)
+ % Name of the file to read.
+ Filename
+ end
+
+ methods
+
+ function obj = Reader(filename)
+ filename = convertCharsToStrings(filename);
Review Comment:
You could use `arguments` block here, but it's not necessary.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #37044:
URL: https://github.com/apache/arrow/pull/37044#issuecomment-1671770626
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 152be67100cdd367a3e6064988085e3b327ad0fe.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/15755613985) has more details.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on a diff in pull request #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #37044:
URL: https://github.com/apache/arrow/pull/37044#discussion_r1286338497
##########
matlab/src/cpp/arrow/matlab/io/feather/proxy/reader.cc:
##########
@@ -0,0 +1,98 @@
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "libmexclass/proxy/ProxyManager.h"
+
+#include "arrow/matlab/error/error.h"
+#include "arrow/matlab/io/feather/proxy/reader.h"
+#include "arrow/matlab/tabular/proxy/record_batch.h"
+
+#include "arrow/util/utf8.h"
+
+#include "arrow/result.h"
+
+#include "arrow/io/file.h"
+#include "arrow/ipc/feather.h"
+#include "arrow/table.h"
+
+namespace arrow::matlab::io::feather::proxy {
+
+ Reader::Reader(const std::string& filename) : filename{filename} {
+ REGISTER_METHOD(Reader, read);
+ REGISTER_METHOD(Reader, getFilename);
+ }
+
+ libmexclass::proxy::MakeResult Reader::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using ReaderProxy = arrow::matlab::io::feather::proxy::Reader;
+
+ mda::StructArray args = constructor_arguments[0];
+ const mda::StringArray filename_utf16_mda = args[0]["Filename"];
+ const auto filename_utf16 = std::u16string(filename_utf16_mda[0]);
Review Comment:
Can use `&` here to avoid a copy.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kevingurney commented on pull request #37044: GH-37041: [MATLAB] Implement Feather V1 Reader using new MATLAB Interface APIs
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #37044:
URL: https://github.com/apache/arrow/pull/37044#issuecomment-1668523623
Linting failures appear unrelated to these changes.
--
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-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org