This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 407423394c GH-35914: [MATLAB] Integrate the latest libmexclass changes
to support error-handling (#35918)
407423394c is described below
commit 407423394cb988669a0b5716899810a924c0aa4c
Author: sgilmore10 <[email protected]>
AuthorDate: Wed Jun 7 22:01:48 2023 -0400
GH-35914: [MATLAB] Integrate the latest libmexclass changes to support
error-handling (#35918)
### Rationale for this change
This change integrates the latest version of `mathworks/libmexclass` into
the MATLAB interface which enables throwing MATLAB errors.
The
[77f3d72](https://github.com/mathworks/libmexclass/commit/77f3d72c22a9ddab7b54ba325d757c3e82e57987)
in `libmexclass` introduced the following changes:
1. Added a new class called `libmexclass::error::Error`.
2. Added a new field called `error` on the
`libmexclass::proxy::method::Context` which is a
`std::optional<libmexclass::proxy::Error>`. By default, this is a
`std::nullopt`. To make MATLAB throw an error, set this optional.
3. To support throwing errors at construction, `libmexclass` now requires
all `Proxy` subclasses to define a static `make` method: `static
libmexclass::proxy::MakeResult make(const
libmexclass::proxy::FunctionArguments& constructor_arguments)`. This workflow
is similar to using an `arrow::Result<T>`object.
Examples of throwing errors in MATLAB can be found on lines
[45](https://github.com/mathworks/libmexclass/blob/77f3d72c22a9ddab7b54ba325d757c3e82e57987/example/proxy/Car.cpp#L45)
and
[94](https://github.com/mathworks/libmexclass/blob/77f3d72c22a9ddab7b54ba325d757c3e82e57987/example/proxy/Car.cpp#L94)
in
[libmexclass/example/proxy/Car.cpp](https://github.com/mathworks/libmexclass/blob/main/example/proxy/Car.cpp)
### What changes are included in this PR?
1. Pulled in the latest version of `libmexclass`:
[77f3d72](https://github.com/mathworks/libmexclass/commit/77f3d72c22a9ddab7b54ba325d757c3e82e57987).
2. Added `static libmexclass::proxy::MakeResult make(const
libmexclass::proxy::FunctionArguments& constructor_arguments)` to
`arrow::matlab::proxy::NumericArray<Type>`.
3. Throw an error when trying to create an unknown proxy class.
### Are these changes tested?
1. Added a new test class called `tGateway.m` that verifies
`libmexclass.proxy.gateway("Create", ...)` errors if given the name of an
unknown proxy class.
### Are there any user-facing changes?
No.
* Closes: #35914
Lead-authored-by: Sarah Gilmore <[email protected]>
Co-authored-by: sgilmore10 <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Co-authored-by: Kevin Gurney <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
matlab/src/cpp/arrow/matlab/array/proxy/array.cc | 2 +-
matlab/src/cpp/arrow/matlab/array/proxy/array.h | 2 +-
.../cpp/arrow/matlab/array/proxy/numeric_array.h | 41 +++++++++++++---------
matlab/src/cpp/arrow/matlab/error/error.h | 40 +++++++++++++++++++++
matlab/src/cpp/arrow/matlab/proxy/factory.cc | 8 ++---
matlab/src/cpp/arrow/matlab/proxy/factory.h | 2 +-
matlab/test/arrow/gateway/tGateway.m | 28 +++++++++++++++
matlab/tools/cmake/BuildMatlabArrowInterface.cmake | 6 ++--
8 files changed, 103 insertions(+), 26 deletions(-)
diff --git a/matlab/src/cpp/arrow/matlab/array/proxy/array.cc
b/matlab/src/cpp/arrow/matlab/array/proxy/array.cc
index fc1d66ae24..56115f5a37 100644
--- a/matlab/src/cpp/arrow/matlab/array/proxy/array.cc
+++ b/matlab/src/cpp/arrow/matlab/array/proxy/array.cc
@@ -21,7 +21,7 @@
namespace arrow::matlab::array::proxy {
- Array::Array(const libmexclass::proxy::FunctionArguments&
constructor_arguments) {
+ Array::Array() {
// Register Proxy methods.
REGISTER_METHOD(Array, toString);
diff --git a/matlab/src/cpp/arrow/matlab/array/proxy/array.h
b/matlab/src/cpp/arrow/matlab/array/proxy/array.h
index 0a69f6fcad..859dc48571 100644
--- a/matlab/src/cpp/arrow/matlab/array/proxy/array.h
+++ b/matlab/src/cpp/arrow/matlab/array/proxy/array.h
@@ -25,7 +25,7 @@ namespace arrow::matlab::array::proxy {
class Array : public libmexclass::proxy::Proxy {
public:
- Array(const libmexclass::proxy::FunctionArguments&
constructor_arguments);
+ Array();
virtual ~Array() {}
diff --git a/matlab/src/cpp/arrow/matlab/array/proxy/numeric_array.h
b/matlab/src/cpp/arrow/matlab/array/proxy/numeric_array.h
index ad2242a755..019add312d 100644
--- a/matlab/src/cpp/arrow/matlab/array/proxy/numeric_array.h
+++ b/matlab/src/cpp/arrow/matlab/array/proxy/numeric_array.h
@@ -17,7 +17,6 @@
#pragma once
-
#include "arrow/array.h"
#include "arrow/array/data.h"
#include "arrow/array/util.h"
@@ -26,6 +25,7 @@
#include "arrow/type_traits.h"
#include "arrow/matlab/array/proxy/array.h"
+#include "arrow/matlab/error/error.h"
#include "arrow/matlab/bit/bit_pack_matlab_logical_array.h"
#include "libmexclass/proxy/Proxy.h"
@@ -42,8 +42,12 @@ const uint8_t* getUnpackedValidityBitmap(const
::matlab::data::TypedArray<bool>&
template<typename CType>
class NumericArray : public arrow::matlab::array::proxy::Array {
public:
- NumericArray(const libmexclass::proxy::FunctionArguments&
constructor_arguments)
- : arrow::matlab::array::proxy::Array(constructor_arguments) {
+ NumericArray(const std::shared_ptr<arrow::Array> numeric_array)
+ : arrow::matlab::array::proxy::Array() {
+ array = numeric_array;
+ }
+
+ static libmexclass::proxy::MakeResult make(const
libmexclass::proxy::FunctionArguments& constructor_arguments) {
using ArrowType = typename arrow::CTypeTraits<CType>::ArrowType;
using BuilderType = typename
arrow::CTypeTraits<CType>::BuilderType;
@@ -64,15 +68,16 @@ class NumericArray : public
arrow::matlab::array::proxy::Array {
auto unpacked_validity_bitmap = has_validity_bitmap ?
getUnpackedValidityBitmap(constructor_arguments[2]) : nullptr;
BuilderType builder;
- auto st = builder.AppendValues(dt,
numeric_mda.getNumberOfElements(), unpacked_validity_bitmap);
-
- // TODO: handle error case
- if (st.ok()) {
- auto maybe_array = builder.Finish();
- if (maybe_array.ok()) {
- array = *maybe_array;
- }
- }
+
+
+ auto status = builder.AppendValues(dt,
numeric_mda.getNumberOfElements(), unpacked_validity_bitmap);
+ MATLAB_ERROR_IF_NOT_OK(status, error::APPEND_VALUES_ERROR_ID);
+
+ auto maybe_array = builder.Finish();
+ MATLAB_ERROR_IF_NOT_OK(maybe_array.status(),
error::BUILD_ARRAY_ERROR_ID);
+
+ return
std::make_shared<arrow::matlab::array::proxy::NumericArray<CType>>(std::move(maybe_array).ValueUnsafe());
+
} else {
const auto data_type =
arrow::CTypeTraits<CType>::type_singleton();
const auto length =
static_cast<int64_t>(numeric_mda.getNumberOfElements()); // cast size_t to
int64_t
@@ -81,11 +86,15 @@ class NumericArray : public
arrow::matlab::array::proxy::Array {
auto data_buffer =
std::make_shared<arrow::Buffer>(reinterpret_cast<const uint8_t*>(dt),
sizeof(CType) *
numeric_mda.getNumberOfElements());
- // Pack the validity bitmap values.
- auto packed_validity_bitmap = has_validity_bitmap ?
arrow::matlab::bit::bitPackMatlabLogicalArray(constructor_arguments[2]).ValueOrDie()
: nullptr;
-
+ std::shared_ptr<arrow::Buffer> packed_validity_bitmap;
+ if (has_validity_bitmap) {
+ // Pack the validity bitmap values.
+ auto maybe_buffer =
arrow::matlab::bit::bitPackMatlabLogicalArray(constructor_arguments[2]);
+ MATLAB_ERROR_IF_NOT_OK(maybe_buffer.status(),
error::BITPACK_VALIDITY_BITMAP_ERROR_ID);
+ packed_validity_bitmap =
std::move(maybe_buffer).ValueUnsafe();
+ }
auto array_data = arrow::ArrayData::Make(data_type, length,
{packed_validity_bitmap, data_buffer});
- array = arrow::MakeArray(array_data);
+ return
std::make_shared<arrow::matlab::array::proxy::NumericArray<CType>>(arrow::MakeArray(array_data));
}
}
diff --git a/matlab/src/cpp/arrow/matlab/error/error.h
b/matlab/src/cpp/arrow/matlab/error/error.h
new file mode 100644
index 0000000000..d54d276735
--- /dev/null
+++ b/matlab/src/cpp/arrow/matlab/error/error.h
@@ -0,0 +1,40 @@
+// 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/status.h"
+#include "libmexclass/error/Error.h"
+
+#include <string_view>
+
+#define MATLAB_ERROR_IF_NOT_OK(expr, id) \
+ do { \
+ arrow::Status _status = (expr); \
+ if (!_status.ok()) { \
+ return libmexclass::error::Error{(id), _status.message()}; \
+ } \
+ } while (0)
+
+namespace arrow::matlab::error {
+ // TODO: Make Error ID Enum class to avoid defining static constexpr
+ static const char* APPEND_VALUES_ERROR_ID =
"arrow:matlab:proxy:make:FailedToAppendValues";
+ static const char* BUILD_ARRAY_ERROR_ID =
"arrow:matlab:proxy:make:FailedToAppendValues";
+ static const char* BITPACK_VALIDITY_BITMAP_ERROR_ID =
"arrow:matlab:proxy:make:FailedToBitPackValidityBitmap";
+ static const char* UNKNOWN_PROXY_ERROR_ID =
"arrow:matlab:proxy:UnknownProxy";
+}
diff --git a/matlab/src/cpp/arrow/matlab/proxy/factory.cc
b/matlab/src/cpp/arrow/matlab/proxy/factory.cc
index b7f86b9fcf..e159c0ea37 100644
--- a/matlab/src/cpp/arrow/matlab/proxy/factory.cc
+++ b/matlab/src/cpp/arrow/matlab/proxy/factory.cc
@@ -18,12 +18,12 @@
#include "arrow/matlab/array/proxy/numeric_array.h"
#include "factory.h"
-
+#include "arrow/matlab/error/error.h"
#include <iostream>
namespace arrow::matlab::proxy {
-std::shared_ptr<Proxy> Factory::make_proxy(const ClassName& class_name, const
FunctionArguments& constructor_arguments) {
+libmexclass::proxy::MakeResult Factory::make_proxy(const ClassName&
class_name, const FunctionArguments& constructor_arguments) {
// Register MATLAB Proxy classes with corresponding C++ Proxy classes.
REGISTER_PROXY(arrow.array.proxy.Float32Array,
arrow::matlab::array::proxy::NumericArray<float>);
REGISTER_PROXY(arrow.array.proxy.Float64Array,
arrow::matlab::array::proxy::NumericArray<double>);
@@ -38,9 +38,7 @@ std::shared_ptr<Proxy> Factory::make_proxy(const ClassName&
class_name, const Fu
REGISTER_PROXY(arrow.array.proxy.Int32Array ,
arrow::matlab::array::proxy::NumericArray<int32_t>);
REGISTER_PROXY(arrow.array.proxy.Int64Array ,
arrow::matlab::array::proxy::NumericArray<int64_t>);
- // TODO: Decide what to do in the case that there isn't a Proxy match.
- std::cout << "Did not find a matching C++ proxy for: " + class_name <<
std::endl;
- return nullptr;
+ return libmexclass::error::Error{error::UNKNOWN_PROXY_ERROR_ID, "Did not
find matching C++ proxy for " + class_name};
};
}
diff --git a/matlab/src/cpp/arrow/matlab/proxy/factory.h
b/matlab/src/cpp/arrow/matlab/proxy/factory.h
index 6f68ff4ac9..fd41a1c10a 100644
--- a/matlab/src/cpp/arrow/matlab/proxy/factory.h
+++ b/matlab/src/cpp/arrow/matlab/proxy/factory.h
@@ -26,7 +26,7 @@ using namespace libmexclass::proxy;
class Factory : public libmexclass::proxy::Factory {
public:
Factory() { }
- virtual std::shared_ptr<Proxy> make_proxy(const ClassName& class_name,
const FunctionArguments& constructor_arguments);
+ virtual libmexclass::proxy::MakeResult make_proxy(const ClassName&
class_name, const FunctionArguments& constructor_arguments);
};
}
diff --git a/matlab/test/arrow/gateway/tGateway.m
b/matlab/test/arrow/gateway/tGateway.m
new file mode 100644
index 0000000000..c2b9ef9d68
--- /dev/null
+++ b/matlab/test/arrow/gateway/tGateway.m
@@ -0,0 +1,28 @@
+% 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 tGateway < matlab.unittest.TestCase
+ % Tests for libmexclass.proxy.gateway error conditions.
+
+ methods (Test)
+ function UnknownProxyError(testCase)
+ % Verify the gateway function errors if given the name of an
+ % unknown proxy class.
+ id = "arrow:matlab:proxy:UnknownProxy";
+ fcn = @()libmexclass.proxy.gateway("Create", "NotAProxyClass", {});
+ testCase.verifyError(fcn, id);
+ end
+ end
+end
\ No newline at end of file
diff --git a/matlab/tools/cmake/BuildMatlabArrowInterface.cmake
b/matlab/tools/cmake/BuildMatlabArrowInterface.cmake
index 0dda3fb770..1f4ab05b06 100644
--- a/matlab/tools/cmake/BuildMatlabArrowInterface.cmake
+++ b/matlab/tools/cmake/BuildMatlabArrowInterface.cmake
@@ -24,7 +24,8 @@ set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_FETCH_CONTENT_NAME
libmexclass)
# libmexclass is accessible for CI without permission issues.
set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_FETCH_CONTENT_GIT_REPOSITORY
"https://github.com/mathworks/libmexclass.git";)
# Use a specific Git commit hash to avoid libmexclass version changing
unexpectedly.
-set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_FETCH_CONTENT_GIT_TAG "44c15d0")
+set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_FETCH_CONTENT_GIT_TAG "77f3d72")
+
set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_FETCH_CONTENT_SOURCE_SUBDIR
"libmexclass/cpp")
# ------------------------------------------
@@ -34,7 +35,8 @@
set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_FETCH_CONTENT_SOURCE_SUBDIR "libmexclass/cpp
set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_PROXY_LIBRARY_NAME arrowproxy)
set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_PROXY_LIBRARY_ROOT_INCLUDE_DIR
"${CMAKE_SOURCE_DIR}/src/cpp")
set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_PROXY_INCLUDE_DIR
"${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/array/proxy"
-
"${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/bit")
+
"${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/bit"
+
"${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/error")
set(MATLAB_ARROW_LIBMEXCLASS_CLIENT_PROXY_SOURCES
"${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/array/proxy/array.cc"
"${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/bit/bit_pack_matlab_logical_array.cc"
"${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/bit/bit_unpack_arrow_buffer.cc")
