Deprecate Array::data (#3880) #4019
Conversation
| @@ -111,6 +110,7 @@ pub trait Array: std::fmt::Debug + Send + Sync { | |||
| /// | |||
| /// This will be deprecated in a future release [(#3880)](https://github.com/apache/arrow-rs/issues/3880) | |||
| #[deprecated(note = "Use Array::to_data or Array::into_data")] | |||
| #[allow(deprecated)] | |||
There was a problem hiding this comment.
Part of #3880 will be to port these to not use Array::data
| @@ -193,7 +193,8 @@ fn interleave_fallback( | |||
| values: &[&dyn Array], | |||
| indices: &[(usize, usize)], | |||
| ) -> Result<ArrayRef, ArrowError> { | |||
| let arrays: Vec<_> = values.iter().map(|x| x.data()).collect(); | |||
| let arrays: Vec<_> = values.iter().map(|x| x.to_data()).collect(); | |||
| let arrays: Vec<_> = arrays.iter().collect(); | |||
There was a problem hiding this comment.
This is perhaps a little unfortunate, but given what this operation is doing, it is not going to be relevant to the benchmarks, the machinery of creating a MutableArrayData alone is likely more costly
There was a problem hiding this comment.
We could potentially change the MutableArrayData constructor to take impl IntoIterator<Item = &'ArrayData> but that seems like it just moves the copy around.
| @@ -467,9 +467,6 @@ mod tests { | |||
| let list_array = GenericListArray::<O>::from(array_data2); | |||
| let binary_array2 = GenericBinaryArray::<O>::from(list_array); | |||
|
|
|||
| assert_eq!(2, binary_array2.data().buffers().len()); | |||
There was a problem hiding this comment.
This test is no longer needed because the ArrayData builder will check the buffer count (is that correct)?
| @@ -4875,15 +4874,9 @@ mod tests { | |||
| assert_eq!(0, cast_array.null_count()); | |||
|
|
|||
| // offsets should be the same | |||
| assert_eq!( | |||
There was a problem hiding this comment.
Is the equality comparison still covered?
There was a problem hiding this comment.
List arrays only contain a single buffer, and so assert_eq!(list_array.value_offsets(), array.value_offsets()); is equivalent
| @@ -81,7 +81,8 @@ pub fn concat(arrays: &[&dyn Array]) -> Result<ArrayRef, ArrowError> { | |||
| _ => Capacities::Array(arrays.iter().map(|a| a.len()).sum()), | |||
| }; | |||
|
|
|||
| let array_data = arrays.iter().map(|a| a.data()).collect::<Vec<_>>(); | |||
| let array_data: Vec<_> = arrays.iter().map(|a| a.to_data()).collect::<Vec<_>>(); | |||
| let array_data = array_data.iter().collect(); | |||
There was a problem hiding this comment.
why do we need the second iter/collect?
Update: Because it wants a Vec of &ArrayData
| @@ -193,7 +193,8 @@ fn interleave_fallback( | |||
| values: &[&dyn Array], | |||
| indices: &[(usize, usize)], | |||
| ) -> Result<ArrayRef, ArrowError> { | |||
| let arrays: Vec<_> = values.iter().map(|x| x.data()).collect(); | |||
| let arrays: Vec<_> = values.iter().map(|x| x.to_data()).collect(); | |||
| let arrays: Vec<_> = arrays.iter().collect(); | |||
There was a problem hiding this comment.
We could potentially change the MutableArrayData constructor to take impl IntoIterator<Item = &'ArrayData> but that seems like it just moves the copy around.
arrow-array/src/array/mod.rs
Outdated
| @@ -111,6 +110,7 @@ pub trait Array: std::fmt::Debug + Send + Sync { | |||
| /// | |||
| /// This will be deprecated in a future release [(#3880)](https://github.com/apache/arrow-rs/issues/3880) | |||
There was a problem hiding this comment.
| /// This will be deprecated in a future release [(#3880)](https://github.com/apache/arrow-rs/issues/3880) | |
| /// This will be removed in a future release [(#3880)](https://github.com/apache/arrow-rs/issues/3880) |
Which issue does this PR close?
Part of #3880
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?