crepererum commented on code in PR #4691:
URL: https://github.com/apache/arrow-rs/pull/4691#discussion_r1293170672
##########
arrow-array/src/array/mod.rs:
##########
@@ -173,12 +173,33 @@ pub trait Array: std::fmt::Debug + Send + Sync {
/// ```
fn offset(&self) -> usize;
- /// Returns the null buffers of this array if any
+ /// Returns the null buffer of this array if any
+ ///
+ /// Note: some arrays can encode their nullability in their children, for
example,
+ /// [`DictionaryArray::values`] values or [`RunArray::values`], or without
a null buffer,
+ /// such as [`NullArray`]. Use [`Array::logical_nulls`] to obtain a
computed mask encoding this
fn nulls(&self) -> Option<&NullBuffer>;
+ /// Returns the logical null buffer of this array if any
+ ///
+ /// In most cases this will be the same as [`Array::nulls`], except for:
Review Comment:
I think `Array::nulls` should be deprecated and a non-deprecated version
`physical_nulls` should be introduced. Users will misuse `nulls` because:
- most people will read over the small note within the doc string
- `nulls` is easier to find than `logical_nulls`
- there are two options of "which null", and we offer an implicit default
(because it is not prefixed with a semantic) that is clearly the wrong one for
many use cases
--
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]
