-
Notifications
You must be signed in to change notification settings - Fork 819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate Array::data (#3880) #4019
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -95,8 +95,7 @@ pub trait Array: std::fmt::Debug + Send + Sync { | |
fn as_any(&self) -> &dyn Any; | ||
|
||
/// Returns a reference to the underlying data of this array | ||
/// | ||
/// 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")] | ||
fn data(&self) -> &ArrayData; | ||
|
||
/// Returns the underlying data of this array | ||
|
@@ -108,9 +107,8 @@ pub trait Array: std::fmt::Debug + Send + Sync { | |
fn into_data(self) -> ArrayData; | ||
|
||
/// Returns a reference-counted pointer to the underlying data of this array. | ||
/// | ||
/// 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Part of #3880 will be to port these to not use |
||
fn data_ref(&self) -> &ArrayData { | ||
self.data() | ||
} | ||
|
@@ -281,6 +279,7 @@ impl Array for ArrayRef { | |
self.as_ref().as_any() | ||
} | ||
|
||
#[allow(deprecated)] | ||
fn data(&self) -> &ArrayData { | ||
self.as_ref().data() | ||
} | ||
|
@@ -348,6 +347,7 @@ impl<'a, T: Array> Array for &'a T { | |
T::as_any(self) | ||
} | ||
|
||
#[allow(deprecated)] | ||
fn data(&self) -> &ArrayData { | ||
T::data(self) | ||
} | ||
|
@@ -435,78 +435,91 @@ pub trait ArrayAccessor: Array { | |
} | ||
|
||
impl PartialEq for dyn Array + '_ { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &Self) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl<T: Array> PartialEq<T> for dyn Array + '_ { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &T) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl PartialEq for NullArray { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &NullArray) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl<T: ArrowPrimitiveType> PartialEq for PrimitiveArray<T> { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &PrimitiveArray<T>) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl<K: ArrowDictionaryKeyType> PartialEq for DictionaryArray<K> { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &Self) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl PartialEq for BooleanArray { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &BooleanArray) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl<OffsetSize: OffsetSizeTrait> PartialEq for GenericStringArray<OffsetSize> { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &Self) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl<OffsetSize: OffsetSizeTrait> PartialEq for GenericBinaryArray<OffsetSize> { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &Self) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl PartialEq for FixedSizeBinaryArray { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &Self) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl<OffsetSize: OffsetSizeTrait> PartialEq for GenericListArray<OffsetSize> { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &Self) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl PartialEq for MapArray { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &Self) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl PartialEq for FixedSizeListArray { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &Self) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
} | ||
|
||
impl PartialEq for StructArray { | ||
#[allow(deprecated)] | ||
fn eq(&self, other: &Self) -> bool { | ||
self.data().eq(other.data()) | ||
} | ||
|
@@ -865,8 +878,8 @@ mod tests { | |
let null_array = new_null_array(array.data_type(), 9); | ||
assert_eq!(&array, &null_array); | ||
assert_eq!( | ||
array.data().buffers()[0].len(), | ||
null_array.data().buffers()[0].len() | ||
array.to_data().buffers()[0].len(), | ||
null_array.to_data().buffers()[0].len() | ||
); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2217,9 +2217,9 @@ fn value_to_string<O: OffsetSizeTrait>( | |
let mut builder = GenericStringBuilder::<O>::new(); | ||
let options = FormatOptions::default(); | ||
let formatter = ArrayFormatter::try_new(array, &options)?; | ||
let data = array.data(); | ||
for i in 0..data.len() { | ||
match data.is_null(i) { | ||
let nulls = array.nulls(); | ||
for i in 0..array.len() { | ||
match nulls.map(|x| x.is_null(i)).unwrap_or_default() { | ||
true => builder.append_null(), | ||
false => { | ||
formatter.value(i).write(&mut builder)?; | ||
|
@@ -3500,7 +3500,7 @@ where | |
FROM::Offset: OffsetSizeTrait + ToPrimitive, | ||
TO::Offset: OffsetSizeTrait + NumCast, | ||
{ | ||
let data = array.data(); | ||
let data = array.to_data(); | ||
assert_eq!(data.data_type(), &FROM::DATA_TYPE); | ||
let str_values_buf = data.buffers()[1].clone(); | ||
let offsets = data.buffers()[0].typed_data::<FROM::Offset>(); | ||
|
@@ -4844,9 +4844,8 @@ mod tests { | |
|
||
#[test] | ||
fn test_cast_list_i32_to_list_u16() { | ||
let value_data = Int32Array::from(vec![0, 0, 0, -1, -2, -1, 2, 100000000]) | ||
.data() | ||
.clone(); | ||
let value_data = | ||
Int32Array::from(vec![0, 0, 0, -1, -2, -1, 2, 100000000]).into_data(); | ||
|
||
let value_offsets = Buffer::from_slice_ref([0, 3, 6, 8]); | ||
|
||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the equality comparison still covered? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. List arrays only contain a single buffer, and so |
||
list_array.data().buffers().to_vec(), | ||
cast_array.data().buffers().to_vec() | ||
); | ||
let array = cast_array | ||
.as_ref() | ||
.as_any() | ||
.downcast_ref::<ListArray>() | ||
.unwrap(); | ||
let array = cast_array.as_list::<i32>(); | ||
assert_eq!(list_array.value_offsets(), array.value_offsets()); | ||
|
||
assert_eq!(DataType::UInt16, array.value_type()); | ||
assert_eq!(3, array.value_length(0)); | ||
assert_eq!(3, array.value_length(1)); | ||
|
@@ -4908,9 +4901,8 @@ mod tests { | |
)] | ||
fn test_cast_list_i32_to_list_timestamp() { | ||
// Construct a value array | ||
let value_data = Int32Array::from(vec![0, 0, 0, -1, -2, -1, 2, 8, 100000000]) | ||
.data() | ||
.clone(); | ||
let value_data = | ||
Int32Array::from(vec![0, 0, 0, -1, -2, -1, 2, 8, 100000000]).into_data(); | ||
|
||
let value_offsets = Buffer::from_slice_ref([0, 3, 6, 9]); | ||
|
||
|
@@ -7355,11 +7347,7 @@ mod tests { | |
fn test_list_to_string() { | ||
let str_array = StringArray::from(vec!["a", "b", "c", "d", "e", "f", "g", "h"]); | ||
let value_offsets = Buffer::from_slice_ref([0, 3, 6, 8]); | ||
let value_data = ArrayData::builder(DataType::Utf8) | ||
.len(str_array.len()) | ||
.buffers(str_array.data().buffers().to_vec()) | ||
.build() | ||
.unwrap(); | ||
let value_data = str_array.into_data(); | ||
|
||
let list_data_type = | ||
DataType::List(Arc::new(Field::new("item", DataType::Utf8, true))); | ||
|
@@ -8123,7 +8111,7 @@ mod tests { | |
let options = CastOptions { safe: true }; | ||
let array = cast_with_options(&s, &DataType::Utf8, &options).unwrap(); | ||
let a = array.as_string::<i32>(); | ||
a.data().validate_full().unwrap(); | ||
a.to_data().validate_full().unwrap(); | ||
|
||
assert_eq!(a.null_count(), 1); | ||
assert_eq!(a.len(), 2); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is no longer needed because the ArrayData builder will check the buffer count (is that correct)?