[GitHub] [arrow-rs] alamb commented on a change in pull request #1223: `DecimalArray` API ergonomics: add iter(), create from iter(), change precision / scale

[GitBox <gi...@apache.org>]

alamb commented on a change in pull request #1223:
URL: https://github.com/apache/arrow-rs/pull/1223#discussion_r790269438



##########
File path: arrow/src/array/array_binary.rs
##########
@@ -816,13 +827,80 @@ impl DecimalArray {
         let array_data = unsafe { builder.build_unchecked() };
         Self::from(array_data)
     }
+
+    /// Creates a [DecimalArray] with default precision and scale,
+    /// based on an iterator of `i128` values without nulls
+    pub fn from_iter_values<I: IntoIterator<Item = i128>>(iter: I) -> Self {
+        let val_buf: Buffer = iter.into_iter().collect();
+        let data = unsafe {
+            ArrayData::new_unchecked(
+                Self::default_type(),
+                val_buf.len() / std::mem::size_of::<i128>(),
+                None,
+                None,
+                0,
+                vec![val_buf],
+                vec![],
+            )
+        };
+        DecimalArray::from(data)
+    }
+
+    /// Return the precision (total digits) that can be stored by this array
     pub fn precision(&self) -> usize {
         self.precision
     }
 
+    /// Return the scale (digits after the decimal) that can be stored by this array
     pub fn scale(&self) -> usize {
         self.scale
     }
+
+    /// Returns a DecimalArray with the same data as self, with the
+    /// specified precision.
+    ///
+    /// panic's if:
+    /// 1. the precision is larger than [`DECIMAL_MAX_PRECISION`]
+    /// 2. scale is larger than [`DECIMAL_MAX_SCALE`];
+    /// 3. scale is > precision
+    pub fn with_precision_and_scale(mut self, precision: usize, scale: usize) -> Self {

Review comment:
       I also thought about an API that returns `Result` rather than `panic` - any thoughts from reviewers?

##########
File path: arrow/src/array/array_binary.rs
##########
@@ -690,19 +693,27 @@ impl Array for FixedSizeBinaryArray {
     }
 }
 
-/// A type of `DecimalArray` whose elements are binaries.
+/// `DecimalArray` stores fixed width decimal numbers,
+/// with a fixed precision and scale.
 ///
 /// # Examples
 ///
 /// ```
 ///    use arrow::array::{Array, DecimalArray, DecimalBuilder};
 ///    use arrow::datatypes::DataType;
-///    let mut builder = DecimalBuilder::new(30, 23, 6);
 ///
-///    builder.append_value(8_887_000_000).unwrap();
-///    builder.append_null().unwrap();
-///    builder.append_value(-8_887_000_000).unwrap();
-///    let decimal_array: DecimalArray = builder.finish();
+///    // Create a DecimalArray with the default precision and scale

Review comment:
       this is an example of how the new API workrs -- I think it is easier to understand and use

##########
File path: arrow/src/array/array_binary.rs
##########
@@ -1342,6 +1511,48 @@ mod tests {
         assert_eq!("9.9", arr.value_as_string(0));
         assert_eq!("-9.9", arr.value_as_string(1));
     }
+    #[test]
+    fn test_decimal_from_iter_values() {
+        let array = DecimalArray::from_iter_values(vec![-100, 0, 101].into_iter());
+        assert_eq!(array.len(), 3);
+        assert_eq!(array.data_type(), &DataType::Decimal(38, 10));
+        assert_eq!(-100, array.value(0));
+        assert!(!array.is_null(0));
+        assert_eq!(0, array.value(1));
+        assert!(!array.is_null(1));
+        assert_eq!(101, array.value(2));
+        assert!(!array.is_null(2));
+    }
+
+    #[test]
+    fn test_decimal_from_iter() {
+        let array: DecimalArray = vec![Some(-100), None, Some(101)].into_iter().collect();
+        assert_eq!(array.len(), 3);
+        assert_eq!(array.data_type(), &DataType::Decimal(38, 10));
+        assert_eq!(-100, array.value(0));
+        assert!(!array.is_null(0));
+        assert!(array.is_null(1));
+        assert_eq!(101, array.value(2));
+        assert!(!array.is_null(2));
+    }
+
+    #[test]
+    fn test_decimal_iter() {

Review comment:
       Also included a basic `iter()` and `into_iter()` functions -- while there is room for performance improvement I think the API is solid




-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org