mrkn commented on a change in pull request #7539:
URL: https://github.com/apache/arrow/pull/7539#discussion_r447259489
##########
File path: cpp/src/arrow/tensor/csf_converter.cc
##########
@@ -57,73 +57,86 @@ inline void IncrementIndex(std::vector<int64_t>& coord,
const std::vector<int64_
// ----------------------------------------------------------------------
// SparseTensorConverter for SparseCSFIndex
-template <typename TYPE>
-class SparseCSFTensorConverter {
- public:
- using NumericTensorType = NumericTensor<TYPE>;
- using value_type = typename NumericTensorType::value_type;
+class SparseCSFTensorConverter : private SparseTensorConverterMixin {
+ using SparseTensorConverterMixin::AssignIndex;
+ using SparseTensorConverterMixin::IsNonZero;
- SparseCSFTensorConverter(const NumericTensorType& tensor,
+ public:
+ SparseCSFTensorConverter(const Tensor& tensor,
const std::shared_ptr<DataType>& index_value_type,
MemoryPool* pool)
: tensor_(tensor), index_value_type_(index_value_type), pool_(pool) {}
- template <typename IndexValueType>
Status Convert() {
- using c_index_value_type = typename IndexValueType::c_type;
-
RETURN_NOT_OK(CheckMaximumValue(std::numeric_limits<c_index_value_type>::max()));
+ RETURN_NOT_OK(CheckSparseIndexMaximumValue(index_value_type_,
tensor_.shape()));
+
+ const int index_elsize =
+ checked_cast<const IntegerType&>(*index_value_type_).bit_width() /
CHAR_BIT;
+ const int value_elsize =
+ checked_cast<const FixedWidthType&>(*tensor_.type()).bit_width() /
CHAR_BIT;
Review comment:
I agreed with you. I felt it is better to extract this idiom as a
function, too.
`internal::GetByteWith` looks good to me, but how about the member function
of `FixedWidthType`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
