Repository: jorgecarleitao/arrow2
Pull request #1112 (2022-06-27 21:01:11)

Removed unused `NativeType::to_ne_bytes`
This is an old function that is not really used nor needed (since `ne` does not take endianess into account).

Also added a bunch of tests covering from and to bytes of custom types.



Patch:
diff --git a/src/array/fixed_size_list/mod.rs b/src/array/fixed_size_list/mod.rs
index d50518190f..d61a64c258 100644
--- a/src/array/fixed_size_list/mod.rs
+++ b/src/array/fixed_size_list/mod.rs
@@ -84,6 +84,11 @@ impl FixedSizeListArray {
         Self::try_new(data_type, values, validity).unwrap()
     }
 
+    /// Returns the size (number of elements per slot) of this [`FixedSizeListArray`].
+    pub const fn size(&self) -> usize {
+        self.size
+    }
+
     /// Alias for `new`
     pub fn from_data(
         data_type: DataType,
@@ -101,10 +106,9 @@ impl FixedSizeListArray {
 
     /// Returns a new null [`FixedSizeListArray`].
     pub fn new_null(data_type: DataType, length: usize) -> Self {
-        let values = new_null_array(
-            Self::get_child_and_size(&data_type).0.data_type().clone(),
-            length,
-        );
+        let (field, size) = Self::get_child_and_size(&data_type);
+
+        let values = new_null_array(field.data_type().clone(), length * size);
         Self::new(data_type, values, Some(Bitmap::new_zeroed(length)))
     }
 
diff --git a/src/array/utf8/mod.rs b/src/array/utf8/mod.rs
index b6b56e14cc..69c5af6e8f 100644
--- a/src/array/utf8/mod.rs
+++ b/src/array/utf8/mod.rs
@@ -486,8 +486,7 @@ impl<O: Offset> Utf8Array<O> {
         P: AsRef<str>,
         I: TrustedLen<Item = Option<P>>,
     {
-        // soundness: I is `TrustedLen`
-        unsafe { Self::from_trusted_len_iter_unchecked(iterator) }
+        MutableUtf8Array::<O>::from_trusted_len_iter(iterator).into()
     }
 
     /// Creates a [`Utf8Array`] from an falible iterator of trusted length.
@@ -512,8 +511,7 @@ impl<O: Offset> Utf8Array<O> {
         P: AsRef<str>,
         I: TrustedLen<Item = std::result::Result<Option<P>, E>>,
     {
-        // soundness: I: TrustedLen
-        unsafe { Self::try_from_trusted_len_iter_unchecked(iter) }
+        MutableUtf8Array::<O>::try_from_trusted_len_iter(iter).map(|x| x.into())
     }
 
     /// Alias for `new`
diff --git a/src/io/parquet/read/deserialize/simple.rs b/src/io/parquet/read/deserialize/simple.rs
index aaab8aee9f..20dba38065 100644
--- a/src/io/parquet/read/deserialize/simple.rs
+++ b/src/io/parquet/read/deserialize/simple.rs
@@ -467,6 +467,12 @@ fn dict_read<'a, K: DictionaryKey, I: 'a + DataPages>(
             chunk_size,
             |x: i32| x as u32,
         )),
+        UInt64 => dyn_iter(primitive::DictIter::<K, _, _, _, _>::new(
+            iter,
+            data_type,
+            chunk_size,
+            |x: i64| x as u64,
+        )),
         Int8 => dyn_iter(primitive::DictIter::<K, _, _, _, _>::new(
             iter,
             data_type,
diff --git a/src/io/parquet/write/dictionary.rs b/src/io/parquet/write/dictionary.rs
index 65f13a4904..05c56207bf 100644
--- a/src/io/parquet/write/dictionary.rs
+++ b/src/io/parquet/write/dictionary.rs
@@ -149,7 +149,7 @@ pub fn array_to_pages<K: DictionaryKey>(
                 DataType::UInt8 => dyn_prim!(u8, i32, array, options, type_),
                 DataType::UInt16 => dyn_prim!(u16, i32, array, options, type_),
                 DataType::UInt32 => dyn_prim!(u32, i32, array, options, type_),
-                DataType::UInt64 => dyn_prim!(i64, i64, array, options, type_),
+                DataType::UInt64 => dyn_prim!(u64, i64, array, options, type_),
                 DataType::Float32 => dyn_prim!(f32, f32, array, options, type_),
                 DataType::Float64 => dyn_prim!(f64, f64, array, options, type_),
                 DataType::Utf8 => {
diff --git a/src/types/native.rs b/src/types/native.rs
index fbc8ac408b..4d4f34eb60 100644
--- a/src/types/native.rs
+++ b/src/types/native.rs
@@ -33,9 +33,6 @@ pub trait NativeType:
     /// To bytes in little endian
     fn to_le_bytes(&self) -> Self::Bytes;
 
-    /// To bytes in native endian
-    fn to_ne_bytes(&self) -> Self::Bytes;
-
     /// To bytes in big endian
     fn to_be_bytes(&self) -> Self::Bytes;
 
@@ -59,11 +56,6 @@ macro_rules! native_type {
                 Self::to_be_bytes(*self)
             }
 
-            #[inline]
-            fn to_ne_bytes(&self) -> Self::Bytes {
-                Self::to_ne_bytes(*self)
-            }
-
             #[inline]
             fn from_be_bytes(bytes: Self::Bytes) -> Self {
                 Self::from_be_bytes(bytes)
@@ -88,25 +80,25 @@ native_type!(i128, PrimitiveType::Int128);
 #[derive(Debug, Copy, Clone, Default, PartialEq, Eq, Hash, Zeroable, Pod)]
 #[allow(non_camel_case_types)]
 #[repr(C)]
-pub struct days_ms([i32; 2]);
+pub struct days_ms(pub i32, pub i32);
 
 impl days_ms {
     /// A new [`days_ms`].
     #[inline]
     pub fn new(days: i32, milliseconds: i32) -> Self {
-        Self([days, milliseconds])
+        Self(days, milliseconds)
     }
 
     /// The number of days
     #[inline]
     pub fn days(&self) -> i32 {
-        self.0[0]
+        self.0
     }
 
     /// The number of milliseconds
     #[inline]
     pub fn milliseconds(&self) -> i32 {
-        self.0[1]
+        self.1
     }
 }
 
@@ -115,24 +107,8 @@ impl NativeType for days_ms {
     type Bytes = [u8; 8];
     #[inline]
     fn to_le_bytes(&self) -> Self::Bytes {
-        let days = self.0[0].to_le_bytes();
-        let ms = self.0[1].to_le_bytes();
-        let mut result = [0; 8];
-        result[0] = days[0];
-        result[1] = days[1];
-        result[2] = days[2];
-        result[3] = days[3];
-        result[4] = ms[0];
-        result[5] = ms[1];
-        result[6] = ms[2];
-        result[7] = ms[3];
-        result
-    }
-
-    #[inline]
-    fn to_ne_bytes(&self) -> Self::Bytes {
-        let days = self.0[0].to_ne_bytes();
-        let ms = self.0[1].to_ne_bytes();
+        let days = self.0.to_le_bytes();
+        let ms = self.1.to_le_bytes();
         let mut result = [0; 8];
         result[0] = days[0];
         result[1] = days[1];
@@ -147,8 +123,8 @@ impl NativeType for days_ms {
 
     #[inline]
     fn to_be_bytes(&self) -> Self::Bytes {
-        let days = self.0[0].to_be_bytes();
-        let ms = self.0[1].to_be_bytes();
+        let days = self.0.to_be_bytes();
+        let ms = self.1.to_be_bytes();
         let mut result = [0; 8];
         result[0] = days[0];
         result[1] = days[1];
@@ -173,7 +149,7 @@ impl NativeType for days_ms {
         ms[1] = bytes[5];
         ms[2] = bytes[6];
         ms[3] = bytes[7];
-        Self([i32::from_be_bytes(days), i32::from_be_bytes(ms)])
+        Self(i32::from_be_bytes(days), i32::from_be_bytes(ms))
     }
 }
 
@@ -181,7 +157,7 @@ impl NativeType for days_ms {
 #[derive(Debug, Copy, Clone, Default, PartialEq, Eq, Hash, Zeroable, Pod)]
 #[allow(non_camel_case_types)]
 #[repr(C)]
-pub struct months_days_ns(i32, i32, i64);
+pub struct months_days_ns(pub i32, pub i32, pub i64);
 
 impl months_days_ns {
     /// A new [`months_days_ns`].
@@ -232,26 +208,6 @@ impl NativeType for months_days_ns {
         result
     }
 
-    #[inline]
-    fn to_ne_bytes(&self) -> Self::Bytes {
-        let months = self.months().to_ne_bytes();
-        let days = self.days().to_ne_bytes();
-        let ns = self.ns().to_ne_bytes();
-        let mut result = [0; 16];
-        result[0] = months[0];
-        result[1] = months[1];
-        result[2] = months[2];
-        result[3] = months[3];
-        result[4] = days[0];
-        result[5] = days[1];
-        result[6] = days[2];
-        result[7] = days[3];
-        (0..8).for_each(|i| {
-            result[8 + i] = ns[i];
-        });
-        result
-    }
-
     #[inline]
     fn to_be_bytes(&self) -> Self::Bytes {
         let months = self.months().to_be_bytes();
@@ -420,11 +376,6 @@ impl NativeType for f16 {
         self.0.to_le_bytes()
     }
 
-    #[inline]
-    fn to_ne_bytes(&self) -> Self::Bytes {
-        self.0.to_ne_bytes()
-    }
-
     #[inline]
     fn to_be_bytes(&self) -> Self::Bytes {
         self.0.to_be_bytes()


Test patch:
diff --git a/tests/it/array/fixed_size_list/mod.rs b/tests/it/array/fixed_size_list/mod.rs
index f7a9b61988..8281aee519 100644
--- a/tests/it/array/fixed_size_list/mod.rs
+++ b/tests/it/array/fixed_size_list/mod.rs
@@ -1,1 +1,104 @@
 mod mutable;
+
+use arrow2::{
+    array::*,
+    bitmap::Bitmap,
+    datatypes::{DataType, Field},
+};
+
+fn data() -> FixedSizeListArray {
+    let values = Int32Array::from_slice([10, 20, 0, 0]);
+
+    FixedSizeListArray::try_new(
+        DataType::FixedSizeList(
+            Box::new(Field::new("a", values.data_type().clone(), true)),
+            2,
+        ),
+        values.boxed(),
+        Some([true, false].into()),
+    )
+    .unwrap()
+}
+
+#[test]
+fn basics() {
+    let array = data();
+    assert_eq!(array.size(), 2);
+    assert_eq!(array.len(), 2);
+    assert_eq!(array.validity(), Some(&Bitmap::from([true, false])));
+
+    assert_eq!(array.value(0).as_ref(), Int32Array::from_slice([10, 20]));
+    assert_eq!(array.value(1).as_ref(), Int32Array::from_slice([0, 0]));
+
+    let array = array.slice(1, 1);
+
+    assert_eq!(array.value(0).as_ref(), Int32Array::from_slice([0, 0]));
+}
+
+#[test]
+fn with_validity() {
+    let array = data();
+
+    let a = array.with_validity(None);
+    assert!(a.validity().is_none());
+}
+
+#[test]
+fn debug() {
+    let array = data();
+
+    assert_eq!(format!("{:?}", array), "FixedSizeListArray[[10, 20], None]");
+}
+
+#[test]
+fn empty() {
+    let array = FixedSizeListArray::new_empty(DataType::FixedSizeList(
+        Box::new(Field::new("a", DataType::Int32, true)),
+        2,
+    ));
+    assert_eq!(array.values().len(), 0);
+    assert_eq!(array.validity(), None);
+}
+
+#[test]
+fn null() {
+    let array = FixedSizeListArray::new_null(
+        DataType::FixedSizeList(Box::new(Field::new("a", DataType::Int32, true)), 2),
+        2,
+    );
+    assert_eq!(array.values().len(), 4);
+    assert_eq!(array.validity().cloned(), Some([false, false].into()));
+}
+
+#[test]
+fn wrong_size() {
+    let values = Int32Array::from_slice([10, 20, 0]);
+    assert!(FixedSizeListArray::try_new(
+        DataType::FixedSizeList(Box::new(Field::new("a", DataType::Int32, true)), 2),
+        values.boxed(),
+        None
+    )
+    .is_err());
+}
+
+#[test]
+fn wrong_len() {
+    let values = Int32Array::from_slice([10, 20, 0]);
+    assert!(FixedSizeListArray::try_new(
+        DataType::FixedSizeList(Box::new(Field::new("a", DataType::Int32, true)), 2),
+        values.boxed(),
+        Some([true, false, false].into()), // it should be 2
+    )
+    .is_err());
+}
+
+#[test]
+fn wrong_data_type() {
+    let values = Int32Array::from_slice([10, 20, 0]);
+    assert!(FixedSizeListArray::try_new(
+        DataType::Binary,
+        values.boxed(),
+        Some([true, false, false].into()), // it should be 2
+    )
+    .is_err());
+}
diff --git a/tests/it/array/mod.rs b/tests/it/array/mod.rs
index ef14b993d2..8b400901d7 100644
--- a/tests/it/array/mod.rs
+++ b/tests/it/array/mod.rs
@@ -99,8 +99,8 @@ fn test_with_validity() {
     assert_eq!(arr_ref, &expected);
 }
 
-// check that `PartialEq` can be derived
-#[derive(PartialEq)]
+// check that we ca derive stuff
+#[derive(PartialEq, Clone, Debug)]
 struct A {
     array: Box<dyn Array>,
 }
diff --git a/tests/it/array/primitive/mod.rs b/tests/it/array/primitive/mod.rs
index 058d503f47..5a618cba25 100644
--- a/tests/it/array/primitive/mod.rs
+++ b/tests/it/array/primitive/mod.rs
@@ -72,7 +72,7 @@ fn from() {
 }
 
 #[test]
-fn months_days_ns() {
+fn months_days_ns_from_slice() {
     let data = &[
         months_days_ns::new(1, 1, 2),
         months_days_ns::new(1, 1, 3),
diff --git a/tests/it/array/utf8/mod.rs b/tests/it/array/utf8/mod.rs
index fc55ebccde..c6a0acec03 100644
--- a/tests/it/array/utf8/mod.rs
+++ b/tests/it/array/utf8/mod.rs
@@ -215,3 +215,21 @@ fn into_mut_4() {
     let array = Utf8Array::<i32>::new(DataType::Utf8, offsets, values, validity);
     assert!(array.into_mut().is_right());
 }
+
+#[test]
+fn rev_iter() {
+    let array = Utf8Array::<i32>::from(&[Some("hello"), Some(" "), None]);
+
+    assert_eq!(
+        array.into_iter().rev().collect::<Vec<_>>(),
+        vec![None, Some(" "), Some("hello")]
+    );
+}
+
+#[test]
+fn iter_nth() {
+    let array = Utf8Array::<i32>::from(&[Some("hello"), Some(" "), None]);
+
+    assert_eq!(array.iter().nth(1), Some(Some(" ")));
+    assert_eq!(array.iter().nth(10), None);
+}
diff --git a/tests/it/array/utf8/mutable.rs b/tests/it/array/utf8/mutable.rs
index 8ee76476cb..80cc24ca3c 100644
--- a/tests/it/array/utf8/mutable.rs
+++ b/tests/it/array/utf8/mutable.rs
@@ -141,3 +141,29 @@ fn test_extend_values() {
     assert_eq!(array.offsets().as_slice(), &[0, 2, 7, 12, 17]);
     assert_eq!(array.validity(), None,);
 }
+
+#[test]
+fn test_extend() {
+    let mut array = MutableUtf8Array::<i32>::new();
+
+    array.extend([Some("hi"), None, Some("there"), None].into_iter());
+
+    let array: Utf8Array<i32> = array.into();
+
+    assert_eq!(
+        array,
+        Utf8Array::<i32>::from([Some("hi"), None, Some("there"), None])
+    );
+}
+
+#[test]
+fn as_arc() {
+    let mut array = MutableUtf8Array::<i32>::new();
+
+    array.extend([Some("hi"), None, Some("there"), None].into_iter());
+
+    assert_eq!(
+        Utf8Array::<i32>::from([Some("hi"), None, Some("there"), None]),
+        array.as_arc().as_ref()
+    );
+}
diff --git a/tests/it/compute/arithmetics/time.rs b/tests/it/compute/arithmetics/time.rs
index cbe53381be..9f31dd60a9 100644
--- a/tests/it/compute/arithmetics/time.rs
+++ b/tests/it/compute/arithmetics/time.rs
@@ -2,6 +2,7 @@ use arrow2::array::*;
 use arrow2::compute::arithmetics::time::*;
 use arrow2::datatypes::{DataType, TimeUnit};
 use arrow2::scalar::*;
+use arrow2::types::months_days_ns;
 
 #[test]
 fn test_adding_timestamp() {
@@ -317,3 +318,68 @@ fn test_date64() {
 
     assert_eq!(result, expected);
 }
+
+#[test]
+fn test_add_interval() {
+    let timestamp =
+        PrimitiveArray::from_slice([1i64]).to(DataType::Timestamp(TimeUnit::Second, None));
+
+    let interval = months_days_ns::new(0, 1, 0);
+
+    let intervals = PrimitiveArray::from_slice([interval]);
+
+    let expected = PrimitiveArray::from_slice([1i64 + 24 * 60 * 60])
+        .to(DataType::Timestamp(TimeUnit::Second, None));
+
+    let result = add_interval(&timestamp, &intervals).unwrap();
+    assert_eq!(result, expected);
+
+    let result = add_interval_scalar(&timestamp, &Some(interval).into()).unwrap();
+    assert_eq!(result, expected);
+}
+
+#[test]
+fn test_add_interval_offset() {
+    let timestamp = PrimitiveArray::from_slice([1i64]).to(DataType::Timestamp(
+        TimeUnit::Second,
+        Some("+01:00".to_string()),
+    ));
+
+    let interval = months_days_ns::new(0, 1, 0);
+
+    let intervals = PrimitiveArray::from_slice([interval]);
+
+    let expected = PrimitiveArray::from_slice([1i64 + 24 * 60 * 60]).to(DataType::Timestamp(
+        TimeUnit::Second,
+        Some("+01:00".to_string()),
+    ));
+
+    let result = add_interval(&timestamp, &intervals).unwrap();
+    assert_eq!(result, expected);
+
+    let result = add_interval_scalar(&timestamp, &Some(interval).into()).unwrap();
+    assert_eq!(result, expected);
+}
+
+#[cfg(feature = "chrono-tz")]
+#[test]
+fn test_add_interval_tz() {
+    let timestamp = PrimitiveArray::from_slice([1i64]).to(DataType::Timestamp(
+        TimeUnit::Second,
+        Some("GMT".to_string()),
+    ));
+
+    let interval = months_days_ns::new(0, 1, 0);
+    let intervals = PrimitiveArray::from_slice([interval]);
+
+    let expected = PrimitiveArray::from_slice([1i64 + 24 * 60 * 60]).to(DataType::Timestamp(
+        TimeUnit::Second,
+        Some("GMT".to_string()),
+    ));
+
+    let result = add_interval(&timestamp, &intervals).unwrap();
+    assert_eq!(result, expected);
+
+    let result = add_interval_scalar(&timestamp, &Some(interval).into()).unwrap();
+    assert_eq!(result, expected);
+}
diff --git a/tests/it/io/parquet/mod.rs b/tests/it/io/parquet/mod.rs
index 955df23e66..3c8b7bafd6 100644
--- a/tests/it/io/parquet/mod.rs
+++ b/tests/it/io/parquet/mod.rs
@@ -962,8 +962,8 @@ fn integration_read(data: &[u8]) -> Result<IntegrationRead> {
 /// logical types.
 #[test]
 fn arrow_type() -> Result<()> {
-    let dt1 = DataType::Duration(TimeUnit::Second);
-    let array = PrimitiveArray::<i64>::from([Some(1), None, Some(2)]).to(dt1.clone());
+    let array1 = PrimitiveArray::<i64>::from([Some(1), None, Some(2)])
+        .to(DataType::Duration(TimeUnit::Second));
     let array2 = Utf8Array::<i64>::from([Some("a"), None, Some("bb")]);
 
     let indices = PrimitiveArray::from_values((0..3u64).map(|x| x % 2));
@@ -978,23 +978,61 @@ fn arrow_type() -> Result<()> {
         vec![b'a', b'b', b'a', b'c'].into(),
         None,
     );
-    let array5 = DictionaryArray::from_data(indices, Box::new(values));
+    let array5 = DictionaryArray::from_data(indices.clone(), Box::new(values));
+
+    let values = PrimitiveArray::from_slice([1i16, 3]);
+    let array6 = DictionaryArray::from_data(indices.clone(), Box::new(values));
+
+    let values = PrimitiveArray::from_slice([1i64, 3]).to(DataType::Timestamp(
+        TimeUnit::Millisecond,
+        Some("UTC".to_string()),
+    ));
+    let array7 = DictionaryArray::from_data(indices.clone(), Box::new(values));
+
+    let values = PrimitiveArray::from_slice([1.0f64, 3.0]);
+    let array8 = DictionaryArray::from_data(indices.clone(), Box::new(values));
+
+    let values = PrimitiveArray::from_slice([1u8, 3]);
+    let array9 = DictionaryArray::from_data(indices.clone(), Box::new(values));
+
+    let values = PrimitiveArray::from_slice([1u16, 3]);
+    let array10 = DictionaryArray::from_data(indices.clone(), Box::new(values));
+
+    let values = PrimitiveArray::from_slice([1u32, 3]);
+    let array11 = DictionaryArray::from_data(indices.clone(), Box::new(values));
+
+    let values = PrimitiveArray::from_slice([1u64, 3]);
+    let array12 = DictionaryArray::from_data(indices, Box::new(values));
 
     let schema = Schema::from(vec![
-        Field::new("a1", dt1, true),
+        Field::new("a1", array1.data_type().clone(), true),
         Field::new("a2", array2.data_type().clone(), true),
         Field::new("a3", array3.data_type().clone(), true),
         Field::new("a4", array4.data_type().clone(), true),
         Field::new("a5", array5.data_type().clone(), true),
-        Field::new("a6", array5.data_type().clone(), false),
+        Field::new("a5a", array5.data_type().clone(), false),
+        Field::new("a6", array6.data_type().clone(), true),
+        Field::new("a7", array7.data_type().clone(), true),
+        Field::new("a8", array8.data_type().clone(), true),
+        Field::new("a9", array9.data_type().clone(), true),
+        Field::new("a10", array10.data_type().clone(), true),
+        Field::new("a11", array11.data_type().clone(), true),
+        Field::new("a12", array12.data_type().clone(), true),
     ]);
     let batch = Chunk::try_new(vec![
-        array.boxed(),
+        array1.boxed(),
         array2.boxed(),
         array3.boxed(),
         array4.boxed(),
         array5.clone().boxed(),
         array5.boxed(),
+        array6.boxed(),
+        array7.boxed(),
+        array8.boxed(),
+        array9.boxed(),
+        array10.boxed(),
+        array11.boxed(),
+        array12.boxed(),
     ])?;
 
     let r = integration_write(&schema, &[batch.clone()])?;
diff --git a/tests/it/types.rs b/tests/it/types.rs
index 7dfa07cc16..ee337e901f 100644
--- a/tests/it/types.rs
+++ b/tests/it/types.rs
@@ -1,4 +1,4 @@
-use arrow2::types::{BitChunkIter, BitChunkOnes};
+use arrow2::types::{days_ms, months_days_ns, BitChunkIter, BitChunkOnes, NativeType};
 
 #[test]
 fn test_basic1() {
@@ -18,3 +18,23 @@ fn test_ones() {
     assert_eq!(iter.next(), Some(0));
     assert_eq!(iter.next(), Some(12));
 }
+
+#[test]
+fn months_days_ns_roundtrip() {
+    let a = months_days_ns(1, 2, 3);
+    let bytes = a.to_le_bytes();
+    assert_eq!(bytes, [1, 0, 0, 0, 2, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0]);
+
+    let a = months_days_ns(1, 1, 1);
+    assert_eq!(a, months_days_ns::from_be_bytes(a.to_be_bytes()));
+}
+
+#[test]
+fn days_ms_roundtrip() {
+    let a = days_ms(1, 2);
+    let bytes = a.to_le_bytes();
+    assert_eq!(bytes, [1, 0, 0, 0, 2, 0, 0, 0]);
+
+    let a = days_ms(1, 2);
+    assert_eq!(a, days_ms::from_be_bytes(a.to_be_bytes()));
+}