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

address vcat return inconsistency #187

Closed
wants to merge 13 commits into from
Closed

address vcat return inconsistency #187

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1689728
address https://github.com/JuliaStats/NullableArrays.jl/issues/167
cjprybol Feb 24, 2017
a36f6a4
reorder import statements
cjprybol Feb 24, 2017
5d9d982
move from operators files to nullablevector files
cjprybol Feb 24, 2017
4aa635c
reduce the piracy
cjprybol Feb 25, 2017
7d948a0
no more piracy!
cjprybol Feb 25, 2017
1b64de2
passing tests
cjprybol Feb 25, 2017
9cc8381
rearrange functions to cleanup comments
cjprybol Feb 25, 2017
b8c87b5
spacing
cjprybol Feb 25, 2017
bb0b1ee
add more tests and reorder tests to match function order
cjprybol Feb 25, 2017
b8211a9
no more stack overflows
cjprybol Feb 25, 2017
d28b7b8
Int64 -> Int
cjprybol Feb 25, 2017
6c30d14
implement 2 argument cases and comment out 3 argument tests
cjprybol Mar 1, 2017
2e2b98a
correct spacing issue
cjprybol Mar 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/nullablevector.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,3 +311,14 @@ function Base.empty!(X::NullableVector)
empty!(X.isnull)
return X
end

notnullarray = Union{filter!(x -> !isa(x, Type{NullableArray}), subtypes(AbstractArray))...}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need this. Just AbstractArray should be enough, and the most specific method will take precedence.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When usingAbstractArray there's no way to exit the vcat -> vcat loop and it results in StackOverflow. We either eternally prepend empty NullableArrays (vcat(NullableVector{eltype(A)}(), A, B)) or eternally try to convert A to a NullableArray (vcat(NullableArray(A), B))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now realize that if I call typed_hcat, typed_vcat instead of calling hcat/vcat I can probably avoid the StackOverflow while using the simpler AbstractArray as you suggested.

notnullmatrix = typeintersect(notnullarray, AbstractMatrix)
notnullvector = typeintersect(notnullarray, AbstractVector)

Base.vcat{T <: notnullarray}(A::T, B::NullableArray) = Base.vcat(NullableArray(A), B)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wasteful since it creates a copy. Maybe we can use this trick to force creating a NullableArray: vcat(NullableArray{eltype(A)}(), A, B).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, no need for Base..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this case of adding an empty NullableArray to the beginning of the vcat/hcat call only works on vcat with Vectors, but I've added it for that case. Every other case I tried will fail in dimension mismatch checks

Base.vcat{T <: notnullmatrix}(A::T, B::NullableArray) = Base.vcat(NullableArray(A), B)
Base.vcat{T <: notnullvector}(A::T, B::NullableVector) = Base.vcat(NullableArray(A), B)

Base.hcat{T <: notnullvector}(A::T, B::NullableArray) = Base.hcat(NullableArray(A), B)
Base.hcat{T <: notnullmatrix}(A::T, B::NullableArray) = Base.hcat(NullableArray(A), B)
2 changes: 1 addition & 1 deletion src/operators.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
## Lifted operators

importall Base.Operators
import Base: promote_op, abs, abs2, sqrt, cbrt, scalarmin, scalarmax, isless
import Base: abs, abs2, cbrt, isless, promote_op, scalarmin, scalarmax, sqrt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted

using Compat: @compat, @functorize

if isdefined(Base, :fieldname) && Base.fieldname(Nullable, 1) == :hasvalue # Julia 0.6
Expand Down
17 changes: 17 additions & 0 deletions test/nullablevector.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,4 +223,21 @@ module TestNullableVector
X = NullableArray(A, M)
empty!(X)
@test isempty(X)

@test typeof(hcat(NullableArray(1:2), 3:4)) == NullableArrays.NullableArray{Int64,2}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for NullableArrays..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@test typeof(hcat(1:2, NullableArray(3:4))) == NullableArrays.NullableArray{Int64,2}
@test typeof(vcat(NullableArray(1:2), 3:4)) == NullableArrays.NullableArray{Int64,1}
@test typeof(vcat(1:2, NullableArray(3:4))) == NullableArrays.NullableArray{Int64,1}

@test typeof(vcat(NullableArray([1 2]), [3 4])) == NullableArrays.NullableArray{Int64,2}
@test typeof(vcat([1 2], NullableArray([3 4]))) == NullableArrays.NullableArray{Int64,2}
@test typeof(hcat(NullableArray(1:2), 3:4, 5:6)) == NullableArrays.NullableArray{Int64,2}
@test typeof(hcat(1:2, NullableArray(3:4), 5:6)) == NullableArrays.NullableArray{Int64,2}
@test typeof(hcat(1:2, 3:4, NullableArray(5:6))) == NullableArrays.NullableArray{Int64,2}
@test typeof(vcat(NullableArray(1:2), 3:4, 5:6)) == NullableArrays.NullableArray{Int64,1}
@test typeof(vcat(1:2, NullableArray(3:4), 5:6)) == NullableArrays.NullableArray{Int64,1}
@test typeof(vcat(1:2, 3:4, NullableArray(5:6))) == NullableArrays.NullableArray{Int64,1}
@test typeof(vcat(NullableArray([1 2]), [3 4], [5 6])) == NullableArrays.NullableArray{Int64,2}
@test typeof(vcat([1 2], NullableArray([3 4]), [5 6])) == NullableArrays.NullableArray{Int64,2}
@test typeof(vcat([1 2], [3 4], NullableArray([5 6]))) == NullableArrays.NullableArray{Int64,2}
end