Skip to content
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

Missing nullable operations? #16889

Closed
TotalVerb opened this issue Jun 12, 2016 · 10 comments
Closed

Missing nullable operations? #16889

TotalVerb opened this issue Jun 12, 2016 · 10 comments
Labels
domain:missing data Base.missing and related functionality

Comments

@TotalVerb
Copy link
Contributor

TotalVerb commented Jun 12, 2016

I was surprised that Nullable doesn't support map and filter. These are pretty common and idiomatic ways to work with Nullables in other languages. Are these operations missing, or was it a conscious decision to exclude them?

It seems like it would be no issue to define filter:

filter(p, Nullable{T}()) ⇒ Nullable{T}()
filter(p, Nullable(x)) ⇒ p(x) ? Nullable(x) : Nullable{T}()

the current behaviour is a non-working lazy Filter. I couldn't find a short way to express this operation currently.

map on the other hand is type-unstable, so I can see why it might not be included:

map(f, Nullable{T}()) ⇒ Nullable{Union{}}()
map(f, Nullable(x)) ⇒ Nullable(f(x))

That's unfortunate. If there's no way to fix that, then maybe there's no point in keeping filter either.

@TotalVerb
Copy link
Contributor Author

TotalVerb commented Jun 12, 2016

Furthermore, I expected Nullable{Int}(10)[] to work as a getter that throws an exception if the nullable were null. The syntax for this seems to be get(Nullable{Int}(10)). This is fairly unlike anything else in the language, and seems to be the only one-argument get method [never mind, there's at least one other in sparse/cholmod.jl]. I would have expected nullables to behave more like zero-or-one-element collections.

@nalimilan
Copy link
Member

nalimilan commented Jun 12, 2016

See #9446 about map. As you can see from my recent comment, I'm no longer opposed to it, but we need to sort out type inference issues first. Or we could live with returning Nullable{Union{}}() for missing values, not sure whether that's good enough.

filter and [] could certainly be implemented if you find them useful enough to make a PR. :-) (The advantage of get is that you can also provide an optional default value.)

See also JuliaStats/NullableArrays.jl#111 and #16080 about missing arithmetic operators for Nullable.

Cc: @johnmyleswhite

@TotalVerb
Copy link
Contributor Author

TotalVerb commented Jun 12, 2016

My experience with get is that getindex is typically the error-raising version, while get is the version that allows a default:

julia> get([1, 2, 3], 2)
ERROR: MethodError: no method matching get(::Array{Int64,1}, ::Int64)
Closest candidates are:
  get(::AbstractArray{T,N}, ::Integer, ::Any)
  get(::ObjectIdDict, ::ANY, ::ANY)
  get(::AbstractArray{T,N}, ::Tuple{}, ::Any)
  ...
 in eval(::Module, ::Any) at ./boot.jl:225
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

julia> get([1, 2, 3], 2, 0)
2

But for Nullables, both meanings seem coalesced into get, which was surprising for me.

(The order of arguments for get also defies convention [since the index comes before the default value, whereas in other functions the ind(ices) are the last arguments], but I think that ship has long sailed.)

@TotalVerb
Copy link
Contributor Author

I'll work on this later today and see if there is a nice solution.

@TotalVerb
Copy link
Contributor Author

I think I have a reasonable solution, though I need to work out the details still. If map is type-unstable, that's too bad. However, Base already has a type-stable version, named broadcast. The semantics of broadcast match perfectly the semantics of lifting over nullables: non-nulls have length 1 and are broadcast to the shape of nulls, which have length 0. So not only does this give good behaviour while reusing Base's existing infrastructure, it also provides a nice syntax for auto-lifting: the func. syntax that Base already has. This is also perfectly sound algebraically, so there is no punning involved.

@TotalVerb
Copy link
Contributor Author

I submitted a PR to NullableArrays: JuliaStats/NullableArrays.jl#115. Further discussion can move over there.

@nalimilan
Copy link
Member

Reopening as this should really go into Base, it has nothing specific to NullableArrays.

@cstjean
Copy link
Contributor

cstjean commented Jun 16, 2016

I'll note that #7903 is about to make numbers non-iterable/non-collectable.

@TotalVerb
Copy link
Contributor Author

I don't think that affects the substance of this issue. A number is not a container (though it is convenient to promote it to a zero-dimensional container when necessary). Currently, this autopromotion is done transparently by allowing numbers to be treated as zero-dimensional containers of themselves.

A Nullable is most certainly a container. That (and speed...) is what distinguishes it from Union{T, Void}. It would be a mistake/omission IMO to make them non-iterable/non-collectable.

@TotalVerb
Copy link
Contributor Author

#16961 should have closed this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:missing data Base.missing and related functionality
Projects
None yet
Development

No branches or pull requests

3 participants