This is an automated email from the ASF dual-hosted git repository.
apitrou 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 cd7ebc0b47 GH-41953: [C++] Minor enhance code style for
FixedShapeTensorType (#41954)
cd7ebc0b47 is described below
commit cd7ebc0b47668339f315b4ba224ce271c46c6cf5
Author: mwish <[email protected]>
AuthorDate: Wed Jun 5 23:21:14 2024 +0800
GH-41953: [C++] Minor enhance code style for FixedShapeTensorType (#41954)
### Rationale for this change
Minor enhance code style for FixedShapeTensorType
### What changes are included in this PR?
1. Remove some `shared_ptr` temp variables
2. Some interfaces allowing return reference
### Are these changes tested?
Covered by existing
### Are there any user-facing changes?
no
* GitHub Issue: #41953
Authored-by: mwish <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/extension/fixed_shape_tensor.cc | 66 ++++++++++++++-------------
cpp/src/arrow/extension/fixed_shape_tensor.h | 4 +-
2 files changed, 36 insertions(+), 34 deletions(-)
diff --git a/cpp/src/arrow/extension/fixed_shape_tensor.cc
b/cpp/src/arrow/extension/fixed_shape_tensor.cc
index 1101b08307..944a134a70 100644
--- a/cpp/src/arrow/extension/fixed_shape_tensor.cc
+++ b/cpp/src/arrow/extension/fixed_shape_tensor.cc
@@ -207,44 +207,44 @@ std::shared_ptr<Array> FixedShapeTensorType::MakeArray(
Result<std::shared_ptr<Tensor>> FixedShapeTensorType::MakeTensor(
const std::shared_ptr<ExtensionScalar>& scalar) {
- const auto ext_scalar =
internal::checked_pointer_cast<ExtensionScalar>(scalar);
- const auto ext_type =
- internal::checked_pointer_cast<FixedShapeTensorType>(scalar->type);
- if (!is_fixed_width(*ext_type->value_type())) {
+ const auto& ext_scalar = internal::checked_cast<const
ExtensionScalar&>(*scalar);
+ const auto& ext_type =
+ internal::checked_cast<const FixedShapeTensorType&>(*scalar->type);
+ if (!is_fixed_width(*ext_type.value_type())) {
return Status::TypeError("Cannot convert non-fixed-width values to
Tensor.");
}
- const auto array =
- internal::checked_pointer_cast<const
FixedSizeListScalar>(ext_scalar->value)->value;
+ const auto& array =
+ internal::checked_cast<const
FixedSizeListScalar*>(ext_scalar.value.get())->value;
if (array->null_count() > 0) {
return Status::Invalid("Cannot convert data with nulls to Tensor.");
}
- const auto value_type =
- internal::checked_pointer_cast<FixedWidthType>(ext_type->value_type());
- const auto byte_width = value_type->byte_width();
+ const auto& value_type =
+ internal::checked_cast<const FixedWidthType&>(*ext_type.value_type());
+ const auto byte_width = value_type.byte_width();
- std::vector<int64_t> permutation = ext_type->permutation();
+ std::vector<int64_t> permutation = ext_type.permutation();
if (permutation.empty()) {
- permutation.resize(ext_type->ndim());
+ permutation.resize(ext_type.ndim());
std::iota(permutation.begin(), permutation.end(), 0);
}
- std::vector<int64_t> shape = ext_type->shape();
+ std::vector<int64_t> shape = ext_type.shape();
internal::Permute<int64_t>(permutation, &shape);
- std::vector<std::string> dim_names = ext_type->dim_names();
+ std::vector<std::string> dim_names = ext_type.dim_names();
if (!dim_names.empty()) {
internal::Permute<std::string>(permutation, &dim_names);
}
std::vector<int64_t> strides;
- RETURN_NOT_OK(ComputeStrides(*value_type.get(), shape, permutation,
&strides));
+ RETURN_NOT_OK(ComputeStrides(value_type, shape, permutation, &strides));
const auto start_position = array->offset() * byte_width;
const auto size = std::accumulate(shape.begin(), shape.end(),
static_cast<int64_t>(1),
std::multiplies<>());
const auto buffer =
SliceBuffer(array->data()->buffers[1], start_position, size *
byte_width);
- return Tensor::Make(ext_type->value_type(), buffer, shape, strides,
dim_names);
+ return Tensor::Make(ext_type.value_type(), buffer, shape, strides,
dim_names);
}
Result<std::shared_ptr<FixedShapeTensorArray>>
FixedShapeTensorArray::FromTensor(
@@ -257,12 +257,14 @@ Result<std::shared_ptr<FixedShapeTensorArray>>
FixedShapeTensorArray::FromTensor
permutation.erase(permutation.begin());
std::vector<int64_t> cell_shape;
+ cell_shape.reserve(permutation.size());
for (auto i : permutation) {
cell_shape.emplace_back(tensor->shape()[i]);
}
std::vector<std::string> dim_names;
if (!tensor->dim_names().empty()) {
+ dim_names.reserve(permutation.size());
for (auto i : permutation) {
dim_names.emplace_back(tensor->dim_names()[i]);
}
@@ -337,9 +339,9 @@ const Result<std::shared_ptr<Tensor>>
FixedShapeTensorArray::ToTensor() const {
// To convert an array of n dimensional tensors to a n+1 dimensional tensor
we
// interpret the array's length as the first dimension the new tensor.
- const auto ext_type =
- internal::checked_pointer_cast<FixedShapeTensorType>(this->type());
- const auto value_type = ext_type->value_type();
+ const auto& ext_type =
+ internal::checked_cast<const FixedShapeTensorType&>(*this->type());
+ const auto& value_type = ext_type.value_type();
ARROW_RETURN_IF(
!is_fixed_width(*value_type),
Status::TypeError(value_type->ToString(), " is not valid data type for a
tensor"));
@@ -350,35 +352,35 @@ const Result<std::shared_ptr<Tensor>>
FixedShapeTensorArray::ToTensor() const {
// will get permutation index 0 and remaining values from
ext_type->permutation() need
// to be shifted to fill the [1, ndim+1) range. Computed permutation will be
used to
// generate the new tensor's shape, strides and dim_names.
- std::vector<int64_t> permutation = ext_type->permutation();
+ std::vector<int64_t> permutation = ext_type.permutation();
if (permutation.empty()) {
- permutation.resize(ext_type->ndim() + 1);
+ permutation.resize(ext_type.ndim() + 1);
std::iota(permutation.begin(), permutation.end(), 0);
} else {
- for (auto i = 0; i < static_cast<int64_t>(ext_type->ndim()); i++) {
+ for (auto i = 0; i < static_cast<int64_t>(ext_type.ndim()); i++) {
permutation[i] += 1;
}
permutation.insert(permutation.begin(), 1, 0);
}
- std::vector<std::string> dim_names = ext_type->dim_names();
+ std::vector<std::string> dim_names = ext_type.dim_names();
if (!dim_names.empty()) {
dim_names.insert(dim_names.begin(), 1, "");
internal::Permute<std::string>(permutation, &dim_names);
}
- std::vector<int64_t> shape = ext_type->shape();
+ std::vector<int64_t> shape = ext_type.shape();
auto cell_size = std::accumulate(shape.begin(), shape.end(),
static_cast<int64_t>(1),
std::multiplies<>());
shape.insert(shape.begin(), 1, this->length());
internal::Permute<int64_t>(permutation, &shape);
std::vector<int64_t> tensor_strides;
- const auto fw_value_type =
internal::checked_pointer_cast<FixedWidthType>(value_type);
+ const auto* fw_value_type =
internal::checked_cast<FixedWidthType*>(value_type.get());
ARROW_RETURN_NOT_OK(
- ComputeStrides(*fw_value_type.get(), shape, permutation,
&tensor_strides));
+ ComputeStrides(*fw_value_type, shape, permutation, &tensor_strides));
- const auto raw_buffer = this->storage()->data()->child_data[0]->buffers[1];
+ const auto& raw_buffer = this->storage()->data()->child_data[0]->buffers[1];
ARROW_ASSIGN_OR_RAISE(
const auto buffer,
SliceBufferSafe(raw_buffer, this->offset() * cell_size *
value_type->byte_width()));
@@ -389,7 +391,7 @@ const Result<std::shared_ptr<Tensor>>
FixedShapeTensorArray::ToTensor() const {
Result<std::shared_ptr<DataType>> FixedShapeTensorType::Make(
const std::shared_ptr<DataType>& value_type, const std::vector<int64_t>&
shape,
const std::vector<int64_t>& permutation, const std::vector<std::string>&
dim_names) {
- const auto ndim = shape.size();
+ const size_t ndim = shape.size();
if (!permutation.empty() && ndim != permutation.size()) {
return Status::Invalid("permutation size must match shape size. Expected:
", ndim,
" Got: ", permutation.size());
@@ -402,18 +404,18 @@ Result<std::shared_ptr<DataType>>
FixedShapeTensorType::Make(
RETURN_NOT_OK(internal::IsPermutationValid(permutation));
}
- const auto size = std::accumulate(shape.begin(), shape.end(),
static_cast<int64_t>(1),
- std::multiplies<>());
+ const int64_t size = std::accumulate(shape.begin(), shape.end(),
+ static_cast<int64_t>(1),
std::multiplies<>());
return std::make_shared<FixedShapeTensorType>(value_type,
static_cast<int32_t>(size),
shape, permutation, dim_names);
}
const std::vector<int64_t>& FixedShapeTensorType::strides() {
if (strides_.empty()) {
- auto value_type =
internal::checked_pointer_cast<FixedWidthType>(this->value_type_);
+ auto value_type =
internal::checked_cast<FixedWidthType*>(this->value_type_.get());
std::vector<int64_t> tensor_strides;
- ARROW_CHECK_OK(ComputeStrides(*value_type.get(), this->shape(),
this->permutation(),
- &tensor_strides));
+ ARROW_CHECK_OK(
+ ComputeStrides(*value_type, this->shape(), this->permutation(),
&tensor_strides));
strides_ = tensor_strides;
}
return strides_;
diff --git a/cpp/src/arrow/extension/fixed_shape_tensor.h
b/cpp/src/arrow/extension/fixed_shape_tensor.h
index 3fec79b5c2..20ec20a64c 100644
--- a/cpp/src/arrow/extension/fixed_shape_tensor.h
+++ b/cpp/src/arrow/extension/fixed_shape_tensor.h
@@ -67,10 +67,10 @@ class ARROW_EXPORT FixedShapeTensorType : public
ExtensionType {
size_t ndim() const { return shape_.size(); }
/// Shape of tensor elements
- const std::vector<int64_t> shape() const { return shape_; }
+ const std::vector<int64_t>& shape() const { return shape_; }
/// Value type of tensor elements
- const std::shared_ptr<DataType> value_type() const { return value_type_; }
+ const std::shared_ptr<DataType>& value_type() const { return value_type_; }
/// Strides of tensor elements. Strides state offset in bytes between
adjacent
/// elements along each dimension. In case permutation is non-empty strides
are
