alamb commented on code in PR #3881:
URL: https://github.com/apache/arrow-rs/pull/3881#discussion_r1140347193
##########
arrow-arith/src/aggregate.rs:
##########
@@ -117,7 +117,7 @@ where
.map(|i| unsafe { array.value_unchecked(i) })
.reduce(|acc, item| if cmp(&acc, &item) { item } else { acc })
} else {
- let nulls = array.data().nulls().unwrap();
+ let nulls = array.nulls().unwrap();
Review Comment:
The point of this PR, as I understand it, is to remove direct use of
`data()` -- so that eventually the Arrays themselves do not need to hold
`ArrayData` but rather the strongly typed variants.
The plan is not to remove ArrayData completely, but rather construct it on
demand if needed from the appropriate parts of each Array
##########
arrow-array/src/array/mod.rs:
##########
@@ -96,12 +96,19 @@ 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)
fn data(&self) -> &ArrayData;
+ /// Returns the underlying data of this array.
+ fn to_data(&self) -> ArrayData;
+
/// Returns the underlying data of this array.
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)
Review Comment:
Should we mark it as `#[deprecated]`?
##########
arrow-array/src/array/mod.rs:
##########
@@ -96,12 +96,19 @@ 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)
fn data(&self) -> &ArrayData;
+ /// Returns the underlying data of this array.
Review Comment:
I think some docs explaining the (subtle) difference between `to_data` and
`into_data`, and what the implications of using the two APIs are would be
helpful here (like `into_data` doesn't increase the underlying ref count?
--
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]
