Add Array::to_data and Array::nulls (#3880) #3881
Conversation
| @@ -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(); | |||
There was a problem hiding this 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
Outdated
| fn data(&self) -> &ArrayData; | ||
|
|
||
| /// Returns the underlying data of this array. |
There was a problem hiding this 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?
| /// 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) |
There was a problem hiding this comment.
Should we mark it as #[deprecated]?
There was a problem hiding this comment.
I expect to do so in a future PR, trying to avoid changing all the things at once 😄
* Add Array::to_data and Array::nulls (apache#3880) * Review feedback * Format
Which issue does this PR close?
Part of #3880
Rationale for this change
In order to be able to remove
ArrayDatafrom inside the variousArrayabstractions, we need to tweak theArrayinterface slightlyWhat changes are included in this PR?
Are there any user-facing changes?
This PR doesn't make any breaking changes, yet...