From 71f31cb553933f4105b080968aa6119b391d67cc Mon Sep 17 00:00:00 2001 From: Peter Deffebach Date: Wed, 26 Jun 2024 18:50:21 -0400 Subject: [PATCH 1/3] fix --- src/macros.jl | 1 - src/parsing.jl | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/macros.jl b/src/macros.jl index 855714e..acb8ebc 100644 --- a/src/macros.jl +++ b/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...)) diff --git a/src/parsing.jl b/src/parsing.jl index a431ec6..00bff67 100644 --- a/src/parsing.jl +++ b/src/parsing.jl @@ -545,20 +545,20 @@ 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 = !isempty(kw) for xi in x if is_macro_head(xi, "@kwarg") + if seen_kw + throw(ArgumentError("@kwarg calls must be at end of block")) + end kw_item = get_kw_from_macro_call(xi) push!(kw, kw_item) seen_kw = true else - if seen_kw - throw(ArgumentError("@kwarg calls must be at end of block")) - end push!(transforms, xi) end end From d2f3982e831b2be3091db3d2ed9b765afc0cd6ab Mon Sep 17 00:00:00 2001 From: Peter Deffebach Date: Fri, 28 Jun 2024 20:26:26 -0400 Subject: [PATCH 2/3] progress --- src/parsing.jl | 35 +++++++++++++++++++++++------------ test/keyword_arguments.jl | 24 ++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/parsing.jl b/src/parsing.jl index 00bff67..e2da685 100644 --- a/src/parsing.jl +++ b/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 @@ -549,16 +560,16 @@ function create_args_vector!(kw, arg; wrap_byrow::Bool=false) if arg isa Expr && arg.head == :block && !outer_flags[ASTABLE_SYM][] x = MacroTools.rmlines(arg).args transforms = [] - seen_kw = !isempty(kw) + seen_kw_macro = false for xi in x if is_macro_head(xi, "@kwarg") - if seen_kw - throw(ArgumentError("@kwarg calls must be at end of block")) - end kw_item = get_kw_from_macro_call(xi) push!(kw, kw_item) - seen_kw = true + seen_kw_macro = true else + if seen_kw_macro + throw(ArgumentError("@kwarg calls must be at end of block")) + end push!(transforms, xi) end end diff --git a/test/keyword_arguments.jl b/test/keyword_arguments.jl index 2d7dd14..0c3116e 100644 --- a/test/keyword_arguments.jl +++ b/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 \ No newline at end of file From bbe4ec278a5bed5ee988971880e6fac5853bf603 Mon Sep 17 00:00:00 2001 From: Peter Deffebach Date: Tue, 16 Jul 2024 14:36:54 -0400 Subject: [PATCH 3/3] add news and modify project.toml --- NEWS.md | 3 +++ Project.toml | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 2bec205..e13c0fb 100644 --- a/NEWS.md +++ b/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)) diff --git a/Project.toml b/Project.toml index 1899e45..4ffdf03 100644 --- a/Project.toml +++ b/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"