Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
79 changes: 64 additions & 15 deletions src/enumerable/enumerable_map.jl
Original file line number Diff line number Diff line change
@@ -1,37 +1,86 @@
struct EnumerableMap{T, S, Q<:Function} <: Enumerable
# This is the HasEltype() version

struct EnumerableMapHasEltype{T, S, Q<:Function} <: Enumerable
source::S
f::Q
end

Base.iteratorsize(::Type{EnumerableMap{T,S,Q}}) where {T,S,Q} = Base.iteratorsize(S) in (Base.HasLength(), Base.HasShape()) ? Base.HasLength() : Base.iteratorsize(S)
Base.iteratorsize(::Type{EnumerableMapHasEltype{T,S,Q}}) where {T,S,Q} = Base.iteratorsize(S) in (Base.HasLength(), Base.HasShape()) ? Base.HasLength() : Base.iteratorsize(S)

Base.eltype(iter::EnumerableMap{T,S,Q}) where {T,S,Q} = T
Base.eltype(iter::Type{EnumerableMapHasEltype{T,S,Q}}) where {T,S,Q} = T

Base.eltype(iter::Type{EnumerableMap{T,S,Q}}) where {T,S,Q} = T
Base.length(iter::EnumerableMapHasEltype{T,S,Q}) where {T,S,Q} = length(iter.source)

Base.length(iter::EnumerableMap{T,S,Q}) where {T,S,Q} = length(iter.source)
function Base.start(iter::EnumerableMapHasEltype{T,S,Q}) where {T,S,Q}
s = start(iter.source)
return s
end

function map(source::Enumerable, f::Function, f_expr::Expr)
TS = eltype(source)
T = Base._return_type(f, Tuple{TS,})
S = typeof(source)
Q = typeof(f)
return EnumerableMap{T,S,Q}(source, f)
function Base.next(iter::EnumerableMapHasEltype{T,S,Q}, s) where {T,S,Q}
x = next(iter.source, s)
v = x[1]
s_new = x[2]
v_new = iter.f(v)::T
return v_new, s_new
end

function Base.start(iter::EnumerableMap{T,S,Q}) where {T,S,Q}
function Base.done(iter::EnumerableMapHasEltype{T,S,Q}, state) where {T,S,Q}
return done(iter.source, state)
end

# This is the EltypeUnknown() version

struct EnumerableMapEltypeUnknown{S, Q<:Function} <: Enumerable
source::S
f::Q
end

Base.iteratorsize(::Type{EnumerableMapEltypeUnknown{S,Q}}) where {S,Q} = Base.iteratorsize(S) in (Base.HasLength(), Base.HasShape()) ? Base.HasLength() : Base.iteratorsize(S)

Base.iteratoreltype(::Type{EnumerableMapEltypeUnknown{S,Q}}) where {S,Q} = Base.EltypeUnknown()

Base.length(iter::EnumerableMapEltypeUnknown) = length(iter.source)

function Base.start(iter::EnumerableMapEltypeUnknown)
s = start(iter.source)
return s
end

function Base.next(iter::EnumerableMap{T,S,Q}, s) where {T,S,Q}
function Base.next(iter::EnumerableMapEltypeUnknown, s)
x = next(iter.source, s)
v = x[1]
s_new = x[2]
v_new = iter.f(v)::T
v_new = iter.f(v)
return v_new, s_new
end

function Base.done(iter::EnumerableMap{T,S,Q}, state) where {T,S,Q}
function Base.done(iter::EnumerableMapEltypeUnknown, state)
return done(iter.source, state)
end

# Implementation of the query operator

function _map(source::Enumerable, f::Function, f_expr::Expr, ::Base.EltypeUnknown)
S = typeof(source)
Q = typeof(f)
println("Unkonwn")
return EnumerableMapEltypeUnknown{S,Q}(source, f)
end

function _map(source::Enumerable, f::Function, f_expr::Expr, ::Base.HasEltype)
TS = eltype(source)
T = Base._return_type(f, Tuple{TS,})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This still relying on inference though? I think to really avoid Inference, we should mirror the implementation of collect on Generators and how the new Broadcast will work.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The strategy here is to use the information from inference if it gives us a concrete type, otherwise not use that information. I think that is the same strategy that Broadcast is using now? As far as I understand it, that would be an ok use of inference.

The question though is whether it actually buys us anything, or whether we could just never use it. At least for a intermediate period it would help with moving things over: it will take a while to change all the sinks to accept iterators that have the EltypeUnknown() trait, and until that is done this implementation here would still return things with HasEltype() in all cases where it does so today.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The strategy here is to use the information from inference if it gives us a concrete type, otherwise not use that information. I think that is the same strategy that Broadcast is using now? As far as I understand it, that would be an ok use of inference.

FWIW, I don't think the question is whether the inferred type is concrete or not. It's rather that the user-visible behavior should be the same whether or not inference gives you a precise type or Any.

if isleaftype(T)
S = typeof(source)
Q = typeof(f)
println("Known")
return EnumerableMapHasEltype{T,S,Q}(source, f)
else
_map(source, f, f_expr, Base.EltypeUnknown())
end
end

function map(source::T, f::Function, f_expr::Expr) where {T<:Enumerable}
return _map(source, f, f_expr, Base.iteratoreltype(T))
end

4 changes: 2 additions & 2 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ source_1 = [1,2,2,3,4]

@test collect(QueryOperators.@filter(QueryOperators.query(source_1), i->i>2)) == [3,4]

@test collect(QueryOperators.@map(QueryOperators.query(source_1), i->i^2)) == [1,4,4,9,16]

group_result_1 = collect(QueryOperators.@groupby(QueryOperators.query(source_1), i->i, i->i^2))

@test group_result_1[1].key == 1
Expand All @@ -28,4 +26,6 @@ group_result_1 = collect(QueryOperators.@groupby(QueryOperators.query(source_1),

@test collect(QueryOperators.@drop(QueryOperators.query(source_1), 2)) == [2,3,4]

include("test_enumerable_map.jl")

end
40 changes: 40 additions & 0 deletions test/test_enumerable_map.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
using QueryOperators
using Base.Test

@testset "map" begin

X = [1,2,3,4]

# Test with eltype known

a = QueryOperators.@map(QueryOperators.query(X), i->i^2)
aa = collect(a)

@test Base.iteratoreltype(typeof(a))==Base.HasEltype()
@test Base.iteratorsize(typeof(a)) == Base.HasLength()
@test length(a) == 4
@test aa == [1,4,9,16]

# Test with eltype unknown

b = QueryOperators.@map(QueryOperators.query(i for i in X), i->i)
bb = collect(b)

@test Base.iteratoreltype(typeof(b))==Base.EltypeUnknown()
@test Base.iteratorsize(typeof(b)) == Base.HasLength()
@test length(b) == 4
@test bb == [1,2,3,4]
@test eltype(bb) == Int

# Test with known source eltype, but inference gives up

c = QueryOperators.@map(QueryOperators.query(X), i->i>10 ? 2 : 4.)
cc = collect(c)

@test Base.iteratoreltype(typeof(c))==Base.EltypeUnknown()
@test Base.iteratorsize(typeof(c)) == Base.HasLength()
@test length(c) == 4
@test cc == [4.,4.,4.,4.]
@test eltype(cc) == Float64

end