JuliaStats / NullableArrays.jl

DEPRECATED Prototype of the new JuliaStats NullableArrays package
Other
35 stars 21 forks source link

isnull.(x) #180

Open felipenoris opened 7 years ago

felipenoris commented 7 years ago

Using current master.

using NullableArrays
x = NullableArray{String}(2)
x[1] = "hey"
isnull.(x)

Gives:

2-element NullableArrays.NullableArray{Bool,1}:
 false
 #NULL

I propose to change it, so that the result is [ false, true ] in this case (edit: or NullableArray([false,true]), which I guess is the current behavior of Nullable comparison for now) Anyone agree with that?

nalimilan commented 7 years ago

That's been considered, but for now @johnmyleswhite preferred to go with the simpler solution of applying standard lifting semantics to all functions (see https://github.com/JuliaStats/NullableArrays.jl/pull/166#issuecomment-260401324, https://github.com/JuliaStats/NullableArrays.jl/issues/144). This might change in the future once we have more experience.

Is there any reason why you cannot/don't want to use isnull(x)?

felipenoris commented 7 years ago

I wanted to use this kind of syntax: x[isnull.(x)].

nalimilan commented 7 years ago

Mmm, I thought isnull(x::NullableArray) was defined as the vectorized version of isnull, but apparently it isn't (which is consistent with isnull retuning false when the argument isn't a Nullable). So there's no correct way of doing this currently except for x.isnull, which should be discouraged.

I guess we should add a special lift method then.

(Though note that your example doesn't make much sense AFAICT, since it will return a vector of null values...)

felipenoris commented 7 years ago

You´re right... That´s why I considered isnull.(x) returning Array{Bool} instead of nullables. If that´s the case, I would think that isnull.(lift(x)) should work. I put up a very simple module with this kind of lift function at https://github.com/felipenoris/Lifting.jl , obviously not the same kind of solution discussed at https://github.com/JuliaLang/julia/pull/18758

update: missing dots