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

Commit

Permalink
add back check to differentiate scalars from AbstractArrays
Browse files Browse the repository at this point in the history
  • Loading branch information
cjprybol committed Mar 15, 2017
1 parent 7310681 commit de280ba
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/datatable/datatable.jl
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,11 @@ type DataTable <: AbstractDataTable
if isa(c, Range)
columns[i] = collect(c)
elseif !isa(c, AbstractVector)
throw(DimensionMismatch("columns must be 1-dimensional"))
if isa(c, AbstractArray)

This comment has been minimized.

Copy link
@nalimilan

nalimilan Mar 16, 2017

Member

This double branch with twice the same instruction isn't pretty. How about just having a isa(c, AbstractArray) && !isa(c, AbstractVector) check?

This comment has been minimized.

Copy link
@cjprybol

cjprybol Mar 16, 2017

Author Contributor

Instructions are different. columns[i] = [c] vs. columns[i] = c. The above code to fill scalars only gets called if the min and max are different. If we have a table DataTable(A = 1, B = 2) then this will get called, but if you have DataTable(A = [1, 1], B = 2), then min and max are different and the above code is called to fill B = 2 to B = [2, 2]

This comment has been minimized.

Copy link
@nalimilan

nalimilan Mar 17, 2017

Member

Ah, I had misread the code. It would be more logical not to rely on that length check for handling scalars then, since it doesn't catch all the intended cases.

throw(DimensionMismatch("columns must be 1-dimensional"))
else
columns[i] = [c]
end
else
columns[i] = c
end
Expand Down
1 change: 1 addition & 0 deletions test/constructors.jl
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ module TestConstructors
@test_throws DimensionMismatch DataTable(a=1, b=[])
@test_throws DimensionMismatch DataTable(Any[collect(1:10)], DataTables.Index([:A, :B]))
@test_throws DimensionMismatch DataTable(A = rand(2,2))
@test_throws DimensionMismatch DataTable(A = rand(2,1))
end

@testset "column types" begin
Expand Down

0 comments on commit de280ba

Please sign in to comment.