From bb934575860d7d88d58f7cea5c983de67b5308d7 Mon Sep 17 00:00:00 2001 From: June Kim Date: Mon, 11 May 2026 21:49:08 -0700 Subject: [PATCH 1/4] Fix @subset to accept literal values (closes #259) Add support for literal boolean values (and other literals) in @subset by implementing a catch-all fun_to_vec method that wraps literals in Returns. This allows expressions like @subset(df, true) or @subset(df, false) to work correctly. The fix uses Returns to create a constant function from literal values, which is only allowed in no_dest contexts (@subset, @rsubset, @with). Tests added to verify: - @subset(df, true) returns all rows - @subset(df, false) returns empty dataframe - Literals combine correctly with other conditions --- src/parsing.jl | 16 ++++++++++++++++ test/subset.jl | 14 ++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/parsing.jl b/src/parsing.jl index 09d237e6..2941e3de 100644 --- a/src/parsing.jl +++ b/src/parsing.jl @@ -421,6 +421,22 @@ fun_to_vec(ex::QuoteNode; outer_flags::Union{NamedTuple, Nothing}=nothing, allow_multicol::Bool = false) = ex +# Catch-all method for literal values (Bool, Int, String, etc.) +# Wraps them in Returns to create a constant function +# This allows syntax like @subset(df, true) or @subset(df, false) +function fun_to_vec(ex; + no_dest::Bool=false, + gensym_names::Bool=false, + outer_flags::Union{NamedTuple, Nothing}=nothing, + allow_multicol::Bool = false) + if no_dest + # For @subset and @with, return a constant function + return :([] => Returns($ex)) + else + throw(ArgumentError("Literal values are only supported in @subset, @rsubset, @with, and similar macros")) + end +end + """ rename_kw_to_pair(ex::Expr) diff --git a/test/subset.jl b/test/subset.jl index 505f1f9b..e9ed7b87 100644 --- a/test/subset.jl +++ b/test/subset.jl @@ -173,4 +173,18 @@ end @test @subset!(groupby(copy(df), :g), :c .== :g) ≅ df[[], :] end +@testset "@subset with literal values" begin + df = DataFrame(A = [1, 2, 3], B = [4, 5, 6]) + + # Test with boolean literal true - should return all rows + @test @subset(df, true) ≅ df + + # Test with boolean literal false - should return empty dataframe + @test @subset(df, false) ≅ df[Int[], :] + + # Test combination with other conditions + @test @subset(df, true, :A .> 1) ≅ df[df.A .> 1, :] + @test @subset(df, false, :A .> 1) ≅ df[Int[], :] +end + end # module From 36b06d793464b4e87fe78bee1ba01a1da6b34bda Mon Sep 17 00:00:00 2001 From: June Kim Date: Mon, 11 May 2026 22:26:31 -0700 Subject: [PATCH 2/4] Fix literal subset to return AbstractVector via ByRow(Returns(...)) The original fix used `[] => Returns(true)` which returns a scalar Bool. DataFrames.subset wraps non-ByRow functions with assert_bool_vec, which throws ArgumentError when the function returns a non-vector value. Fix: use `[] => ByRow(Returns(ex))` instead. ByRow bypasses the assert_bool_vec check (assert_bool_vec(::ByRow) = identity) and DataFrames' _empty_selector_helper calls the inner function once per row, producing a proper boolean vector. Also use $(Base.Returns) interpolation to avoid depending on caller scope resolution. Adds tests for @subset!, grouped DataFrames with literals. --- src/parsing.jl | 8 +++++--- test/subset.jl | 9 +++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/parsing.jl b/src/parsing.jl index 2941e3de..c19e733d 100644 --- a/src/parsing.jl +++ b/src/parsing.jl @@ -422,7 +422,10 @@ fun_to_vec(ex::QuoteNode; allow_multicol::Bool = false) = ex # Catch-all method for literal values (Bool, Int, String, etc.) -# Wraps them in Returns to create a constant function +# Wraps them in ByRow(Returns(...)) to create a constant vector function. +# ByRow is required because DataFrames.subset enforces that transformation +# functions return an AbstractVector, and ByRow with empty source columns +# produces a vector via _empty_selector_helper. # This allows syntax like @subset(df, true) or @subset(df, false) function fun_to_vec(ex; no_dest::Bool=false, @@ -430,8 +433,7 @@ function fun_to_vec(ex; outer_flags::Union{NamedTuple, Nothing}=nothing, allow_multicol::Bool = false) if no_dest - # For @subset and @with, return a constant function - return :([] => Returns($ex)) + return :([] => $ByRow($(Base.Returns)($ex))) else throw(ArgumentError("Literal values are only supported in @subset, @rsubset, @with, and similar macros")) end diff --git a/test/subset.jl b/test/subset.jl index e9ed7b87..d1375ea4 100644 --- a/test/subset.jl +++ b/test/subset.jl @@ -185,6 +185,15 @@ end # Test combination with other conditions @test @subset(df, true, :A .> 1) ≅ df[df.A .> 1, :] @test @subset(df, false, :A .> 1) ≅ df[Int[], :] + + # @subset! with literal values + @test @subset!(copy(df), true) ≅ df + @test @subset!(copy(df), false) ≅ df[Int[], :] + + # @subset with grouped data frame and literal + gd = groupby(df, :B) + @test @subset(gd, true) ≅ df + @test @subset(gd, false) ≅ df[Int[], :] end end # module From dae8df6889845f9df82ec49b9ca62dc8834f7429 Mon Sep 17 00:00:00 2001 From: June Kim Date: Fri, 22 May 2026 14:52:09 -0700 Subject: [PATCH 3/4] Raise a clear error for literal subset predicates (#259) A bare literal like `@subset(df, true)` lowers to a scalar predicate, which `DataFrames.subset` rejects exactly as `subset(df, [] => Returns(true))` does. Accepting it (via `ByRow(Returns(...))`) made the macro diverge from the function and would mean different things across macros (`@select(df, 1)` vs `@subset(df, true)`). Replace the prior MethodError from #259 with a clear ArgumentError pointing at the column-expression form. Tests assert the expansion-time error. --- src/parsing.jl | 21 ++++++++++----------- test/subset.jl | 30 +++++++++++------------------- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/src/parsing.jl b/src/parsing.jl index c19e733d..02b07a6a 100644 --- a/src/parsing.jl +++ b/src/parsing.jl @@ -421,22 +421,21 @@ fun_to_vec(ex::QuoteNode; outer_flags::Union{NamedTuple, Nothing}=nothing, allow_multicol::Bool = false) = ex -# Catch-all method for literal values (Bool, Int, String, etc.) -# Wraps them in ByRow(Returns(...)) to create a constant vector function. -# ByRow is required because DataFrames.subset enforces that transformation -# functions return an AbstractVector, and ByRow with empty source columns -# produces a vector via _empty_selector_helper. -# This allows syntax like @subset(df, true) or @subset(df, false) +# Catch-all method for literal values (Bool, Int, String, etc.). +# A bare literal isn't a column expression: `@subset(df, true)` lowers to a +# scalar predicate, which `DataFrames.subset` rejects exactly as +# `subset(df, [] => Returns(true))` does. Rather than accept it (which would +# make the macro diverge from the function, and would mean different things +# across macros — e.g. `@select(df, 1)` vs `@subset(df, true)`), surface a +# clear error in place of the bare MethodError from #259. function fun_to_vec(ex; no_dest::Bool=false, gensym_names::Bool=false, outer_flags::Union{NamedTuple, Nothing}=nothing, allow_multicol::Bool = false) - if no_dest - return :([] => $ByRow($(Base.Returns)($ex))) - else - throw(ArgumentError("Literal values are only supported in @subset, @rsubset, @with, and similar macros")) - end + throw(ArgumentError( + "literal value `$ex` is not a valid column expression; pass a " * + "column reference or a vectorised predicate (e.g. `:x .> 0`)")) end diff --git a/test/subset.jl b/test/subset.jl index d1375ea4..be577451 100644 --- a/test/subset.jl +++ b/test/subset.jl @@ -173,27 +173,19 @@ end @test @subset!(groupby(copy(df), :g), :c .== :g) ≅ df[[], :] end -@testset "@subset with literal values" begin +@testset "@subset with literal values errors clearly" begin df = DataFrame(A = [1, 2, 3], B = [4, 5, 6]) - # Test with boolean literal true - should return all rows - @test @subset(df, true) ≅ df - - # Test with boolean literal false - should return empty dataframe - @test @subset(df, false) ≅ df[Int[], :] - - # Test combination with other conditions - @test @subset(df, true, :A .> 1) ≅ df[df.A .> 1, :] - @test @subset(df, false, :A .> 1) ≅ df[Int[], :] - - # @subset! with literal values - @test @subset!(copy(df), true) ≅ df - @test @subset!(copy(df), false) ≅ df[Int[], :] - - # @subset with grouped data frame and literal - gd = groupby(df, :B) - @test @subset(gd, true) ≅ df - @test @subset(gd, false) ≅ df[Int[], :] + # A bare literal is not a valid predicate — it must error (consistent + # with `subset(df, [] => Returns(true))`), not be silently broadcast. + # The error is raised during macro expansion (as the prior MethodError + # from #259 was), so it surfaces via @eval as a LoadError — matching + # the suite's convention for other invalid-expansion cases. + @test_throws LoadError @eval @subset($df, true) + @test_throws LoadError @eval @subset($df, false) + @test_throws LoadError @eval @subset($df, true, :A .> 1) + @test_throws LoadError @eval @subset!(copy($df), true) + @test_throws LoadError @eval @subset(groupby($df, :B), true) end end # module From e23e7d4fe31918725b9152b3a262f7ff6ecbb3a1 Mon Sep 17 00:00:00 2001 From: June Kim Date: Fri, 22 May 2026 15:36:06 -0700 Subject: [PATCH 4/4] Make literal handling byrow-aware (#259) ByRow only applies under the @byrow flag, so behaviour mirrors the underlying subset in both modes: @subset(df, true) -> errors, like subset(df, [] => Returns(true)) @rsubset(df, true) -> all rows, like subset(df, [] => ByRow(Returns(true))) Applying ByRow unconditionally (prior approach) made @subset accept what the function rejects. Tests now cover both modes; master had none for literal predicates either way. --- src/parsing.jl | 22 +++++++++++++++------- test/subset.jl | 17 +++++++++++------ 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/parsing.jl b/src/parsing.jl index 02b07a6a..68a6f68f 100644 --- a/src/parsing.jl +++ b/src/parsing.jl @@ -422,20 +422,28 @@ fun_to_vec(ex::QuoteNode; allow_multicol::Bool = false) = ex # Catch-all method for literal values (Bool, Int, String, etc.). -# A bare literal isn't a column expression: `@subset(df, true)` lowers to a -# scalar predicate, which `DataFrames.subset` rejects exactly as -# `subset(df, [] => Returns(true))` does. Rather than accept it (which would -# make the macro diverge from the function, and would mean different things -# across macros — e.g. `@select(df, 1)` vs `@subset(df, true)`), surface a -# clear error in place of the bare MethodError from #259. +# Behaviour mirrors the underlying `DataFrames.subset` exactly, keyed on the +# `@byrow` flag: +# * byrow -> `[] => ByRow(Returns(ex))`, a per-row constant vector, same +# as `subset(df, [] => ByRow(Returns(true)))` (which works). +# * !byrow -> a bare literal is a scalar predicate, which `subset` rejects +# (`subset(df, [] => Returns(true))` errors). Raise a clear +# ArgumentError in place of the #259 MethodError. +# ByRow is applied only when byrow is set; applying it unconditionally would +# make `@subset(df, true)` accept what the function rejects. function fun_to_vec(ex; no_dest::Bool=false, gensym_names::Bool=false, outer_flags::Union{NamedTuple, Nothing}=nothing, allow_multicol::Bool = false) + byrow = outer_flags !== nothing && outer_flags[BYROW_SYM][] + if byrow + return :([] => $ByRow($(Base.Returns)($ex))) + end throw(ArgumentError( "literal value `$ex` is not a valid column expression; pass a " * - "column reference or a vectorised predicate (e.g. `:x .> 0`)")) + "column reference or a vectorised predicate (e.g. `:x .> 0`), " * + "or use `@rsubset` for a per-row literal")) end diff --git a/test/subset.jl b/test/subset.jl index be577451..05f2067d 100644 --- a/test/subset.jl +++ b/test/subset.jl @@ -173,19 +173,24 @@ end @test @subset!(groupby(copy(df), :g), :c .== :g) ≅ df[[], :] end -@testset "@subset with literal values errors clearly" begin +@testset "@subset / @rsubset with literal values" begin df = DataFrame(A = [1, 2, 3], B = [4, 5, 6]) - # A bare literal is not a valid predicate — it must error (consistent - # with `subset(df, [] => Returns(true))`), not be silently broadcast. - # The error is raised during macro expansion (as the prior MethodError - # from #259 was), so it surfaces via @eval as a LoadError — matching - # the suite's convention for other invalid-expansion cases. + # @subset (not byrow): a bare literal is a scalar predicate, which + # subset rejects (subset(df, [] => Returns(true)) errors). It must error, + # not be silently broadcast. The error is raised during macro expansion + # (as the #259 MethodError was), so it surfaces via @eval as LoadError. @test_throws LoadError @eval @subset($df, true) @test_throws LoadError @eval @subset($df, false) @test_throws LoadError @eval @subset($df, true, :A .> 1) @test_throws LoadError @eval @subset!(copy($df), true) @test_throws LoadError @eval @subset(groupby($df, :B), true) + + # @rsubset (byrow): a per-row literal is well-defined and matches + # subset(df, [] => ByRow(Returns(true))), which works. + @test @rsubset(df, true) ≅ df + @test @rsubset(df, false) ≅ df[Int[], :] + @test @rsubset(groupby(df, :B), true) ≅ df end end # module