alamb commented on code in PR #10890:
URL: https://github.com/apache/arrow-rs/pull/10890#discussion_r3873685395
##########
arrow-array/src/array/mod.rs:
##########
@@ -1060,19 +1060,24 @@ unsafe fn get_offsets_from_buffer<O: ArrowNativeType>(
}
/// Helper function for printing potentially long arrays.
-fn print_long_array<A, F>(array: &A, f: &mut std::fmt::Formatter, print_item:
F) -> std::fmt::Result
-where
- A: Array,
- F: Fn(&A, usize, &mut std::fmt::Formatter) -> std::fmt::Result,
-{
+///
+/// Note this function is deliberately not generic over the array or closure
Review Comment:
THis is a pretty good example of one pattern : use &dyn rather than make an
entire new copy
##########
arrow-array/src/array/primitive_array.rs:
##########
@@ -1360,73 +1361,97 @@ where
}
}
-impl<T: ArrowPrimitiveType> std::fmt::Debug for PrimitiveArray<T> {
- fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
- let data_type = self.data_type();
-
- write!(f, "PrimitiveArray<{data_type}>\n[\n")?;
- print_long_array(self, f, |array, index, f| match data_type {
- DataType::Date32 | DataType::Date64 => {
- let v = self.value(index).to_i64().unwrap();
- match as_date::<T>(v) {
Review Comment:
you cna see in the old code there is this strange intermixinig of `match` on
`data_type` but also the function is templated on `PrimitiveType` so we get
copies of code that most branches can never be called
--
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]
