You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kevingurney (via GitHub)" <gi...@apache.org> on 2023/10/19 17:13:52 UTC
[PR] GH-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
kevingurney opened a new pull request, #38357:
URL: https://github.com/apache/arrow/pull/38357
### Rationale for this change
Now that many of the commonly-used "primitive" array types have been added to the MATLAB interface, we can implement an `arrow.array.ListArray` class.
This pull request adds a new `arrow.array.ListArray` class which can be converted to a MATLAB `cell` array by calling the static `toMATLAB` method.
### What changes are included in this PR?
1. Added a new `arrow.array.ListArray` MATLAB class.
2. Added a new `arrow.type.traits.ListTraits` MATLAB class.
### Are these changes tested?
Yes.
1. Added a new `tListArray.m` test class.
### Are there any user-facing changes?
Yes.
1. Users can now create an `arrow.array.ListArray` from an `offsets` and `values` array. `ListArray`s can be converted into MATLAB `cell` arrays by calling the static `arrow.array.ListArray.toMATLAB` method.
### Notes
1. We chose to use the "missing-class" `missing` value as the `NullSubstitutionValue` for the time being for `ListArray`. However, we eventually want to add `arrow.array.NullArray`, and will most likely want to use the "missing-class" `missing` value to represent `NullArray` values in MATLAB. So, this could cause some ambiguity in the future. We have been thinking about whether we should consider introducing some sort of special "sentinel value" to represent null values when converting to MATLAB `cell` arrays. Perhaps, something like `arrow.Null`, or something to that effect, in order to avoid this ambiguity. If we think it makes sense to do that, we may want to retroactively change the `NullSubstitutionValue` to be `arrow.Null` and break compatibility. Since we are still in pre-`0.1`, we don't think the impact of such a behavior change would be very large.
2. Implementing `ListArray` is fairly involved. So, in the spirit of incremental delivery, we chose not to include an implementation of `arrow.array.ListArray.fromMATLAB` in this initial pull request. We plan on following up with some more changes to `arrow.array.ListArray` in #38353 and #38354.
### Future Directions
1. #38353
2. #38354
3. Consider adding a null sentinel value like `arrow.Null` for conversion to MATLAB `cell` arrays.
--
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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38357:
URL: https://github.com/apache/arrow/pull/38357#discussion_r1366059621
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
+ using ListArrayProxy = ::arrow::matlab::array::proxy::ListArray;
+ using ArrayProxy = ::arrow::matlab::array::proxy::Array;
+
+ mda::StructArray opts = constructor_arguments[0];
+ const mda::TypedArray<uint64_t> offsets_proxy_id_mda = opts[0]["OffsetsProxyID"];
+ const mda::TypedArray<uint64_t> values_proxy_id_mda = opts[0]["ValuesProxyID"];
+ const mda::TypedArray<bool> validity_bitmap_mda = opts[0]["Valid"];
+
+ const auto offsets_proxy_id = offsets_proxy_id_mda[0];
+ const auto values_proxy_id = values_proxy_id_mda[0];
+
+ const auto offsets_proxy = std::static_pointer_cast<Int32ArrayProxy>(ProxyManager::getProxy(offsets_proxy_id));
+ const auto values_proxy = std::static_pointer_cast<ArrayProxy>(ProxyManager::getProxy(values_proxy_id));
+
+ const auto offsets = offsets_proxy->unwrap();
+ const auto values = values_proxy->unwrap();
+
+ // Pack the validity bitmap values.
+ MATLAB_ASSIGN_OR_ERROR(auto validity_bitmap_buffer,
+ bit::packValid(validity_bitmap_mda),
+ error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+
+ // Create a ListArray from values and offsets.
+ MATLAB_ASSIGN_OR_ERROR(auto array,
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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38357:
URL: https://github.com/apache/arrow/pull/38357#discussion_r1366211407
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
Review Comment:
Good point. Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on code in PR #38357:
URL: https://github.com/apache/arrow/pull/38357#discussion_r1365908049
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
+ using ListArrayProxy = ::arrow::matlab::array::proxy::ListArray;
+ using ArrayProxy = ::arrow::matlab::array::proxy::Array;
+
+ mda::StructArray opts = constructor_arguments[0];
+ const mda::TypedArray<uint64_t> offsets_proxy_id_mda = opts[0]["OffsetsProxyID"];
+ const mda::TypedArray<uint64_t> values_proxy_id_mda = opts[0]["ValuesProxyID"];
+ const mda::TypedArray<bool> validity_bitmap_mda = opts[0]["Valid"];
+
+ const auto offsets_proxy_id = offsets_proxy_id_mda[0];
+ const auto values_proxy_id = values_proxy_id_mda[0];
+
+ const auto offsets_proxy = std::static_pointer_cast<Int32ArrayProxy>(ProxyManager::getProxy(offsets_proxy_id));
+ const auto values_proxy = std::static_pointer_cast<ArrayProxy>(ProxyManager::getProxy(values_proxy_id));
+
+ const auto offsets = offsets_proxy->unwrap();
+ const auto values = values_proxy->unwrap();
+
+ // Pack the validity bitmap values.
+ MATLAB_ASSIGN_OR_ERROR(auto validity_bitmap_buffer,
+ bit::packValid(validity_bitmap_mda),
+ error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+
+ // Create a ListArray from values and offsets.
+ MATLAB_ASSIGN_OR_ERROR(auto array,
+ arrow::ListArray::FromArrays(*offsets, *values, arrow::default_memory_pool(), validity_bitmap_buffer),
+ error::LIST_ARRAY_FROM_ARRAYS_FAILED);
+
+ // Return a ListArray Proxy.
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ return std::make_shared<ListArrayProxy>(std::move(list_array));
+ }
+
+ void ListArray::getValues(libmexclass::proxy::method::Context& context) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ auto value_array = list_array->values();
+
+ // Wrap the array within a proxy object if possible.
+ MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(auto value_array_proxy,
Review Comment:
Just a general observation. We should add a utility function that manages the proxy and creates the `struct` returned. Would reduce code duplication since we do this in many places.
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
Review Comment:
I don't think you need to specify `::` (the global namespace) before `arrow` here. We need it above because `matlab` without the `::` prefix resolves to `arrow::matlab`.
##########
matlab/test/arrow/array/tListArray.m:
##########
@@ -0,0 +1,165 @@
+%TLISTARRAY Tests for arrow.array.ListArray
+
+% 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 tListArray < matlab.unittest.TestCase
+
+ properties (Constant)
+ Traits = arrow.type.traits.traits(arrow.type.ID.List)
+ end
+
+ properties (TestParameter)
+ TestArrowArray
+ end
+
+ methods (TestParameterDefinition, Static)
+
+ function TestArrowArray = initializeTestArrowArray()
+ %% Empty (zero-element) list
Review Comment:
Could you include the list type, i.e. List<Float64>
##########
matlab/test/arrow/array/tListArray.m:
##########
@@ -0,0 +1,165 @@
+%TLISTARRAY Tests for arrow.array.ListArray
+
+% 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 tListArray < matlab.unittest.TestCase
+
+ properties (Constant)
+ Traits = arrow.type.traits.traits(arrow.type.ID.List)
+ end
+
+ properties (TestParameter)
+ TestArrowArray
+ end
+
+ methods (TestParameterDefinition, Static)
+
+ function TestArrowArray = initializeTestArrowArray()
+ %% Empty (zero-element) list
+ Type = arrow.list(arrow.float64());
+ NumElements = int64(0);
+ Valid = logical.empty(0, 1);
+ Offsets = arrow.array(int32(0));
+ Values = arrow.array([]);
+ ArrowArray = arrow.array.ListArray.fromArrays(Offsets, Values, Valid=Valid);
+ MatlabArray = {cell.empty(0, 1)};
+
+ TestArrowArray.EmptyList = struct( ...
+ ArrowArray=ArrowArray, ...
+ MatlabArray=MatlabArray, ...
+ Properties=struct(...
+ Type=Type, ...
+ NumElements=NumElements, ...
+ Valid=Valid, ...
+ Offsets=Offsets, ...
+ Values=Values ...
+ ) ...
+ );
+
+ %% List with NULLs
Review Comment:
Could you mention the list type here too?
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
+ using ListArrayProxy = ::arrow::matlab::array::proxy::ListArray;
+ using ArrayProxy = ::arrow::matlab::array::proxy::Array;
+
+ mda::StructArray opts = constructor_arguments[0];
+ const mda::TypedArray<uint64_t> offsets_proxy_id_mda = opts[0]["OffsetsProxyID"];
+ const mda::TypedArray<uint64_t> values_proxy_id_mda = opts[0]["ValuesProxyID"];
+ const mda::TypedArray<bool> validity_bitmap_mda = opts[0]["Valid"];
+
+ const auto offsets_proxy_id = offsets_proxy_id_mda[0];
+ const auto values_proxy_id = values_proxy_id_mda[0];
+
+ const auto offsets_proxy = std::static_pointer_cast<Int32ArrayProxy>(ProxyManager::getProxy(offsets_proxy_id));
+ const auto values_proxy = std::static_pointer_cast<ArrayProxy>(ProxyManager::getProxy(values_proxy_id));
+
+ const auto offsets = offsets_proxy->unwrap();
+ const auto values = values_proxy->unwrap();
+
+ // Pack the validity bitmap values.
+ MATLAB_ASSIGN_OR_ERROR(auto validity_bitmap_buffer,
+ bit::packValid(validity_bitmap_mda),
+ error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+
+ // Create a ListArray from values and offsets.
+ MATLAB_ASSIGN_OR_ERROR(auto array,
Review Comment:
Maybe not in this PR, but should we add an option to validate the `Offsets` and `Values` array are correct?
--
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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38357:
URL: https://github.com/apache/arrow/pull/38357#discussion_r1366059058
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
+ using ListArrayProxy = ::arrow::matlab::array::proxy::ListArray;
+ using ArrayProxy = ::arrow::matlab::array::proxy::Array;
+
+ mda::StructArray opts = constructor_arguments[0];
+ const mda::TypedArray<uint64_t> offsets_proxy_id_mda = opts[0]["OffsetsProxyID"];
+ const mda::TypedArray<uint64_t> values_proxy_id_mda = opts[0]["ValuesProxyID"];
+ const mda::TypedArray<bool> validity_bitmap_mda = opts[0]["Valid"];
+
+ const auto offsets_proxy_id = offsets_proxy_id_mda[0];
+ const auto values_proxy_id = values_proxy_id_mda[0];
+
+ const auto offsets_proxy = std::static_pointer_cast<Int32ArrayProxy>(ProxyManager::getProxy(offsets_proxy_id));
+ const auto values_proxy = std::static_pointer_cast<ArrayProxy>(ProxyManager::getProxy(values_proxy_id));
+
+ const auto offsets = offsets_proxy->unwrap();
+ const auto values = values_proxy->unwrap();
+
+ // Pack the validity bitmap values.
+ MATLAB_ASSIGN_OR_ERROR(auto validity_bitmap_buffer,
+ bit::packValid(validity_bitmap_mda),
+ error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+
+ // Create a ListArray from values and offsets.
+ MATLAB_ASSIGN_OR_ERROR(auto array,
Review Comment:
Ah, yes! Thanks for the reminder! I meant to create a follow up issue for this, but I forgot.
I've created #38361 to track this.
--
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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38357:
URL: https://github.com/apache/arrow/pull/38357#issuecomment-1776217686
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 3beb93af36b3388a6871846365502c36ae4cfaa4.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/17982348684) has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce 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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38357:
URL: https://github.com/apache/arrow/pull/38357#discussion_r1366214518
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
+ using ListArrayProxy = ::arrow::matlab::array::proxy::ListArray;
+ using ArrayProxy = ::arrow::matlab::array::proxy::Array;
+
+ mda::StructArray opts = constructor_arguments[0];
+ const mda::TypedArray<uint64_t> offsets_proxy_id_mda = opts[0]["OffsetsProxyID"];
+ const mda::TypedArray<uint64_t> values_proxy_id_mda = opts[0]["ValuesProxyID"];
+ const mda::TypedArray<bool> validity_bitmap_mda = opts[0]["Valid"];
+
+ const auto offsets_proxy_id = offsets_proxy_id_mda[0];
+ const auto values_proxy_id = values_proxy_id_mda[0];
+
+ const auto offsets_proxy = std::static_pointer_cast<Int32ArrayProxy>(ProxyManager::getProxy(offsets_proxy_id));
+ const auto values_proxy = std::static_pointer_cast<ArrayProxy>(ProxyManager::getProxy(values_proxy_id));
+
+ const auto offsets = offsets_proxy->unwrap();
+ const auto values = values_proxy->unwrap();
+
+ // Pack the validity bitmap values.
+ MATLAB_ASSIGN_OR_ERROR(auto validity_bitmap_buffer,
+ bit::packValid(validity_bitmap_mda),
+ error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+
+ // Create a ListArray from values and offsets.
+ MATLAB_ASSIGN_OR_ERROR(auto array,
+ arrow::ListArray::FromArrays(*offsets, *values, arrow::default_memory_pool(), validity_bitmap_buffer),
+ error::LIST_ARRAY_FROM_ARRAYS_FAILED);
+
+ // Return a ListArray Proxy.
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ return std::make_shared<ListArrayProxy>(std::move(list_array));
+ }
+
+ void ListArray::getValues(libmexclass::proxy::method::Context& context) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ auto value_array = list_array->values();
+
+ // Wrap the array within a proxy object if possible.
+ MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(auto value_array_proxy,
Review Comment:
I've created #38369 to investigate creating helper utilities for `Proxy` creation/management. Please feel free to add additional context to the issue if you have any specific ideas for what you think would be helpful here.
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
+ using ListArrayProxy = ::arrow::matlab::array::proxy::ListArray;
+ using ArrayProxy = ::arrow::matlab::array::proxy::Array;
+
+ mda::StructArray opts = constructor_arguments[0];
+ const mda::TypedArray<uint64_t> offsets_proxy_id_mda = opts[0]["OffsetsProxyID"];
+ const mda::TypedArray<uint64_t> values_proxy_id_mda = opts[0]["ValuesProxyID"];
+ const mda::TypedArray<bool> validity_bitmap_mda = opts[0]["Valid"];
+
+ const auto offsets_proxy_id = offsets_proxy_id_mda[0];
+ const auto values_proxy_id = values_proxy_id_mda[0];
+
+ const auto offsets_proxy = std::static_pointer_cast<Int32ArrayProxy>(ProxyManager::getProxy(offsets_proxy_id));
+ const auto values_proxy = std::static_pointer_cast<ArrayProxy>(ProxyManager::getProxy(values_proxy_id));
+
+ const auto offsets = offsets_proxy->unwrap();
+ const auto values = values_proxy->unwrap();
+
+ // Pack the validity bitmap values.
+ MATLAB_ASSIGN_OR_ERROR(auto validity_bitmap_buffer,
+ bit::packValid(validity_bitmap_mda),
+ error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+
+ // Create a ListArray from values and offsets.
+ MATLAB_ASSIGN_OR_ERROR(auto array,
+ arrow::ListArray::FromArrays(*offsets, *values, arrow::default_memory_pool(), validity_bitmap_buffer),
+ error::LIST_ARRAY_FROM_ARRAYS_FAILED);
+
+ // Return a ListArray Proxy.
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ return std::make_shared<ListArrayProxy>(std::move(list_array));
+ }
+
+ void ListArray::getValues(libmexclass::proxy::method::Context& context) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ auto value_array = list_array->values();
+
+ // Wrap the array within a proxy object if possible.
+ MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(auto value_array_proxy,
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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38357:
URL: https://github.com/apache/arrow/pull/38357#discussion_r1366060656
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
+ using ListArrayProxy = ::arrow::matlab::array::proxy::ListArray;
+ using ArrayProxy = ::arrow::matlab::array::proxy::Array;
+
+ mda::StructArray opts = constructor_arguments[0];
+ const mda::TypedArray<uint64_t> offsets_proxy_id_mda = opts[0]["OffsetsProxyID"];
+ const mda::TypedArray<uint64_t> values_proxy_id_mda = opts[0]["ValuesProxyID"];
+ const mda::TypedArray<bool> validity_bitmap_mda = opts[0]["Valid"];
+
+ const auto offsets_proxy_id = offsets_proxy_id_mda[0];
+ const auto values_proxy_id = values_proxy_id_mda[0];
+
+ const auto offsets_proxy = std::static_pointer_cast<Int32ArrayProxy>(ProxyManager::getProxy(offsets_proxy_id));
+ const auto values_proxy = std::static_pointer_cast<ArrayProxy>(ProxyManager::getProxy(values_proxy_id));
+
+ const auto offsets = offsets_proxy->unwrap();
+ const auto values = values_proxy->unwrap();
+
+ // Pack the validity bitmap values.
+ MATLAB_ASSIGN_OR_ERROR(auto validity_bitmap_buffer,
+ bit::packValid(validity_bitmap_mda),
+ error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+
+ // Create a ListArray from values and offsets.
+ MATLAB_ASSIGN_OR_ERROR(auto array,
+ arrow::ListArray::FromArrays(*offsets, *values, arrow::default_memory_pool(), validity_bitmap_buffer),
+ error::LIST_ARRAY_FROM_ARRAYS_FAILED);
+
+ // Return a ListArray Proxy.
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ return std::make_shared<ListArrayProxy>(std::move(list_array));
+ }
+
+ void ListArray::getValues(libmexclass::proxy::method::Context& context) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ auto value_array = list_array->values();
+
+ // Wrap the array within a proxy object if possible.
+ MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(auto value_array_proxy,
Review Comment:
Agreed. This is a recurring code pattern, and it would definitely be worth investing some time into creating a utility for this.
--
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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38357:
URL: https://github.com/apache/arrow/pull/38357#discussion_r1366209662
##########
matlab/test/arrow/array/tListArray.m:
##########
@@ -0,0 +1,165 @@
+%TLISTARRAY Tests for arrow.array.ListArray
+
+% 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 tListArray < matlab.unittest.TestCase
+
+ properties (Constant)
+ Traits = arrow.type.traits.traits(arrow.type.ID.List)
+ end
+
+ properties (TestParameter)
+ TestArrowArray
+ end
+
+ methods (TestParameterDefinition, Static)
+
+ function TestArrowArray = initializeTestArrowArray()
+ %% Empty (zero-element) list
Review Comment:
Good catch! Fixed.
##########
matlab/test/arrow/array/tListArray.m:
##########
@@ -0,0 +1,165 @@
+%TLISTARRAY Tests for arrow.array.ListArray
+
+% 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 tListArray < matlab.unittest.TestCase
+
+ properties (Constant)
+ Traits = arrow.type.traits.traits(arrow.type.ID.List)
+ end
+
+ properties (TestParameter)
+ TestArrowArray
+ end
+
+ methods (TestParameterDefinition, Static)
+
+ function TestArrowArray = initializeTestArrowArray()
+ %% Empty (zero-element) list
+ Type = arrow.list(arrow.float64());
+ NumElements = int64(0);
+ Valid = logical.empty(0, 1);
+ Offsets = arrow.array(int32(0));
+ Values = arrow.array([]);
+ ArrowArray = arrow.array.ListArray.fromArrays(Offsets, Values, Valid=Valid);
+ MatlabArray = {cell.empty(0, 1)};
+
+ TestArrowArray.EmptyList = struct( ...
+ ArrowArray=ArrowArray, ...
+ MatlabArray=MatlabArray, ...
+ Properties=struct(...
+ Type=Type, ...
+ NumElements=NumElements, ...
+ Valid=Valid, ...
+ Offsets=Offsets, ...
+ Values=Values ...
+ ) ...
+ );
+
+ %% List with NULLs
Review Comment:
Fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney merged PR #38357:
URL: https://github.com/apache/arrow/pull/38357
--
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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38357:
URL: https://github.com/apache/arrow/pull/38357#discussion_r1366060656
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
+ using ListArrayProxy = ::arrow::matlab::array::proxy::ListArray;
+ using ArrayProxy = ::arrow::matlab::array::proxy::Array;
+
+ mda::StructArray opts = constructor_arguments[0];
+ const mda::TypedArray<uint64_t> offsets_proxy_id_mda = opts[0]["OffsetsProxyID"];
+ const mda::TypedArray<uint64_t> values_proxy_id_mda = opts[0]["ValuesProxyID"];
+ const mda::TypedArray<bool> validity_bitmap_mda = opts[0]["Valid"];
+
+ const auto offsets_proxy_id = offsets_proxy_id_mda[0];
+ const auto values_proxy_id = values_proxy_id_mda[0];
+
+ const auto offsets_proxy = std::static_pointer_cast<Int32ArrayProxy>(ProxyManager::getProxy(offsets_proxy_id));
+ const auto values_proxy = std::static_pointer_cast<ArrayProxy>(ProxyManager::getProxy(values_proxy_id));
+
+ const auto offsets = offsets_proxy->unwrap();
+ const auto values = values_proxy->unwrap();
+
+ // Pack the validity bitmap values.
+ MATLAB_ASSIGN_OR_ERROR(auto validity_bitmap_buffer,
+ bit::packValid(validity_bitmap_mda),
+ error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+
+ // Create a ListArray from values and offsets.
+ MATLAB_ASSIGN_OR_ERROR(auto array,
+ arrow::ListArray::FromArrays(*offsets, *values, arrow::default_memory_pool(), validity_bitmap_buffer),
+ error::LIST_ARRAY_FROM_ARRAYS_FAILED);
+
+ // Return a ListArray Proxy.
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ return std::make_shared<ListArrayProxy>(std::move(list_array));
+ }
+
+ void ListArray::getValues(libmexclass::proxy::method::Context& context) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ auto value_array = list_array->values();
+
+ // Wrap the array within a proxy object if possible.
+ MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(auto value_array_proxy,
Review Comment:
Agreed. This is a recurring code pattern, and it would definitely be worth investing some time in creating a utility for this.
--
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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on code in PR #38357:
URL: https://github.com/apache/arrow/pull/38357#discussion_r1366214518
##########
matlab/src/cpp/arrow/matlab/array/proxy/list_array.cc:
##########
@@ -0,0 +1,103 @@
+// 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/array/proxy/list_array.h"
+#include "arrow/matlab/array/proxy/numeric_array.h"
+#include "arrow/matlab/array/proxy/wrap.h"
+#include "arrow/matlab/error/error.h"
+#include "libmexclass/proxy/ProxyManager.h"
+
+namespace arrow::matlab::array::proxy {
+
+ ListArray::ListArray(std::shared_ptr<arrow::ListArray> list_array) : proxy::Array{std::move(list_array)} {
+ REGISTER_METHOD(ListArray, getValues);
+ REGISTER_METHOD(ListArray, getOffsets);
+ }
+
+ libmexclass::proxy::MakeResult ListArray::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+ using Int32ArrayProxy = ::arrow::matlab::array::proxy::NumericArray<arrow::Int32Type>;
+ using ListArrayProxy = ::arrow::matlab::array::proxy::ListArray;
+ using ArrayProxy = ::arrow::matlab::array::proxy::Array;
+
+ mda::StructArray opts = constructor_arguments[0];
+ const mda::TypedArray<uint64_t> offsets_proxy_id_mda = opts[0]["OffsetsProxyID"];
+ const mda::TypedArray<uint64_t> values_proxy_id_mda = opts[0]["ValuesProxyID"];
+ const mda::TypedArray<bool> validity_bitmap_mda = opts[0]["Valid"];
+
+ const auto offsets_proxy_id = offsets_proxy_id_mda[0];
+ const auto values_proxy_id = values_proxy_id_mda[0];
+
+ const auto offsets_proxy = std::static_pointer_cast<Int32ArrayProxy>(ProxyManager::getProxy(offsets_proxy_id));
+ const auto values_proxy = std::static_pointer_cast<ArrayProxy>(ProxyManager::getProxy(values_proxy_id));
+
+ const auto offsets = offsets_proxy->unwrap();
+ const auto values = values_proxy->unwrap();
+
+ // Pack the validity bitmap values.
+ MATLAB_ASSIGN_OR_ERROR(auto validity_bitmap_buffer,
+ bit::packValid(validity_bitmap_mda),
+ error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+
+ // Create a ListArray from values and offsets.
+ MATLAB_ASSIGN_OR_ERROR(auto array,
+ arrow::ListArray::FromArrays(*offsets, *values, arrow::default_memory_pool(), validity_bitmap_buffer),
+ error::LIST_ARRAY_FROM_ARRAYS_FAILED);
+
+ // Return a ListArray Proxy.
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ return std::make_shared<ListArrayProxy>(std::move(list_array));
+ }
+
+ void ListArray::getValues(libmexclass::proxy::method::Context& context) {
+ namespace mda = ::matlab::data;
+ using libmexclass::proxy::ProxyManager;
+
+ auto list_array = std::static_pointer_cast<arrow::ListArray>(array);
+ auto value_array = list_array->values();
+
+ // Wrap the array within a proxy object if possible.
+ MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(auto value_array_proxy,
Review Comment:
I've created #38369 to investigate creating helper utilities for `Proxy` creation/management.
--
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-37815: [MATLAB] Add `arrow.array.ListArray` MATLAB class [arrow]
Posted by "kevingurney (via GitHub)" <gi...@apache.org>.
kevingurney commented on PR #38357:
URL: https://github.com/apache/arrow/pull/38357#issuecomment-1775475198
+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