Skip to content
This repository was archived by the owner on May 4, 2019. It is now read-only.

== and missing values #74

Closed
nalimilan opened this issue Sep 18, 2015 · 6 comments
Closed

== and missing values #74

nalimilan opened this issue Sep 18, 2015 · 6 comments

Comments

@nalimilan
Copy link
Member

I would expect ==(::Nullable, ::Nullable) to return Nullable() when one of the arguments is missing, instead of raising an error -- and defer the error to the point when the code calls get (if it does). What do you think?

For reference, the current behavior is:

julia> Nullable(1) == Nullable(1)
Nullable(true)

julia> Nullable(1) == Nullable(2)
Nullable(false)

julia> Nullable(1) == Nullable()
ERROR: 
 in == at /home/milan/.julia/NullableArrays/src/operators.jl:55

PS: I guess ==(::Nullable, ::Nullable) is supposed to get merged into Base at some point?

@nalimilan
Copy link
Member Author

This issue is actually closely related to JuliaLang/julia#13207. If Nullable() == Nullable() continues to raise an error, there's little point for ==(::Nullable, ::Nullable) to return a Nullable{Bool}: better return a Bool. Then support for Nullable in if wouldn't be required.

I'm not saying that's a good idea, but that's another data point in that discussion.

@johnmyleswhite
Copy link
Member

Let's just submit a PR to Base that uses the natural lifting semantics for ==.

I really don't want us to waste our time discussing these kinds of issues until we have a more polished package that people can use with confidence that it's both correct and performant.

I find these theory-world level design debates fairly poisonous and I've wasted too much of my life negotiating semantics when I should have just been writing more code.

@nalimilan
Copy link
Member Author

By "natural lifting semantics", do you mean my proposal, or the current behaviour in NullableArrays.jl?

@johnmyleswhite
Copy link
Member

I mean the semantics we use for all the math operations f(x::Nullable...) is NULL if and only if any of the inputs is NULL.

@nalimilan
Copy link
Member Author

I would be fine with that.

@nalimilan
Copy link
Member Author

I've just found out this:

julia> Nullable(1) == Nullable()
ERROR: 
 [inlined code] from error.jl:22
 in == at /home/milan/.julia/NullableArrays.jl/src/operators.jl:55

julia> Nullable(1) == Nullable{Int}()
Nullable{Bool}()

The bug is only the check for isbits(), which fails for Union{}. I suggest replacing all isbits() checks in operators.jl with isbitsorbottom(x) = isbits(x) || x == Union{}. This will also allow things like Nullable(1) + Nullable() == Nullable(). OK?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants