Skip to content
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

Added support for broadcasted piping .|>. #16

Merged
merged 14 commits into from
Jun 3, 2020
27 changes: 19 additions & 8 deletions src/Pipe.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export @pipe

const PLACEHOLDER = :_

function rewrite(ff::Expr,target)
function rewrite(ff::Expr, target, elementwise=false)
function replace(arg::Any)
arg #Normally do nothing
end
Expand All @@ -18,11 +18,16 @@ function rewrite(ff::Expr,target)
end
function replace(arg::Expr)
rep = copy(arg)
rep.args = map(replace,rep.args)
rep.args = replace.(rep.args)
rep
end

rep_args = map(replace,ff.args)
if elementwise
rep_arg1 = Symbol(:., ff.args[1])
rep_args = [rep_arg1; replace.(ff.args[2:end])]
else
rep_args = replace.(ff.args)
end
if ff.args != rep_args
#_ subsitution
ff.args=rep_args
Expand All @@ -34,16 +39,19 @@ function rewrite(ff::Expr,target)
rewrite_apply(ff,target)
end


function rewrite_apply(ff, target)
:($ff($target)) #function application
function rewrite_apply(ff, target, elementwise=false)
if elementwise
:($ff.($target)) #function application
else
:($ff($target)) #function application
end
end

function rewrite(ff::Symbol, target)
function rewrite(ff::Symbol, target, elementwise=false)
if ff==PLACEHOLDER
target
else
rewrite_apply(ff,target)
rewrite_apply(ff,target,elementwise)
end
end

Expand All @@ -55,6 +63,9 @@ function funnel(ee::Expr)
if (ee.args[1]==:|>)
target = funnel(ee.args[2]) #Recurse
rewrite(ee.args[3],target)
elseif (ee.args[1]==:.|>)
target = funnel(ee.args[2]) #Recurse
rewrite(ee.args[3],target,true)
else
#Not in a piping situtation
ee #make no change
Expand Down
7 changes: 7 additions & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,10 @@ end
@test _macroexpand( :(@pipe a|>b|>c(_) ) ) == :(c(b(a)))
@test _macroexpand( :(@pipe a|>b(x,_)|>c|>d(_,y) ) ) == :(d(c(b(x,a)),y))
@test _macroexpand( :(@pipe a|>b(xb,_)|>c|>d(_,xd)|>e(xe) |>f(xf,_,yf)|>_[i] ) ) == :(f(xf,(e(xe))(d(c(b(xb,a)),xd)),yf)[i]) #Very Complex

# broadcasting
fn(x) = x^2
@test _macroexpand( :(@pipe fn |> _.(1:2) ) ) == :(fn.(1:2))
Copy link
Owner

Choose a reason for hiding this comment

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

what happens if we do:

_macroexpand( :(@pipe fn .|> _.(1:2) ) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now it produces :(var"._".(1:2)) and @pipe fn .|> _.(1:2) crashes.
I'm not entirely sure, what is desired behaviour, how the resulting code should look like.
In plain Julia fn .|> x->x.(1:2) == fn |> x->x.(1:2), and since fn isa not iterable AFAIK, then fn ,|> produces same result as fn |>, so I'm not sure where I want to go with this.

Copy link
Owner

Choose a reason for hiding this comment

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

we want to match the normal behavour.

Here is an interesting set of cases:

julia> [1,2,2] |> x-> atan.([10,20,30], x)
3-element Array{Float64,1}:
 1.4711276743037347
 1.4711276743037347
 1.5042281630190728

julia> [1,2,2] .|> x-> atan.([10,20,30], x)
3-element Array{Array{Float64,1},1}:
 [1.4711276743037347, 1.5208379310729538, 1.5374753309166493]
 [1.373400766945016, 1.4711276743037347, 1.5042281630190728]
 [1.373400766945016, 1.4711276743037347, 1.5042281630190728]

julia> [1,2,2] .|> x-> atan([10,20,30], x)
ERROR: MethodError: no method matching atan(::Array{Int64,1}, ::Int64)

Copy link
Contributor Author

@racinmat racinmat May 19, 2020

Choose a reason for hiding this comment

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

I see. If I understand it correctly, the embedding of input array is not able to cover cases 2 and 3, because the . notation is not expressive enough, so I should explicitly call broadcast function in the macro, right?
Because I can write test for

@test _macroexpand( :(@pipe [1,2,2] |> atan.([10,20,30], _) ) ) == :(atan.([10,20,30], [1,2,2]))

but I'm kinda lost in decising what should

@test _macroexpand( :(@pipe [1,2,2] .|> atan.([10,20,30], _) ) )

equal to.
What do you think? @oxinabox

@test _macroexpand( :(@pipe 1:10 .|> _*2 ) ) == :((1:10) .* 2)
Copy link
Owner

Choose a reason for hiding this comment

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

can we also test

[true,false] .|> !
[1, 2] |> .+(_, x)
[1, 2] |>  _ .+ x)
[1, 2] .|> .+(_, x)
[1, 2] .|>  _ .+ x)

@test _macroexpand( :(@pipe 1:10 .|> fn ) ) == :(fn.(1:10))
@test _macroexpand( :(@pipe a .|> fn .|> _*2 ) ) == :(fn.(a) .* 2)