-
Notifications
You must be signed in to change notification settings - Fork 875
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
Improve docs for NullArray, new_null_array and new_empty_array #240
Conversation
@@ -42,12 +25,33 @@ use crate::array::{Array, ArrayData}; | |||
use crate::datatypes::*; | |||
|
|||
/// An Array where all elements are nulls | |||
/// |
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.
90dfc74
to
cbb9011
Compare
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.
Looks great! 👍 The note/link to arrow::array::new_null_array
is especially handy; I have a poor man's version of this in a few codebases after I looked up NullArray, saw it was an array of the null type, and thought "ok, guess that's not supported".
Codecov Report
@@ Coverage Diff @@
## master #240 +/- ##
=======================================
Coverage 82.52% 82.52%
=======================================
Files 162 162
Lines 43672 43672
=======================================
Hits 36040 36040
Misses 7632 7632
Continue to review full report at Codecov.
|
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.
LGTM
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.
Looks great. 👍
Rationale for this change
I have often found myself being confused and searching for this code, so I am hoping to leave some hints to find it. Here is an example where it may have helped: apache/datafusion#223 (though to be honest
new_null_array
may be newer than the code in datafusionWhat changes are included in this PR?
NullArray
pointing atnew_null_array
and move comments toNullArray
rather than modulenew_null_array
andnew_empty_array
Are there any user-facing changes?
Better docs