diff --git a/src/eachrow.jl b/src/eachrow.jl index c796e3a2..77570ecb 100644 --- a/src/eachrow.jl +++ b/src/eachrow.jl @@ -4,35 +4,41 @@ ## ############################################################################## + # Recursive function that traverses the syntax tree of e, replaces instances of # ":(:(x))" with ":x[row]". +eachrow_replace(x) = x +eachrow_replace(e::QuoteNode) = Expr(:ref, e, :row) + function eachrow_replace(e::Expr) - # Traverse the syntax tree of e - if onearg(e, :cols) - @warn "cols(x) is deprecated, use \$x instead" - # cols(:x) becomes cols(:x)[row] - return Expr(:ref, Expr(:call, :cols, e.args[2]), :row) - elseif is_column_expr(e) - return Expr(:ref, Expr(:$, e.args[1]), :row) + if onearg(e, :^) + return e.args[2] end - if e.head == :. - if e.args[1] isa QuoteNode - e.args[1] = Expr(:ref, e.args[1], :row) - return e - else - return e - end + # Traverse the syntax tree of e + col = get_column_expr(e) + if col !== nothing + return :($e[row]) + # equivalent to protect_replace_syms + elseif e.head == :. + x_new = eachrow_replace(e.args[1]) + y = e.args[2] + y_new = y isa Expr ? eachrow_replace(y) : y + + return Expr(:., x_new, y_new) + else + mapexpr(eachrow_replace, e) end - - Expr(e.head, (isempty(e.args) ? e.args : map(eachrow_replace, e.args))...) end -eachrow_replace(e::QuoteNode) = Expr(:ref, e, :row) +protect_eachrow_replace(e) = e +protect_eachrow_replace(e::Expr) = eachrow_replace(e) -# Set the base case for helper, i.e. for when expand hits an object of type -# other than Expr (generally a Symbol or a literal). -eachrow_replace(x) = x +function eachrow_replace_dotted(e, membernames) + x_new = eachrow_repalce(e.args[1]) + y_new = protect_eachrow_repalce(e.args[2]) + Expr(:., x_new, y_new) +end function eachrow_find_newcols(e::Expr, newcol_decl) if e.head == :macrocall && e.args[1] == Symbol("@newcol") diff --git a/src/macros.jl b/src/macros.jl index d9f2b85e..e954a371 100644 --- a/src/macros.jl +++ b/src/macros.jl @@ -1548,7 +1548,7 @@ function combine_helper(x, args...; deprecation_warning = false) fe = first(exprs) if length(exprs) == 1 && - !(fe isa QuoteNode || onearg(fe, :cols) || is_column_expr(fe)) && + get_column_expr(fe) === nothing && !(fe.head == :(=) || fe.head == :kw) @warn "Returning a Table object from @by and @combine now requires `$(DOLLAR)AsTable` on the LHS." @@ -1668,7 +1668,7 @@ function by_helper(x, what, args...) exprs, outer_flags = create_args_vector(args...) fe = first(exprs) if length(exprs) == 1 && - !(fe isa QuoteNode || onearg(fe, :cols) || is_column_expr(fe)) && + get_column_expr(fe) === nothing && !(fe.head == :(=) || fe.head == :kw) @warn "Returning a Table object from @by and @combine now requires `\$AsTable` on the LHS." diff --git a/src/parsing.jl b/src/parsing.jl index f881a0e1..3a250138 100644 --- a/src/parsing.jl +++ b/src/parsing.jl @@ -8,40 +8,62 @@ end onearg(e::Expr, f) = e.head == :call && length(e.args) == 2 && e.args[1] == f onearg(e, f) = false -is_column_expr(x) = false -is_column_expr(e::Expr) = e.head == :$ +""" + get_column_expr(x) + +If the input is a valid column identifier, i.e. +a `QuoteNode` or an expression beginning with +`$DOLLAR`, returns the underlying identifier. + +If input is not a valid column identifier, +returns `nothing`. +""" +get_column_expr(x) = nothing +function get_column_expr(e::Expr) + e.head == :$ && return e.args[1] + if onearg(e, :cols) + Base.depwarn("cols is deprecated use $DOLLAR to escape column names instead", :cols) + return e.args[2] + end + return nothing +end +get_column_expr(x::QuoteNode) = x -mapexpr(f, e) = Expr(e.head, map(f, e.args)...) +mapexpr(f, e) = Expr(e.head, Base.Generator(f, e.args)...) -replace_syms!(x, membernames) = x -replace_syms!(q::QuoteNode, membernames) = - replace_syms!(Meta.quot(q.value), membernames) -function replace_syms!(e::Expr, membernames) +replace_syms!(membernames, x) = x +replace_syms!(membernames, q::QuoteNode) = addkey!(membernames, q) + +function replace_syms!(membernames, e::Expr) if onearg(e, :^) - e.args[2] - elseif onearg(e, :_I_) - @warn "_I_() for escaping variables is deprecated, use cols() instead" - addkey!(membernames, :($(e.args[2]))) - elseif onearg(e, :cols) - @warn "cols(x) for escaping variables is deprecated, use \$x instead" - addkey!(membernames, :($(e.args[2]))) - elseif is_column_expr(e) - addkey!(membernames, :($(e.args[1]))) - elseif e.head == :quote - addkey!(membernames, Meta.quot(e.args[1]) ) + return e.args[2] + end + + col = get_column_expr(e) + if col !== nothing + return addkey!(membernames, col) elseif e.head == :. - replace_dotted!(e, membernames) + return replace_dotted!(membernames, e) else - mapexpr(x -> replace_syms!(x, membernames), e) + return mapexpr(x -> replace_syms!(membernames, x), e) end end +protect_replace_syms!(membernames, e) = e +protect_replace_syms!(membernames, e::Expr) = replace_syms!(membernames, e) + +function replace_dotted!(membernames, e) + x_new = replace_syms!(membernames, e.args[1]) + y_new = protect_replace_syms!(membernames, e.args[2]) + Expr(:., x_new, y_new) +end + is_simple_non_broadcast_call(x) = false function is_simple_non_broadcast_call(expr::Expr) expr.head == :call && length(expr.args) >= 2 && expr.args[1] isa Symbol && - all(x -> x isa QuoteNode || onearg(x, :cols) || is_column_expr(x), expr.args[2:end]) + all(a -> get_column_expr(a) !== nothing, expr.args[2:end]) end is_simple_broadcast_call(x) = false @@ -51,21 +73,14 @@ function is_simple_broadcast_call(expr::Expr) expr.args[1] isa Symbol && expr.args[2] isa Expr && expr.args[2].head == :tuple && - all(x -> x isa QuoteNode || onearg(x, :cols) || is_column_expr(x), expr.args[2].args) + all(a -> get_column_expr(a) !== nothing, expr.args[2].args) end function args_to_selectors(v) - t = map(v) do arg - if arg isa QuoteNode - arg - elseif onearg(arg, :cols) - @warn "cols(x) is deprecated, use \$x instead" - arg.args[2] - elseif is_column_expr(arg) - arg.args[1] - else - throw(ArgumentError("This path should not be reached, arg: $(arg)")) - end + t = Base.Generator(v) do arg + col = get_column_expr(arg) + col === nothing && throw(ArgumentError("This path should not be reached, arg: $(arg)")) + col end :(DataFramesMeta.make_source_concrete($(Expr(:vect, t...)))) @@ -190,11 +205,9 @@ function get_source_fun(function_expr; exprflags = deepcopy(DEFAULT_FLAGS)) else membernames = Dict{Any, Symbol}() - body = replace_syms!(function_expr, membernames) - + body = replace_syms!(membernames, function_expr) source = :(DataFramesMeta.make_source_concrete($(Expr(:vect, keys(membernames)...)))) inputargs = Expr(:tuple, values(membernames)...) - fun = quote $inputargs -> begin $body @@ -245,121 +258,65 @@ function fun_to_vec(ex::Expr; check_macro_flags_consistency(final_flags) if gensym_names - ex = Expr(:kw, gensym(), ex) + ex = Expr(:kw, QuoteNode(gensym()), ex) end # :x # handled below via dispatch on ::QuoteNode - # Fix any references to `cols` and replace them - # with $ - if onearg(ex, :cols) - ex = Expr(:$, ex.args[2]) - end - - # $:x - if is_column_expr(ex) - return ex.args[1] + ex_col = get_column_expr(ex) + if ex_col !== nothing + return ex_col end if no_dest - source, fun = get_source_fun(ex, exprflags = final_flags) + src, fun = get_source_fun(ex, exprflags = final_flags) return quote - $source => $fun + $src => $fun end end - @assert ex.head == :kw || ex.head == :(=) - lhs = ex.args[1] - - # fix cols - if onearg(lhs, :cols) - lhs = Expr(:$, lhs.args[2]) - end - - rhs = MacroTools.unblock(ex.args[2]) - - if onearg(rhs, :cols) - @warn "cols(x) is deprecated, use \$x instead" - rhs = Expr(:$, rhs.args[2]) - end - - if is_macro_head(rhs, "@byrow") - s = "In keyword argument inputs, `@byrow` must be on the left hand side. " * - "Did you write `y = @byrow f(:x)` instead of `@byrow y = f(:x)`?" - throw(ArgumentError(s)) - end - - # y = ... - if lhs isa Symbol - msg = "Using an un-quoted Symbol on the LHS is deprecated. " * - "Write $(QuoteNode(lhs)) = ... instead." - Base.depwarn(msg, "") - lhs = QuoteNode(lhs) + if !(ex.head == :kw || ex.head == :(=)) + throw(ArgumentError("Malformed expression in DataFramesMeta.jl macro")) end - # :y = :x - if lhs isa QuoteNode && rhs isa QuoteNode - source = rhs - dest = lhs + lhs = let t = ex.args[1] + if t isa Symbol + t = QuoteNode(t) + msg = "Using an un-quoted Symbol on the LHS is deprecated. " * + "Write $t = ... instead." - return quote - $source => $dest + @warn msg end - end - - # :y = $:x - if lhs isa QuoteNode && is_column_expr(rhs) - source = rhs.args[1] - dest = lhs - return quote - $source => $dest + s = get_column_expr(t) + if s === nothing + throw(ArgumentError("Malformed expression oh LHS in DataFramesMeta.jl macro")) end - end - - # $:y = :x - if is_column_expr(lhs) && rhs isa QuoteNode - source = rhs - dest = lhs.args[1] - return quote - $source => $dest - end + s end - # $:y = $:x - if is_column_expr(lhs) && is_column_expr(rhs) - source = rhs.args[1] - dest = lhs.args[1] - return quote - $source => $dest - end - end - - # :y = f(:x) - # :y = f($:x) - # :y = :x + 1 - # :y = $:x + 1 - source, fun = get_source_fun(rhs; exprflags = final_flags) - if lhs isa QuoteNode + rhs = MacroTools.unblock(ex.args[2]) + rhs_col = get_column_expr(rhs) + if rhs_col !== nothing + src = rhs_col dest = lhs - return quote - $source => $fun => $dest - end + return :($src => $dest) end - # $:y = f(:x) - if is_column_expr(lhs) - dest = lhs.args[1] - - return quote - $source => $fun => $dest - end + if is_macro_head(rhs, "@byrow") || is_macro_head(rhs, "@passmissing") + s = "In keyword argument inputs, `@byrow` and `@passmissing`" * + "must be on the left hand side. " * + "Did you write `y = @byrow f(:x)` instead of `@byrow y = f(:x)`?" + throw(ArgumentError(s)) end - throw(ArgumentError("This path should not be reached")) + dest = lhs + src, fun = get_source_fun(rhs; exprflags = final_flags) + return :($src => $fun => $dest) end + fun_to_vec(ex::QuoteNode; no_dest::Bool=false, gensym_names::Bool=false, @@ -376,21 +333,6 @@ function make_source_concrete(x::AbstractVector) end end -protect_replace_syms!(e, membernames) = e -function protect_replace_syms!(e::Expr, membernames) - if e.head == :quote - e - else - replace_syms!(e, membernames) - end -end - -function replace_dotted!(e, membernames) - x_new = replace_syms!(e.args[1], membernames) - y_new = protect_replace_syms!(e.args[2], membernames) - Expr(:., x_new, y_new) -end - function create_args_vector(args...; wrap_byrow::Bool=false) create_args_vector(Expr(:block, args...); wrap_byrow = wrap_byrow) end diff --git a/test/eachrow.jl b/test/eachrow.jl index 0f0a4147..7c34acc6 100644 --- a/test/eachrow.jl +++ b/test/eachrow.jl @@ -179,4 +179,20 @@ y = 0 @test df.b == [400] end +@testset "eachrow with getproperty" begin + df = DataFrame(a = [(x = 1, y = 2), (x = 3, y = 4)], b = 1:2) + + res = @eachrow df begin + :b = :a.x + :b + end + + @test res.b == [2, 5] + + res = @eachrow df begin + :b = $"a".x + :b + end + + @test res.b == [2, 5] +end + end # module diff --git a/test/parsing.jl b/test/parsing.jl new file mode 100644 index 00000000..458d4c91 --- /dev/null +++ b/test/parsing.jl @@ -0,0 +1,63 @@ +module TestParsing + +using Test +using DataFramesMeta +using Statistics + +const ≅ = isequal + +macro protect(x) + esc(DataFramesMeta.get_column_expr(x)) +end + +@testset "Returning columnn identifiers" begin + + v_sym = @protect $[:a, :b] + @test v_sym == [:a, :b] + + v_str = @protect $["a", "b"] + @test v_str == ["a", "b"] + + qn = @protect :x + @test qn == :x + + qn_protect = @protect $:x + @test qn_protect == :x + + str_protect = @protect $"x" + @test str_protect == "x" + + x = "a" + sym_protect = @protect $x + @test sym_protect === "a" + + v = ["a", "b"] + v_protect = @protect $v + @test v === v_protect + + b = @protect $(Between(:a, :b)) + @test b == Between(:a, :b) + + b = @protect $(begin 1 end) + @test b == 1 + + c = @protect cols(:a) + @test c == :a + + i = @protect $1 + @test i == 1 + + n = @protect "hello" + @test n === nothing + + n = @protect 1 + @test n === nothing + + n = @protect begin x end + @test n === nothing + + n = @protect x + @test n === nothing +end + +end # module \ No newline at end of file