tustvold commented on code in PR #4484:
URL: https://github.com/apache/arrow-rs/pull/4484#discussion_r1257486684
##########
arrow-cast/src/cast.rs:
##########
@@ -3680,6 +3682,45 @@ fn cast_primitive_to_list<OffsetSize: OffsetSizeTrait +
NumCast>(
Ok(list_array)
}
+/// Wraps a list array with another list array, using the specified
`OffsetSize`.
+/// This function converts a dynamic array reference to a typed list array,
creates a new list array
+/// with the same elements as the original array, and wraps it with another
list array.
+fn wrap_list_array_with_another_one<OffsetSize: OffsetSizeTrait>(
+ array: &dyn Array,
+) -> Result<ArrayRef, ArrowError> {
+ let array = array.as_list::<OffsetSize>();
+ let array_data_type = array.data_type();
+ let field = Arc::new(Field::new("item", array_data_type.clone(), true));
Review Comment:
I think this need to match the field that we are casting to, i.e. propagate
`to: &Field`
##########
arrow-cast/src/cast.rs:
##########
@@ -8083,6 +8125,88 @@ mod tests {
assert_eq!(strs, &["b", "c"])
}
+ #[test]
+ fn test_list_casting() -> Result<(), ArrowError> {
+ let data = vec![Some(vec![Some(1), Some(2), Some(3)])];
+ let list_array = ListArray::from_iter_primitive::<Int32Type, _,
_>(data);
+
+ // 2d list
+ let to_2d_list = DataType::List(Arc::new(Field::new(
+ "item",
+ DataType::List(Arc::new(Field::new("item", DataType::Int32,
true))),
+ true,
+ )));
+
+ // 3d list
+ let to_3d_list =
+ DataType::List(Arc::new(Field::new("item", to_2d_list.clone(),
true)));
+
+ // Verify 1d to 2d
+ let casted_list_array = cast(&list_array, &to_2d_list).unwrap();
+ let expected_list_array =
wrap_list_array_with_another_one::<i32>(&list_array)?;
+ assert_eq!(&expected_list_array, &casted_list_array);
+
+ // Verify 1d to 3d
+ let casted_list_array = cast(&list_array, &to_3d_list).unwrap();
+ let list_array_2d =
wrap_list_array_with_another_one::<i32>(&list_array)?;
Review Comment:
I'm not seeing any tests that wrap_list_array_with_another_one is correct
##########
arrow-cast/src/cast.rs:
##########
@@ -8083,6 +8125,88 @@ mod tests {
assert_eq!(strs, &["b", "c"])
}
+ #[test]
+ fn test_list_casting() -> Result<(), ArrowError> {
+ let data = vec![Some(vec![Some(1), Some(2), Some(3)])];
+ let list_array = ListArray::from_iter_primitive::<Int32Type, _,
_>(data);
+
+ // 2d list
+ let to_2d_list = DataType::List(Arc::new(Field::new(
+ "item",
+ DataType::List(Arc::new(Field::new("item", DataType::Int32,
true))),
+ true,
+ )));
+
+ // 3d list
+ let to_3d_list =
+ DataType::List(Arc::new(Field::new("item", to_2d_list.clone(),
true)));
+
+ // Verify 1d to 2d
+ let casted_list_array = cast(&list_array, &to_2d_list).unwrap();
+ let expected_list_array =
wrap_list_array_with_another_one::<i32>(&list_array)?;
+ assert_eq!(&expected_list_array, &casted_list_array);
+
+ // Verify 1d to 3d
+ let casted_list_array = cast(&list_array, &to_3d_list).unwrap();
+ let list_array_2d =
wrap_list_array_with_another_one::<i32>(&list_array)?;
+ let expected_list_array =
+ wrap_list_array_with_another_one::<i32>(&list_array_2d)?;
+ assert_eq!(&expected_list_array, &casted_list_array);
+
+ // Verify 2d to 3d
+ let data = vec![Some(vec![Some(1), Some(2), Some(3)])];
+ let list_array = ListArray::from_iter_primitive::<Int32Type, _,
_>(data);
+ let list_array_2d = ListArray::from(list_array.into_data());
+
+ let casted_list_array = cast(&list_array_2d, &to_3d_list).unwrap();
+ assert_eq!(&expected_list_array, &casted_list_array);
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_large_list_casting() -> Result<(), ArrowError> {
+ // Verify Large List
+ let data = vec![Some(vec![Some(1), Some(2), Some(3)])];
+ let list_array = LargeListArray::from_iter_primitive::<Int32Type, _,
_>(data);
+
+ // 2d list
+ let to_2d_list = DataType::LargeList(Arc::new(Field::new(
Review Comment:
You could use `Field::new_list` to make this more concise
##########
arrow-schema/src/datatype.rs:
##########
@@ -407,6 +407,19 @@ impl DataType {
}
}
+ /// Returns the number of dimensions if the data type is nested (List,
FixedSizeList, LargeList).
+ /// Otherwise, returns 0.
+ pub fn get_list_ndims(&self) -> u8 {
Review Comment:
Where is this used? What is it for?
##########
arrow-cast/src/cast.rs:
##########
@@ -3680,6 +3682,45 @@ fn cast_primitive_to_list<OffsetSize: OffsetSizeTrait +
NumCast>(
Ok(list_array)
}
+/// Wraps a list array with another list array, using the specified
`OffsetSize`.
+/// This function converts a dynamic array reference to a typed list array,
creates a new list array
+/// with the same elements as the original array, and wraps it with another
list array.
+fn wrap_list_array_with_another_one<OffsetSize: OffsetSizeTrait>(
+ array: &dyn Array,
+) -> Result<ArrayRef, ArrowError> {
+ let array = array.as_list::<OffsetSize>();
+ let array_data_type = array.data_type();
+ let field = Arc::new(Field::new("item", array_data_type.clone(), true));
+ let array_len = array.len();
+ let offsets = OffsetBuffer::<OffsetSize>::from_lengths([array_len]);
+ let values = Arc::new(array.clone()) as ArrayRef;
+ let new_list_array =
+ GenericListArray::<OffsetSize>::new(field, offsets, values, None);
+ Ok(Arc::new(new_list_array))
+}
+
+/// Helper function that casts an Generic list container
+fn cast_list<OffsetSize: OffsetSizeTrait>(
+ array: &dyn Array,
+ to: &Field,
+ to_type: &DataType,
+ cast_options: &CastOptions,
+) -> Result<ArrayRef, ArrowError> {
+ match to.data_type() {
Review Comment:
I'm confused by this logic, it looks like it will always wrap an array with
another list, even if one already exists? I would expect it to be at least
checking `array.datatype()` to see if nesting is needed
--
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]
