lidavidm commented on code in PR #261:
URL: https://github.com/apache/arrow-cookbook/pull/261#discussion_r981102285
##########
cpp/code/basic_arrow.cc:
##########
@@ -70,48 +76,48 @@ TEST(BasicArrow, ReturnNotOk) { ASSERT_OK(ReturnNotOk()); }
/// Only supports floating point and integral types. Does not support decimals.
class TableSummation {
double partial = 0.0;
- public:
+ public:
arrow::Result<double> Compute(std::shared_ptr<arrow::RecordBatch> batch) {
- for (std::shared_ptr<arrow::Array> array : batch->columns()) {
+ for (const auto& array : batch->columns()) {
ARROW_RETURN_NOT_OK(arrow::VisitArrayInline(*array, this));
}
return partial;
}
// Default implementation
- arrow::Status Visit(const arrow::Array& array) {
- return arrow::Status::NotImplemented("Can not compute sum for array of
type ",
- array.type()->ToString());
- }
-
template <typename ArrayType, typename T = typename ArrayType::TypeClass>
- arrow::enable_if_number<T, arrow::Status> Visit(const ArrayType& array) {
- for (arrow::util::optional<typename T::c_type> value : array) {
- if (value.has_value()) {
- partial += static_cast<double>(value.value());
+ arrow::Status Visit(const ArrayType& array) {
+ if constexpr (arrow::is_number_type<T>::value) {
+ for (auto value : array) {
+ if (value.has_value()) {
+ partial += static_cast<double>(value.value());
+ }
}
+ return arrow::Status::OK();
+ } else {
+ return arrow::Status::NotImplemented("Can not compute sum for array of
type ",
+ array.type()->ToString());
}
- return arrow::Status::OK();
}
}; // TableSummation
arrow::Status VisitorSummationExample() {
StartRecipe("VisitorSummationExample");
- std::shared_ptr<arrow::Schema> schema = arrow::schema({
Review Comment:
Note that in general, it was consciously chosen to write out return types to
be explicit for readers who may be unfamiliar with Arrow. Some things like
lambdas and initializing a builder on the stack make sense to simplify to me,
but helpers like `schema` should be written out.
##########
cpp/code/basic_arrow.cc:
##########
@@ -35,6 +35,11 @@ arrow::Status ReturnNotOkMacro() {
if (!st.ok()) {
return st;
}
+ auto res = builder.Finish();
+ std::shared_ptr<arrow::Array> arr;
+ if (res.ok()) {
+ arr = std::move(res).ValueUnsafe();
+ }
Review Comment:
This recipe is specifically trying to demonstrate how `Status` works and not
how to construct an Array, really.
##########
cpp/source/basic.rst:
##########
@@ -41,9 +41,9 @@ tedious:
:caption: Checking the status of every function manually
:dedent: 2
-The macro :c:macro:`ARROW_RETURN_NOT_OK` will take care of some of this
-boilerplate for you. It will run the contained expression and check the
resulting
-``Status`` or ``Result`` object. If it failed then it will return the failure.
+The macros :c:macro:`ARROW_RETURN_NOT_OK` and :c:macro:`ARROW_ASSIGN_OR_RAISE`
will take care of some of these
+boilerplates for you. They will run the contained expression and check the
resulting
+``Status`` or ``Result`` object. If they failed then they will return the
failure.
Review Comment:
For clarity, I think Status and Result should be introduced separately
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
