Fix keyword argument bug (#400)

* fix

* progress

NEWS.md

@@ -1,3 +1,6 @@
+# DataFramesMeta v0.15.3 Release Notes
+* Fixed a bug ([#399](https://github.com/JuliaData/DataFramesMeta.jl/issues/399)) where keyword arguments were accidentally ignored ([#400](https://github.com/JuliaData/DataFramesMeta.jl/pull/400#pullrequestreview-2180944667))
+
 # DataFramesMeta v0.15.2 Release notes
 * Bumped the Chain.jl compat entry in the Project.toml ([#382](https://github.com/JuliaData/DataFramesMeta.jl/pull/391))
 

Project.toml

@@ -1,6 +1,6 @@
 name = "DataFramesMeta"
 uuid = "1313f7d8-7da2-5740-9ea0-a2ca25f37964"
-version = "0.15.2"
+version = "0.15.3"
 
 [deps]
 Chain = "8be319e6-bccf-4806-a6f7-6fae938471bc"

src/macros.jl

@@ -759,7 +759,6 @@ write
 
 function subset_helper(x, args...)
     x, exprs, outer_flags, kw = get_df_args_kwargs(x, args...; wrap_byrow = false)
-
     t = (fun_to_vec(ex; no_dest=true, outer_flags=outer_flags) for ex in exprs)
     quote
         $subset($x, $(t...); (skipmissing = true,)..., $(kw...))

src/parsing.jl

@@ -499,22 +499,35 @@ end
 
 function get_df_args_kwargs(x, args...; wrap_byrow = false)
     kw = []
+    # x is normally a data frame. But if the call looks like
+    # transform(df, :x = 1; copycols = false)
+    # then x is actually Expr(:parameters, Expr(:kw, :copycole, false))
+    # When this happens, we assign x to the data frame, use only
+    # the rest of the args, and keep trask of the keyword argument.
     if x isa Expr && x.head === :parameters
         append!(kw, x.args)
         x = first(args)
         args = args[2:end]
     end
 
-    transforms, outer_flags, kw = create_args_vector!(kw, args...; wrap_byrow = wrap_byrow)
+    if args isa Tuple
+        blockarg = Expr(:block, args...)
+    else
+        blockarg = args
+    end
 
-    return (x, transforms, outer_flags, kw)
-end
+    # create_args_vector! has an exclamation point because
+    # we modify the keyword arguments kw
+    transforms, outer_flags, kw = create_args_vector!(kw, blockarg; wrap_byrow = wrap_byrow)
 
-function create_args_vector!(kw, args...; wrap_byrow::Bool=false)
-    create_args_vector!(kw, Expr(:block, args...); wrap_byrow = wrap_byrow)
+    return (x, transforms, outer_flags, kw)
 end
 
 function get_kw_from_macro_call(e::Expr)
+    if length(e.args) != 3
+        throw(ArgumentError("Invalid @kwarg expression"))
+    end
+
     nv = e.args[3]
 
     return nv
@@ -532,8 +545,6 @@ the block as an array. If a simple expression,
 wrap the expression in a one-element vector.
 """
 function create_args_vector!(kw, arg; wrap_byrow::Bool=false)
-    # TODO: Pass vector of keyword arguments to this function
-    # and modify by detecting presence of `@kwarg`.
     arg, outer_flags = extract_macro_flags(MacroTools.unblock(arg))
 
     if wrap_byrow
@@ -545,18 +556,18 @@ function create_args_vector!(kw, arg; wrap_byrow::Bool=false)
     end
 
     # @astable means the whole block is one transformation
+
     if arg isa Expr && arg.head == :block && !outer_flags[ASTABLE_SYM][]
         x = MacroTools.rmlines(arg).args
-        kw = []
         transforms = []
-        seen_kw = false
+        seen_kw_macro = false
         for xi in x
             if is_macro_head(xi, "@kwarg")
                 kw_item = get_kw_from_macro_call(xi)
                 push!(kw, kw_item)
-                seen_kw = true
+                seen_kw_macro = true
             else
-                if seen_kw
+                if seen_kw_macro
                     throw(ArgumentError("@kwarg calls must be at end of block"))
                 end
                 push!(transforms, xi)

test/keyword_arguments.jl

@@ -401,4 +401,28 @@ end
     @test df2 == correct
 end
 
+@testset "Multiple arguments #399" begin
+    correct = df[df.a .== 1, :]
+    correct_view = view(df, df.a .== 1, :)
+
+    df2 = @subset(df, :a .== 1, :b .== 3; view = true)
+    @test df2 ≈ correct_view
+
+    @test_throws ArgumentError @subset(df, :a .== 1, :b .== 3; skipmissing = false)
+    @test_throws ArgumentError @subset(df, :a .== 1, :b .== 3; skipmissing = false, view = true)
+
+    correct = transform(df, :a => ByRow(t -> t + 1) => :y, :b => ByRow(t -> t + 2) => :z)
+    df2 = @rtransform(df, :y = :a + 1, :z = :b + 2; copycols = false)
+    @test df2 ≅ correct
+    @test df.a === df2.a
+
+    correct = DataFrame(b_mean = [3.5, 5.0], b_first = [3, 5])
+    df2 = @combine(gd, :b_mean = mean(skipmissing(:b)), :b_first = first(:b); keepkeys = false)
+    @test df2 ≅ correct
+end
+
+@testset "@kwarg errors" begin
+
+end
+
 end # module