You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "sgilmore10 (via GitHub)" <gi...@apache.org> on 2023/10/04 19:08:57 UTC

[PR] [GH-38015]: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

sgilmore10 opened a new pull request, #38020:
URL: https://github.com/apache/arrow/pull/38020

   <!--
   Thanks for opening a pull request!
   If this is your first pull request you can find detailed information on how 
   to contribute here:
     * [New Contributor's Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
     * [Contributing Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
   
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
   
   Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
   
   Then could you also rename the pull request title in the following format?
   
       GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   or
   
       MINOR: [${COMPONENT}] ${SUMMARY}
   
   In the case of PARQUET issues on JIRA the title also supports:
   
       PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   -->
   
   ### Rationale for this change
   
   To unblock use cases that are not satisfied by the default Arrow -> MATLAB conversions (i.e. the `toMATLAB()` on `arrow.array.Array`), we would like expose the underlying Arrow data representation as a property on `arrow.array.Array`. One possible name for this property would be `DataLayout`, which would be an `arrow.array.DataLayout` object. Note, this class does not yet exist, so we would have to add it.
   
   For example, the `DataLayout` property for temporal array types would return an object of the following class type: 
   
   ```matlab
   classdef TemporalDataLayout < arrow.array.DataLayout
       properties
          Values % an arrow.array.Int32Array or an arrow.array.Int64Array
          Valid  % an arrow.buffer.Buffer 
       end
   end
   ```
   
   However, the `Valid` property on this class would need to be an `arrow.buffer.Buffer` object, which does not yet exist in the MATLAB interface.  Therefore, it would be helpful to first add the `arrow.buffer.Buffer` class before adding the `DataLayout` property/class hierarchy. It's worth mentioning that adding `arrow.buffer.Buffer` will open up additional advanced use cases in the future.
   
   ### What changes are included in this PR?
   
   Added `arrow.buffer.Buffer` MATLAB class.
   
   *Properties of `arrow.buffer.Buffer`*
    1. `NumBytes` - a  scalar `int64` value representing the size of the buffer in bytes. 
   
   *Methods of `arrow.buffer.Buffer`*
   1. `toMATLAB` - returns the data in the buffer as `Nx1` `uint8` vector, where `N` is the number of bytes.
   2. `fromMATLAB(data)` - Static method that creates an `arrow.buffer.Buffer` from a numeric array. 
   
   **Example: **
   ```matlab
   >> dataIn = [1 2];
   >> buffer = arrow.buffer.Buffer.fromMATLAB(dataIn)
   
   buffer = 
   
     Buffer with properties:
   
       NumBytes: 16
   
   >> dataOut = toMATLAB(buffer)
   
   dataOut =
   
     16×1 uint8 column vector
   
        0
        0
        0
        0
        0
        0
      240
       63
        0
        0
        0
        0
        0
        0
        0
       64
   
   % Reinterpret bit pattern as a double array 
   >> toDouble = typecast(dataOut, "double")
   
   toDouble =
   
        1
        2
   ```
   
   ### Are these changes tested?
   
   Yes. Added a new test class called `tBuffer.m`
   
   ### Are there any user-facing changes?
   
   Yes. Users can now create `arrow.buffer.Buffer` objects via the `fromMATLAB` static method. However, there's not much users can do with this object as of now. We implemented this class to facilitate adding `DataLayout` property to `arrow.array.Array`, as described in the **Rational for this change** section. 
   
   <!--
   Please uncomment the line below (and provide explanation) if the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld). We use this to highlight fixes to issues that may affect users without their knowledge. For this reason, fixing bugs that cause errors don't count, since those are usually obvious.
   -->
   <!-- **This PR contains a "Critical Fix".** -->


-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1348795130


##########
matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc:
##########
@@ -0,0 +1,97 @@
+// 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.
+
+
+#include "arrow/matlab/buffer/proxy/buffer.h"
+#include "arrow/matlab/buffer/matlab_buffer.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::buffer::proxy {
+
+    Buffer::Buffer(std::shared_ptr<arrow::Buffer> buffer) : buffer{std::move(buffer)} {
+        
+        REGISTER_METHOD(Buffer, getNumBytes);
+        REGISTER_METHOD(Buffer, isEqual);
+        REGISTER_METHOD(Buffer, toMATLAB);
+    }
+
+    libmexclass::proxy::MakeResult Buffer::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+        namespace mda = ::matlab::data;
+        using BufferProxy = proxy::Buffer;
+        using MatlabBuffer = arrow::matlab::buffer::MatlabBuffer;
+
+        mda::StructArray opts = constructor_arguments[0];
+        const mda::TypedArray<uint8_t> values_mda = opts[0]["Values"];
+        auto buffer = std::make_shared<MatlabBuffer>(values_mda);
+        return std::make_shared<BufferProxy>(std::move(buffer));
+    }
+
+    std::shared_ptr<arrow::Buffer> Buffer::unwrap() {
+        return buffer;
+    }
+
+    void Buffer::getNumBytes(libmexclass::proxy::method::Context& context) {
+        namespace mda = ::matlab::data;
+        mda::ArrayFactory factory;        
+        auto num_bytes_mda = factory.createScalar(buffer->size());
+        context.outputs[0] = num_bytes_mda; 
+    }
+
+    void Buffer::isEqual(libmexclass::proxy::method::Context& context) { 
+        namespace mda = ::matlab::data;
+
+        bool is_equal = true;
+        const mda::TypedArray<uint64_t> buffer_proxy_ids = context.inputs[0];
+        for (const auto& buffer_proxy_id : buffer_proxy_ids) {
+            // Retrieve the Buffer proxy from the ProxyManager
+            auto proxy = libmexclass::proxy::ProxyManager::getProxy(buffer_proxy_id);
+            auto buffer_proxy = std::static_pointer_cast<proxy::Buffer>(proxy);
+            auto buffer_to_compare = buffer_proxy->unwrap();
+
+            if (!buffer->Equals(*buffer_to_compare)) {
+                is_equal = false;
+                break;
+            }
+        }
+        mda::ArrayFactory factory;
+        context.outputs[0] = factory.createScalar(is_equal);
+    }
+
+    void Buffer::toMATLAB(libmexclass::proxy::method::Context& context) { 
+        namespace mda = ::matlab::data;
+
+        // If buffer->is_cpu() returns false, invoking buffer->data() may cause a crash.
+        // Avoid this potential crash by first invoking ViewOrCopy(buffer, memory_manager_device).
+        // This function tries to create a no-copy view of the buffer on the given memory
+        // manager device. If not possible, then ViewOrCopy copies the buffer's contents.
+        MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(
+            auto cpu_buffer, 
+            arrow::Buffer::ViewOrCopy(buffer, arrow::default_cpu_memory_manager()),
+            context, error::BUFFER_VIEW_OR_COPY_FAILED
+        );
+
+        const auto* data_begin = cpu_buffer->data();
+        const auto num_bytes = cpu_buffer->size();
+        // data_begin is a uint8_t*, so num_bytes is equal to the number of elements

Review Comment:
   number of elements -> number of bytes



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1346621986


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");

Review Comment:
   I was wondering about this myself. My only hesitation was that strings in MATLAB are UTF-16 encoded, while string arrays in Arrow are UTF-8 encoded. I could convert the strings from UTF-16 to UTF-8 before extracting the uint8 values. 



-- 
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


Re: [PR] [GH-38015]: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1346457271


##########
matlab/src/cpp/arrow/matlab/proxy/factory.cc:
##########
@@ -40,12 +40,14 @@
 #include "arrow/matlab/io/feather/proxy/reader.h"
 #include "arrow/matlab/io/csv/proxy/table_writer.h"
 #include "arrow/matlab/io/csv/proxy/table_reader.h"
+#include "arrow/matlab/buffer/proxy/buffer.h"
 
 #include "factory.h"
 
 namespace arrow::matlab::proxy {
 
 libmexclass::proxy::MakeResult Factory::make_proxy(const ClassName& class_name, const FunctionArguments& constructor_arguments) {
+    REGISTER_PROXY(arrow.buffer.proxy.Buffer       , arrow::matlab::buffer::proxy::Buffer);

Review Comment:
   We may want to keep this list in alphabetical order.



##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");

Review Comment:
   Hmm. We may want to support a string because `arrow::Buffer` is also used for general raw data such as serialized Apache Arrow {File,Stream} format data.



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #38020:
URL: https://github.com/apache/arrow/pull/38020#issuecomment-1753433668

   Sorry, I had some pending comments last week - but I accidentally never published them.


-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1346638994


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");

Review Comment:
   I see. Does MATLAB support binary data (not string)? (Is it uint8 values.)
   I think that `arrow::Buffer`'s data should not be converted. (We should use it as-is.)



##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");

Review Comment:
   I see. Does MATLAB support binary data (not string)? (Is it uint8 values?)
   I think that `arrow::Buffer`'s data should not be converted. (We should use it as-is.)



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1352550044


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);

Review Comment:
   My bad!



-- 
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


Re: [PR] [GH-38015]: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38020:
URL: https://github.com/apache/arrow/pull/38020#issuecomment-1747482544

   <!--
     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.
   -->
   
   Thanks for opening a pull request!
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
   
   Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
   
   Then could you also rename the pull request title in the following format?
   
       GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   or
   
       MINOR: [${COMPONENT}] ${SUMMARY}
   
   In the case of PARQUET issues on JIRA the title also supports:
   
       PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   See also:
   
     * [Other pull requests](https://github.com/apache/arrow/pulls/)
     * [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
   


-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1352553170


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB(datetime(2023, 1, 1));
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+        end
+
+        function fromMATLABNonRealError(testCase)
+            % Verify fromMATLAB throws an error if given a complex
+            % numeric  array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(10 + 3i);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeReal");
+        end
+
+        function fromMATLABNonSparseError(testCase)
+            % Verify fromMATLAB throws an error if given a sparse
+            % numeric array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(sparse(ones([5 1])));
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNonsparse");
+        end
+
+        function NumBytesNoSetter(testCase)
+            % Verifies the NumBytes property is not settable.
+
+            import arrow.buffer.Buffer
+
+            buffer = Buffer.fromMATLAB([1 2 3]);
+            fcn = @() setfield(buffer, "NumBytes", int64(1));
+            testCase.verifyError(fcn, "MATLAB:class:SetProhibited");
+        end
+
+        function IsEqualTrue(testCase)
+            % Verifies two buffers are considered equal if
+            %   1. They have the same size (NumBytes)
+            %   2. They contain the same bytes
+
+            import arrow.buffer.Buffer
+
+            % Compare two non-empty buffers
+            buffer1 = Buffer.fromMATLAB([1 2 3]);
+            buffer2 = Buffer.fromMATLAB([1 2 3]);
+            testCase.verifyTrue(isequal(buffer1, buffer2));
+
+            % Compare two buffers, one of which was created from a double 
+            % array and the other created from a uint8 array. However, both
+            % arrays have the same bit pattern.
+            data = zeros([1 24], "uint8");
+            data([7 8 16 23 24]) = [240 63 64  8 64];
+            buffer3 = Buffer.fromMATLAB(data);
+            testCase.verifyTrue(isequal(buffer1, buffer3));
+
+            % Compare two empty buffers 
+            buffer4 = Buffer.fromMATLAB([]);
+            buffer5 = Buffer.fromMATLAB([]);
+            testCase.verifyTrue(isequal(buffer4, buffer5));
+
+            % Compare more than two buffers
+            testCase.verifyTrue(isequal(buffer1, buffer2, buffer3));
+        end
+
+        function IsEqualFalse(testCase)
+            % Verifies two buffers are considered equal if

Review Comment:
   Oh that was a copy-paste issue. I'll fix that. 



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1347911169


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");

Review Comment:
   Thanks. I see.
   I think that we don't need to add support for string. It seems that using string to represent binary data isn't natural in MATLAB.



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1346615131


##########
matlab/src/cpp/arrow/matlab/proxy/factory.cc:
##########
@@ -40,12 +40,14 @@
 #include "arrow/matlab/io/feather/proxy/reader.h"
 #include "arrow/matlab/io/csv/proxy/table_writer.h"
 #include "arrow/matlab/io/csv/proxy/table_reader.h"
+#include "arrow/matlab/buffer/proxy/buffer.h"
 
 #include "factory.h"
 
 namespace arrow::matlab::proxy {
 
 libmexclass::proxy::MakeResult Factory::make_proxy(const ClassName& class_name, const FunctionArguments& constructor_arguments) {
+    REGISTER_PROXY(arrow.buffer.proxy.Buffer       , arrow::matlab::buffer::proxy::Buffer);

Review Comment:
   Good point. I'll fix it!



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1352578843


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -69,7 +69,7 @@ function fromMATLABFromEmptyUint8(testCase)
 
         function fromMATLABUint8Vector(testCase)

Review Comment:
   No problem! Just fixed the other two test cases.



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1347474328


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");

Review Comment:
   Yes, we can use `uint8` values to represent binary data in MALTAB. One solution would be to
   
   1. Convert `string` data into `uint16` values
   2. Call `typecast` on the `uint16` values to re-interpret the bit pattern as `uint8` values.
   
   That would allow users to create `arrow.buffer.Buffer`s from `string`s without modifying the underlying data.



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1350313789


##########
matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc:
##########
@@ -0,0 +1,97 @@
+// 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.
+
+
+#include "arrow/matlab/buffer/proxy/buffer.h"
+#include "arrow/matlab/buffer/matlab_buffer.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::buffer::proxy {
+
+    Buffer::Buffer(std::shared_ptr<arrow::Buffer> buffer) : buffer{std::move(buffer)} {
+        
+        REGISTER_METHOD(Buffer, getNumBytes);
+        REGISTER_METHOD(Buffer, isEqual);
+        REGISTER_METHOD(Buffer, toMATLAB);
+    }
+
+    libmexclass::proxy::MakeResult Buffer::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+        namespace mda = ::matlab::data;
+        using BufferProxy = proxy::Buffer;
+        using MatlabBuffer = arrow::matlab::buffer::MatlabBuffer;
+
+        mda::StructArray opts = constructor_arguments[0];
+        const mda::TypedArray<uint8_t> values_mda = opts[0]["Values"];
+        auto buffer = std::make_shared<MatlabBuffer>(values_mda);
+        return std::make_shared<BufferProxy>(std::move(buffer));
+    }
+
+    std::shared_ptr<arrow::Buffer> Buffer::unwrap() {
+        return buffer;
+    }
+
+    void Buffer::getNumBytes(libmexclass::proxy::method::Context& context) {
+        namespace mda = ::matlab::data;
+        mda::ArrayFactory factory;        
+        auto num_bytes_mda = factory.createScalar(buffer->size());
+        context.outputs[0] = num_bytes_mda; 
+    }
+
+    void Buffer::isEqual(libmexclass::proxy::method::Context& context) { 
+        namespace mda = ::matlab::data;
+
+        bool is_equal = true;
+        const mda::TypedArray<uint64_t> buffer_proxy_ids = context.inputs[0];
+        for (const auto& buffer_proxy_id : buffer_proxy_ids) {
+            // Retrieve the Buffer proxy from the ProxyManager
+            auto proxy = libmexclass::proxy::ProxyManager::getProxy(buffer_proxy_id);
+            auto buffer_proxy = std::static_pointer_cast<proxy::Buffer>(proxy);
+            auto buffer_to_compare = buffer_proxy->unwrap();
+
+            if (!buffer->Equals(*buffer_to_compare)) {
+                is_equal = false;
+                break;
+            }
+        }
+        mda::ArrayFactory factory;
+        context.outputs[0] = factory.createScalar(is_equal);
+    }
+
+    void Buffer::toMATLAB(libmexclass::proxy::method::Context& context) { 
+        namespace mda = ::matlab::data;
+
+        // If buffer->is_cpu() returns false, invoking buffer->data() may cause a crash.
+        // Avoid this potential crash by first invoking ViewOrCopy(buffer, memory_manager_device).
+        // This function tries to create a no-copy view of the buffer on the given memory
+        // manager device. If not possible, then ViewOrCopy copies the buffer's contents.
+        MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(
+            auto cpu_buffer, 
+            arrow::Buffer::ViewOrCopy(buffer, arrow::default_cpu_memory_manager()),
+            context, error::BUFFER_VIEW_OR_COPY_FAILED
+        );
+
+        const auto* data_begin = cpu_buffer->data();
+        const auto num_bytes = cpu_buffer->size();
+        // data_begin is a uint8_t*, so num_bytes is equal to the number of elements

Review Comment:
   I actually meant number of elements in this case.  I wanted to make clear that in this case, the number of bytes is the same as the number of elements because each each element is 1 byte.



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1352554461


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));

Review Comment:
   Yeah, that's a good idea.



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1352572963


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -69,7 +69,7 @@ function fromMATLABFromEmptyUint8(testCase)
 
         function fromMATLABUint8Vector(testCase)

Review Comment:
   Sorry, I think I meant to imply with my previous comment that we should capitalize all of the `Uint` to `UInt` for consistency.



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #38020:
URL: https://github.com/apache/arrow/pull/38020#issuecomment-1755541009

   +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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1347474328


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");

Review Comment:
   Yes, we can use `uint8` values to represent binary data in MALTAB. One solution would be to
   
   1. Convert `string` data into `uint16` values
   2. Call `typecast` on the `uint16` values to re-interpret the bit pattern as `uint8` values.
   
   That would allow users to create `arrow.buffer.Buffer`s from `string`s without modifying the underlying data. However, we would have to make a copy to do this.
   
   @kou, do you think we should add support for creating `arrow.buffer.Buffer`s from MATLAB `string`s using this approach?



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1352574658


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -69,7 +69,7 @@ function fromMATLABFromEmptyUint8(testCase)
 
         function fromMATLABUint8Vector(testCase)

Review Comment:
   I will merge this as soon as we make that change. Thank you!



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney merged PR #38020:
URL: https://github.com/apache/arrow/pull/38020


-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1350311091


##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");

Review Comment:
   Thanks for clarifying!



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38020:
URL: https://github.com/apache/arrow/pull/38020#discussion_r1348795360


##########
matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc:
##########
@@ -0,0 +1,97 @@
+// 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.
+
+
+#include "arrow/matlab/buffer/proxy/buffer.h"
+#include "arrow/matlab/buffer/matlab_buffer.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::buffer::proxy {
+
+    Buffer::Buffer(std::shared_ptr<arrow::Buffer> buffer) : buffer{std::move(buffer)} {
+        

Review Comment:
   Extra newline here.



##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)

Review Comment:
   Uint -> UInt



##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);

Review Comment:
   uint8.empty(0, 1) -> uint8.empty(1, 0)



##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source', "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source');
+
+            % Create buffer from 3x1 double array
+            source =  [3 9 27]';
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));
+            values = toMATLAB(buffer);
+            expected = typecast(source, "uint8");
+            testCase.verifyEqual(values, expected);
+            testCase.verifyEqual(typecast(values, "double"), source);
+        end
+
+        function fromMATLABNonNumericError(testCase)
+            % Verify fromMATLAB throws an error if given a non-numeric
+            % array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(true);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB("A");
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+
+            fcn = @() Buffer.fromMATLAB(datetime(2023, 1, 1));
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNumeric");
+        end
+
+        function fromMATLABNonRealError(testCase)
+            % Verify fromMATLAB throws an error if given a complex
+            % numeric  array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(10 + 3i);
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeReal");
+        end
+
+        function fromMATLABNonSparseError(testCase)
+            % Verify fromMATLAB throws an error if given a sparse
+            % numeric array as input.
+            import arrow.buffer.Buffer
+
+            fcn = @() Buffer.fromMATLAB(sparse(ones([5 1])));
+            testCase.verifyError(fcn, "MATLAB:validators:mustBeNonsparse");
+        end
+
+        function NumBytesNoSetter(testCase)
+            % Verifies the NumBytes property is not settable.
+
+            import arrow.buffer.Buffer
+
+            buffer = Buffer.fromMATLAB([1 2 3]);
+            fcn = @() setfield(buffer, "NumBytes", int64(1));
+            testCase.verifyError(fcn, "MATLAB:class:SetProhibited");
+        end
+
+        function IsEqualTrue(testCase)
+            % Verifies two buffers are considered equal if
+            %   1. They have the same size (NumBytes)
+            %   2. They contain the same bytes
+
+            import arrow.buffer.Buffer
+
+            % Compare two non-empty buffers
+            buffer1 = Buffer.fromMATLAB([1 2 3]);
+            buffer2 = Buffer.fromMATLAB([1 2 3]);
+            testCase.verifyTrue(isequal(buffer1, buffer2));
+
+            % Compare two buffers, one of which was created from a double 
+            % array and the other created from a uint8 array. However, both
+            % arrays have the same bit pattern.
+            data = zeros([1 24], "uint8");
+            data([7 8 16 23 24]) = [240 63 64  8 64];
+            buffer3 = Buffer.fromMATLAB(data);
+            testCase.verifyTrue(isequal(buffer1, buffer3));
+
+            % Compare two empty buffers 
+            buffer4 = Buffer.fromMATLAB([]);
+            buffer5 = Buffer.fromMATLAB([]);
+            testCase.verifyTrue(isequal(buffer4, buffer5));
+
+            % Compare more than two buffers
+            testCase.verifyTrue(isequal(buffer1, buffer2, buffer3));
+        end
+
+        function IsEqualFalse(testCase)
+            % Verifies two buffers are considered equal if

Review Comment:
   I think it would be good to modify this comment to discuss when two Buffers are *not* considered equal.



##########
matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.h:
##########
@@ -0,0 +1,48 @@
+// 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.
+
+#pragma once
+
+#include "arrow/buffer.h"
+
+#include "libmexclass/proxy/Proxy.h"
+
+namespace arrow::matlab::buffer::proxy {
+
+class Buffer : public libmexclass::proxy::Proxy {
+    public:
+        Buffer(std::shared_ptr<arrow::Buffer> buffer);
+    
+        ~Buffer() {}
+
+        std::shared_ptr<arrow::Buffer> unwrap();
+
+        static libmexclass::proxy::MakeResult make(const libmexclass::proxy::FunctionArguments& constructor_arguments);
+
+
+    protected:
+
+        void getNumBytes(libmexclass::proxy::method::Context& context);
+

Review Comment:
   There is a lot of extra newlines here. Might look a little bit better if we removed some of them.



##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 double array
+            source = [3 9 27];
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(24));

Review Comment:
   Might be helpful to add a comment here like:
   
   ```matlab
   % Since the input vector is a 1x3 double (i.e. 64-bit floating point value)  array - each of the 3 elements takes up 8 bytes (3 elements * 8 bytes per element = 24 bytes).
   ```



##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 1x3 uint8 array
+            source = uint8([3 9 27]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+
+            % Create buffer from 3x1 uint8 array
+            source =  uint8([3 9 27]');
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(3));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8([3 9 27]'));
+        end
+
+        function fromMATLABDoubleVector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an double vector.

Review Comment:
   an -> a



##########
matlab/test/arrow/buffer/tBuffer.m:
##########
@@ -0,0 +1,206 @@
+%TBUFFER Unit tests for arrow.buffer.Buffer
+
+% 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 tBuffer < matlab.unittest.TestCase
+
+    methods (Test)
+        function Smoke(testCase)
+            import arrow.buffer.Buffer
+            
+            source = uint8([1 2 3]);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyInstanceOf(buffer, "arrow.buffer.Buffer");
+        end
+
+        function fromMATLABUint8Scalar(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % instance when given a scalar uint8 value.
+
+            import arrow.buffer.Buffer
+            
+            source = uint8(20);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(1));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, source);
+        end
+
+        function fromMATLABFromEmptyUint8(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an empty uint8 array.
+
+            import arrow.buffer.Buffer
+            
+            % Create buffer from 0x0 uint8 array
+            source = uint8.empty(0, 0);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 0x1 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+
+            % Create buffer from 1x0 uint8 array
+            source = uint8.empty(0, 1);
+            buffer = Buffer.fromMATLAB(source);
+            testCase.verifyEqual(buffer.NumBytes, int64(0));
+            values = toMATLAB(buffer);
+            testCase.verifyEqual(values, uint8.empty(0, 1));
+        end
+
+        function fromMATLABUint8Vector(testCase)
+            % Verifies fromMATLAB returns the expected arrow.buffer.Buffer
+            % value when given an uint8 vector.

Review Comment:
   an -> a



##########
matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc:
##########
@@ -0,0 +1,97 @@
+// 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.
+
+
+#include "arrow/matlab/buffer/proxy/buffer.h"
+#include "arrow/matlab/buffer/matlab_buffer.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::buffer::proxy {
+
+    Buffer::Buffer(std::shared_ptr<arrow::Buffer> buffer) : buffer{std::move(buffer)} {
+        
+        REGISTER_METHOD(Buffer, getNumBytes);
+        REGISTER_METHOD(Buffer, isEqual);
+        REGISTER_METHOD(Buffer, toMATLAB);
+    }
+
+    libmexclass::proxy::MakeResult Buffer::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+        namespace mda = ::matlab::data;
+        using BufferProxy = proxy::Buffer;
+        using MatlabBuffer = arrow::matlab::buffer::MatlabBuffer;
+
+        mda::StructArray opts = constructor_arguments[0];
+        const mda::TypedArray<uint8_t> values_mda = opts[0]["Values"];
+        auto buffer = std::make_shared<MatlabBuffer>(values_mda);
+        return std::make_shared<BufferProxy>(std::move(buffer));
+    }
+
+    std::shared_ptr<arrow::Buffer> Buffer::unwrap() {
+        return buffer;
+    }
+
+    void Buffer::getNumBytes(libmexclass::proxy::method::Context& context) {
+        namespace mda = ::matlab::data;
+        mda::ArrayFactory factory;        
+        auto num_bytes_mda = factory.createScalar(buffer->size());
+        context.outputs[0] = num_bytes_mda; 
+    }
+
+    void Buffer::isEqual(libmexclass::proxy::method::Context& context) { 
+        namespace mda = ::matlab::data;
+
+        bool is_equal = true;
+        const mda::TypedArray<uint64_t> buffer_proxy_ids = context.inputs[0];
+        for (const auto& buffer_proxy_id : buffer_proxy_ids) {
+            // Retrieve the Buffer proxy from the ProxyManager
+            auto proxy = libmexclass::proxy::ProxyManager::getProxy(buffer_proxy_id);
+            auto buffer_proxy = std::static_pointer_cast<proxy::Buffer>(proxy);
+            auto buffer_to_compare = buffer_proxy->unwrap();
+
+            if (!buffer->Equals(*buffer_to_compare)) {
+                is_equal = false;
+                break;
+            }
+        }
+        mda::ArrayFactory factory;
+        context.outputs[0] = factory.createScalar(is_equal);
+    }
+
+    void Buffer::toMATLAB(libmexclass::proxy::method::Context& context) { 
+        namespace mda = ::matlab::data;
+
+        // If buffer->is_cpu() returns false, invoking buffer->data() may cause a crash.
+        // Avoid this potential crash by first invoking ViewOrCopy(buffer, memory_manager_device).
+        // This function tries to create a no-copy view of the buffer on the given memory
+        // manager device. If not possible, then ViewOrCopy copies the buffer's contents.
+        MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(
+            auto cpu_buffer, 
+            arrow::Buffer::ViewOrCopy(buffer, arrow::default_cpu_memory_manager()),
+            context, error::BUFFER_VIEW_OR_COPY_FAILED
+        );
+
+        const auto* data_begin = cpu_buffer->data();
+        const auto num_bytes = cpu_buffer->size();
+        // data_begin is a uint8_t*, so num_bytes is equal to the number of elements

Review Comment:
   Ah, OK, that makes sense. I think I misinterpreted this the first time I read it. My apologies.



##########
matlab/src/cpp/arrow/matlab/buffer/proxy/buffer.cc:
##########
@@ -0,0 +1,97 @@
+// 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.
+
+
+#include "arrow/matlab/buffer/proxy/buffer.h"
+#include "arrow/matlab/buffer/matlab_buffer.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::buffer::proxy {
+
+    Buffer::Buffer(std::shared_ptr<arrow::Buffer> buffer) : buffer{std::move(buffer)} {
+        
+        REGISTER_METHOD(Buffer, getNumBytes);
+        REGISTER_METHOD(Buffer, isEqual);
+        REGISTER_METHOD(Buffer, toMATLAB);
+    }
+
+    libmexclass::proxy::MakeResult Buffer::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+        namespace mda = ::matlab::data;
+        using BufferProxy = proxy::Buffer;
+        using MatlabBuffer = arrow::matlab::buffer::MatlabBuffer;
+
+        mda::StructArray opts = constructor_arguments[0];
+        const mda::TypedArray<uint8_t> values_mda = opts[0]["Values"];
+        auto buffer = std::make_shared<MatlabBuffer>(values_mda);
+        return std::make_shared<BufferProxy>(std::move(buffer));
+    }
+
+    std::shared_ptr<arrow::Buffer> Buffer::unwrap() {
+        return buffer;
+    }
+
+    void Buffer::getNumBytes(libmexclass::proxy::method::Context& context) {
+        namespace mda = ::matlab::data;
+        mda::ArrayFactory factory;        
+        auto num_bytes_mda = factory.createScalar(buffer->size());
+        context.outputs[0] = num_bytes_mda; 
+    }
+
+    void Buffer::isEqual(libmexclass::proxy::method::Context& context) { 
+        namespace mda = ::matlab::data;
+
+        bool is_equal = true;
+        const mda::TypedArray<uint64_t> buffer_proxy_ids = context.inputs[0];
+        for (const auto& buffer_proxy_id : buffer_proxy_ids) {
+            // Retrieve the Buffer proxy from the ProxyManager
+            auto proxy = libmexclass::proxy::ProxyManager::getProxy(buffer_proxy_id);
+            auto buffer_proxy = std::static_pointer_cast<proxy::Buffer>(proxy);
+            auto buffer_to_compare = buffer_proxy->unwrap();
+
+            if (!buffer->Equals(*buffer_to_compare)) {
+                is_equal = false;
+                break;
+            }
+        }
+        mda::ArrayFactory factory;
+        context.outputs[0] = factory.createScalar(is_equal);
+    }
+
+    void Buffer::toMATLAB(libmexclass::proxy::method::Context& context) { 
+        namespace mda = ::matlab::data;
+
+        // If buffer->is_cpu() returns false, invoking buffer->data() may cause a crash.
+        // Avoid this potential crash by first invoking ViewOrCopy(buffer, memory_manager_device).
+        // This function tries to create a no-copy view of the buffer on the given memory
+        // manager device. If not possible, then ViewOrCopy copies the buffer's contents.
+        MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(
+            auto cpu_buffer, 
+            arrow::Buffer::ViewOrCopy(buffer, arrow::default_cpu_memory_manager()),
+            context, error::BUFFER_VIEW_OR_COPY_FAILED
+        );
+
+        const auto* data_begin = cpu_buffer->data();
+        const auto num_bytes = cpu_buffer->size();
+        // data_begin is a uint8_t*, so num_bytes is equal to the number of elements

Review Comment:
   Marking this as resolved.



-- 
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


Re: [PR] GH-38015: [MATLAB] Add `arrow.buffer.Buffer` class to the MATLAB Interface [arrow]

Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on PR #38020:
URL: https://github.com/apache/arrow/pull/38020#issuecomment-1753434454

   > Sorry, I had some pending comments last week - but I accidentally never published them.
   
   No worries! I was out myself.


-- 
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