-
Notifications
You must be signed in to change notification settings - Fork 35
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
Add recode() function #67
Conversation
ea87d27
to
48528f9
Compare
# and using a wider type than necessary would be annoying | ||
T = default === nothing ? V : promote_type(typeof(default), V) | ||
# TODO: result should not be a nullable array when one of the pairs' LHS is null | ||
if T <: Nullable || eltype(a) <: Nullable |
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.
I'm not sure this will matter if it will be changed for Union{T, Null}
, but to check for nullability here could you use the method
typeintersect(T, Nullable) !== Union{}
you mentioned here?
Same for the next function.
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.
Yeah, that's a bit problematic. In theory we should use typeintersect
, but in practice it doesn't sound very useful to return a NullableArray
when T == Any
.
The TODO refers to the fact that with Union{T, Null}
, we will be able to know whether one of the pairs' LHS is null
at compile time just from its type, which isn't possible with Nullable
. This will allow returning a non-nullable array when we know in advance all missing values will be replaced with a non-null value.
This looks great! |
60c44d2
to
3f331e1
Compare
Modeled after dplyr's and Stata's eponymous functions.
Codecov Report
@@ Coverage Diff @@
## master #67 +/- ##
==========================================
- Coverage 92.24% 92.06% -0.19%
==========================================
Files 9 10 +1
Lines 529 693 +164
==========================================
+ Hits 488 638 +150
- Misses 41 55 +14
Continue to review full report at Codecov.
|
Modeled after dplyr's and Stata's eponymous functions.
The generic
AbstractArray
method could live in a more basic package, but I'm not sure which one.