-
Notifications
You must be signed in to change notification settings - Fork 21
[WIP/RFC] functional operations on nullables as collections #115
Conversation
I have never seen such a large coverage decrease! [I turned the other tests off for faster testing locally, and forgot to turn them back on again.] |
It feels a little strange this being is in NullableArrays rather than base (or, if it really can't get into base/0.5, its own package)... 👍 to this functionality. Edit: IMO The map, getindex are more obviously good for base, I suspect the broadcast stuff is distinct from that... and more controversial (?). |
Already map over Nullable is controversial in Base. JuliaLang/julia#9446 |
importall Base | ||
import Base: promote_op, LinearFast | ||
|
||
# conceptually, nullable types can be considered infinite-order tensor powers of |
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 don't think this comment really helps understanding the code. Personally, I rather find it hard to get. :-)
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 so attached either. I'll leave this comment out.
# warning: not type-stable | ||
map{T}(f, u::Nullable{T}) = u.isnull ? Nullable{Union{}}() : Nullable(f(u.value)) | ||
|
||
# multi-argument map doesn't broadcast, so not very useful, but no harm having |
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.
Idem.
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.
left out the comments
I agree this should go into Base ( There are many points to discuss in the description, but let's try to keep focused on this PR. I think we should keep Regarding type-inference issues, they are dependent on fixing @vchuravy I think with the new |
Thanks for the comments. I see that NullableArrays is not the right place to submit this PR. After addressing @nalimilan's comments [thanks!], I'll move this PR to Base. |
Thank you @nalimilan for the review comments! Much appreciated. I will close this PR after Travis passes and move it to Base later tomorrow. |
Travis passing now (except for unrelated, as far as I can tell, failure on nightly). Will move PR to Base. |
This implements Scala-style
map
andfilter
. I did not implement Scala's fold semantics, because I consider that to be a mistake, since it is inconsistent with the regularfoldr
andfoldl
, which I did keep. Sincemap
is not type-stable, I also implementedbroadcast
as a type-stablemap
.broadcast
also preserves the broadcasting capability of the version for arrays.I also implemented x[] and x[1] to replace
get(x)
. Finally, I added in the iteration interface, which allows imperative-style for loops as well as functional-style higher order functions.John presented several options for propagation in this old mailing list post, which I quote:
The implementation of
broadcast
here doubles as an implementation of (2). This is simply a nice result of the algebraic properties of broadcasting. With new broadcast syntactic sugar, (2) is no longer very verbose. I think alternative syntaxes like ? are hence no longer required.Notes: (optional, sorry for the long PR)
get(Nullable(), 1)
doesn't work (it expects aUnion{}
second argument?!) and how equality comparison is a bit broken (tryNullable() == Nullable()
). I am new to Nullables so please excuse if there is a reason for this behaviour, but I found it heavily unintuitive..
broadcast syntax will not be available.get(u)
in favour ofu[]
. The former syntax seems strange to me, as all other uses ofget
in Julia include a default value. I don't know if this is a popular viewpoint; my background in nullables comes more from Lisp (where(cons x nil)
can be used asNullable(x)
) and Scala than from numerical computing languages.SquareArray{T, ∞}
, which provides a baseline for what should and shouldn't work.) This is perfectly consistent algebraically but the algebra is unimportant. It's only a guide for what kind of functionality should be implemented.broadcast
and for indexing, as identical operations.