address vcat return inconsistency

[cjprybol]

https://github.com/JuliaStats/NullableArrays.jl/issues/167

[codecov-io]

Codecov Report

Merging #187 into master will increase coverage by 0.05%. The diff coverage is 75%.

@@            Coverage Diff             @@
##           master     #187      +/-   ##
==========================================
+ Coverage   64.56%   64.62%   +0.05%     
==========================================
  Files          13       13              
  Lines         697      701       +4     
==========================================
+ Hits          450      453       +3     
- Misses        247      248       +1

Impacted Files	Coverage Δ
src/nullablevector.jl	86.44% <75%> (-0.41%)	:x:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b584c81...2e2b98a. Read the comment docs.

[nalimilan]

Thanks, but I'd really like to avoid having to copy so much code from Base. This is https://github.com/JuliaLang/julia/issues/2326.

In the short term, I'm not sure what's the best approach. Isn't there a way to use the machinery from Base without reimplementing the full method?

[nalimilan]

I guess one partial solution is to define methods like vcat(::AbstractArray, ::NullableArray, ::AbstractArray...) which call Base.type_hcat with the correct return type. Cf. https://github.com/JuliaStats/DataArrays.jl/pull/213.

[cjprybol]

I need to figure out how to handle the 3+ argument case but this is working nicely for the 2 argument cases

julia> @test typeof(vcat(1:2, NullableArray(3:4))) == NullableArrays.NullableArray{Int64,1}
Test Passed
  Expression: typeof(vcat(1:2,NullableArray(3:4))) == NullableArrays.NullableArray{Int64,1}
   Evaluated: NullableArrays.NullableArray{Int64,1} == NullableArrays.NullableArray{Int64,1}

julia> @test typeof(vcat([1 2], NullableArray([3 4]))) == NullableArrays.NullableArray{Int64,2}
Test Passed
  Expression: typeof(vcat([1 2],NullableArray([3 4]))) == NullableArrays.NullableArray{Int64,2}
   Evaluated: NullableArrays.NullableArray{Int64,2} == NullableArrays.NullableArray{Int64,2}

julia> @test typeof(hcat(1:2, NullableArray(3:4))) == NullableArrays.NullableArray{Int64,2}
Test Passed
  Expression: typeof(hcat(1:2,NullableArray(3:4))) == NullableArrays.NullableArray{Int64,2}
   Evaluated: NullableArrays.NullableArray{Int64,2} == NullableArrays.NullableArray{Int64,2}

julia> @test typeof(hcat([1 2], NullableArray([3 4]))) == NullableArrays.NullableArray{Int64,2}
Test Passed
  Expression: typeof(hcat([1 2],NullableArray([3 4]))) == NullableArrays.NullableArray{Int64,2}
   Evaluated: NullableArrays.NullableArray{Int64,2} == NullableArrays.NullableArray{Int64,2}

[cjprybol]

Should these promote to NullableCategoricalArray?

julia> vcat(categorical([1,2,3]), NullableArray([4,5,6]))
6-element NullableArrays.NullableArray{Int64,1}:
 1
 2
 3
 4
 5
 6

julia> vcat(NullableArray([4,5,6]), categorical([1,2,3]))
6-element NullableArrays.NullableArray{Int64,1}:
 4
 5
 6
 1
 2
 3

[nalimilan]

Should these promote to NullableCategoricalArray?

Yes, but that's tricky since doing so forces one package to depend on the other one, and as long as StatsModels depends on CategoricalArrays we'd better get rid of the NullableArrays dependency (https://github.com/JuliaData/CategoricalArrays.jl/pull/55). So for now I would leave this case as is.

[cjprybol]

I think we should close this. I read through a few dozen pages of search results for vcat usage in Julia code here on GitHub and saw a usage pattern ~ vcat(a,b) >> vcat(a...) ~= vcat(a,b,c) >> vcat(a,b,c,d). If we could redirect this into a brainstorm about how to address https://github.com/JuliaLang/julia/issues/2326, I think that would be a much better use of our time and I'd be happy to try and implement suggestions. I tried variants of https://github.com/JuliaLang/julia/issues/2326#issuecomment-44790021 and couldn't get any of them to work

[nalimilan]

I've tried starting a discussion about the way to move forward at https://github.com/JuliaLang/julia/pull/20815, let's see what can be done about this.

[ararslan]

In the meantime, thanks so much for your hard work here, Cameron. Much appreciated.