-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: collect without inference #135
Changes from all commits
cf31fa0
ef30634
a74cbe7
0814597
59347bb
6c05925
89cc30e
dac0578
1f51b23
ca280ef
d196912
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
""" | ||
`collect_columns(itr)` | ||
|
||
Collect an iterable as a `Columns` object if it iterates `Tuples` or `NamedTuples`, as a normal | ||
`Array` otherwise. | ||
|
||
## Examples | ||
|
||
```jldoctest collect | ||
julia> s = [(1,2), (3,4)]; | ||
|
||
julia> collect_columns(s) | ||
2-element Columns{Tuple{Int64,Int64}}: | ||
(1, 2) | ||
(3, 4) | ||
|
||
julia> s = Iterators.filter(isodd, 1:8); | ||
|
||
julia> collect_columns(s) | ||
4-element Array{Int64,1}: | ||
1 | ||
3 | ||
5 | ||
7 | ||
``` | ||
""" | ||
collect_columns(itr) = collect_columns(itr, Base.iteratorsize(itr)) | ||
|
||
function collect_columns(itr, ::Union{Base.HasShape, Base.HasLength}) | ||
st = start(itr) | ||
el, st = next(itr, st) | ||
dest = similar(arrayof(typeof(el)), length(itr)) | ||
dest[1] = el | ||
collect_to_columns!(dest, itr, 2, st) | ||
end | ||
|
||
function collect_to_columns!(dest::AbstractArray{T}, itr, offs, st) where {T} | ||
# collect to dest array, checking the type of each result. if a result does not | ||
# match, widen the result type and re-dispatch. | ||
i = offs | ||
while !done(itr, st) | ||
el, st = next(itr, st) | ||
if fieldwise_isa(el, T) | ||
@inbounds dest[i] = el | ||
i += 1 | ||
else | ||
new = widencolumns(dest, i, el, T) | ||
@inbounds new[i] = el | ||
return collect_to_columns!(new, itr, i+1, st) | ||
end | ||
end | ||
return dest | ||
end | ||
|
||
function collect_columns(itr, ::Base.SizeUnknown) | ||
st = start(itr) | ||
el, st = next(itr, st) | ||
dest = similar(arrayof(typeof(el)), 1) | ||
dest[1] = el | ||
grow_to_columns!(dest, itr, 2, st) | ||
end | ||
|
||
function grow_to_columns!(dest::AbstractArray{T}, itr, offs, st) where {T} | ||
# collect to dest array, checking the type of each result. if a result does not | ||
# match, widen the result type and re-dispatch. | ||
i = offs | ||
while !done(itr, st) | ||
el, st = next(itr, st) | ||
if fieldwise_isa(el, T) | ||
push!(dest, el) | ||
i += 1 | ||
else | ||
new = widencolumns(dest, i, el, T) | ||
push!(new, el) | ||
return grow_to_columns!(new, itr, i+1, st) | ||
end | ||
end | ||
return dest | ||
end | ||
|
||
@generated function fieldwise_isa(el::S, ::Type{T}) where {S<:Tup, T} | ||
if all((s <: t) for (s, t) in zip(S.parameters, T.parameters)) | ||
return :(true) | ||
else | ||
return :(false) | ||
end | ||
end | ||
|
||
@generated function fieldwise_isa(el::S, ::Type{T}) where {S, T} | ||
if S <: T | ||
return :(true) | ||
else | ||
return :(false) | ||
end | ||
end | ||
|
||
function widencolumns(dest, i, el::S, ::Type{T}) where{S <: Tup, T} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this should have |
||
sp, tp = S.parameters, T.parameters | ||
idx = find(!(s <: t) for (s, t) in zip(sp, tp)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be good to see if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, figured out that was missing when changing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should also add a test for #101 in the |
||
new = dest | ||
for l in idx | ||
newcol = Array{promote_type(sp[l], tp[l])}(length(dest)) | ||
copy!(newcol, 1, column(dest, l), 1, i-1) | ||
new = setcol(new, l, newcol) | ||
end | ||
new | ||
end | ||
|
||
function widencolumns(dest, i, el::S, ::Type{T}) where{S, T} | ||
new = Array{promote_type(S, T)}(length(dest)) | ||
copy!(new, 1, dest, 1, i-1) | ||
new | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
@testset "collectnamedtuples" begin | ||
v = [@NT(a = 1, b = 2), @NT(a = 1, b = 3)] | ||
@test collect_columns(v) == Columns(@NT(a = Int[1, 1], b = Int[2, 3])) | ||
|
||
# test inferrability with constant eltype | ||
itr = [@NT(a = 1, b = 2), @NT(a = 1, b = 2), @NT(a = 1, b = 12)] | ||
st = start(itr) | ||
el, st = next(itr, st) | ||
dest = similar(IndexedTables.arrayof(typeof(el)), 3) | ||
dest[1] = el | ||
@inferred IndexedTables.collect_to_columns!(dest, itr, 2, st) | ||
|
||
v = [@NT(a = 1, b = 2), @NT(a = 1.2, b = 3)] | ||
@test collect_columns(v) == Columns(@NT(a = [1, 1.2], b = Int[2, 3])) | ||
@test typeof(collect_columns(v)) == typeof(Columns(@NT(a = [1, 1.2], b = Int[2, 3]))) | ||
|
||
v = [@NT(a = 1, b = 2), @NT(a = 1.2, b = "3")] | ||
@test collect_columns(v) == Columns(@NT(a = [1, 1.2], b = Any[2, "3"])) | ||
@test typeof(collect_columns(v)) == typeof(Columns(@NT(a = [1, 1.2], b = Any[2, "3"]))) | ||
|
||
v = [@NT(a = 1, b = 2), @NT(a = 1.2, b = 2), @NT(a = 1, b = "3")] | ||
@test collect_columns(v) == Columns(@NT(a = [1, 1.2, 1], b = Any[2, 2, "3"])) | ||
@test typeof(collect_columns(v)) == typeof(Columns(@NT(a = [1, 1.2, 1], b = Any[2, 2, "3"]))) | ||
|
||
# length unknown | ||
itr = Iterators.filter(isodd, 1:8) | ||
tuple_itr = (@NT(a = i+1, b = i-1) for i in itr) | ||
@test collect_columns(tuple_itr) == Columns(@NT(a = [2, 4, 6, 8], b = [0, 2, 4, 6])) | ||
tuple_itr_real = (i == 1 ? @NT(a = 1.2, b =i-1) : @NT(a = i+1, b = i-1) for i in itr) | ||
@test collect_columns(tuple_itr_real) == Columns(@NT(a = Real[1.2, 4, 6, 8], b = [0, 2, 4, 6])) | ||
end | ||
|
||
@testset "collecttuples" begin | ||
v = [(1, 2), (1, 3)] | ||
@test collect_columns(v) == Columns((Int[1, 1], Int[2, 3])) | ||
@inferred collect_columns(v) | ||
|
||
v = [(1, 2), (1.2, 3)] | ||
@test collect_columns(v) == Columns(([1, 1.2], Int[2, 3])) | ||
|
||
v = [(1, 2), (1.2, "3")] | ||
@test collect_columns(v) == Columns(([1, 1.2], Any[2, "3"])) | ||
@test typeof(collect_columns(v)) == typeof(Columns(([1, 1.2], Any[2, "3"]))) | ||
|
||
v = [(1, 2), (1.2, 2), (1, "3")] | ||
@test collect_columns(v) == Columns(([1, 1.2, 1], Any[2, 2, "3"])) | ||
# length unknown | ||
itr = Iterators.filter(isodd, 1:8) | ||
tuple_itr = ((i+1, i-1) for i in itr) | ||
@test collect_columns(tuple_itr) == Columns(([2, 4, 6, 8], [0, 2, 4, 6])) | ||
tuple_itr_real = (i == 1 ? (1.2, i-1) : (i+1, i-1) for i in itr) | ||
@test collect_columns(tuple_itr_real) == Columns(([1.2, 4, 6, 8], [0, 2, 4, 6])) | ||
@test typeof(collect_columns(tuple_itr_real)) == typeof(Columns(([1.2, 4, 6, 8], [0, 2, 4, 6]))) | ||
end | ||
|
||
@testset "collectscalars" begin | ||
v = (i for i in 1:3) | ||
@test collect_columns(v) == [1,2,3] | ||
@inferred collect_columns(v) | ||
|
||
v = (i == 1 ? 1.2 : i for i in 1:3) | ||
@test collect_columns(v) == collect(v) | ||
|
||
itr = Iterators.filter(isodd, 1:100) | ||
@test collect_columns(itr) == collect(itr) | ||
real_itr = (i == 1 ? 1.5 : i for i in itr) | ||
@test collect_columns(real_itr) == collect(real_itr) | ||
@test eltype(collect_columns(real_itr)) == Float64 | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this doesn't need to be
@generated
, aBase.@pure
or@inline
might work here. But I guess once a package has@generated
it really doesn't matter how many of them are there :-pThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Base.@pure
seemed to not work as nicely, maybe we can leave as is now and do a "remove all generated functions" PR in the future (maybe in Julia 0.7).