fix select bug with copycols=false on SubDataFrame (#3231)

NEWS.md

@@ -1,3 +1,11 @@
+# DataFrames.jl v1.4.4 Patch Release Notes
+
+## Bug fixes
+
+* Fix bug in `select` and `transform` with `copycols=false` on `SubDataFrame`
+  that incorrectly allowed passing transformations
+  ([#3231](/~https://github.com/JuliaData/DataFrames.jl/pull/3231))
+
 # DataFrames.jl v1.4.3 Patch Release Notes
 
 ## Bug fixes

Project.toml

@@ -1,6 +1,6 @@
 name = "DataFrames"
 uuid = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
-version = "1.4.3"
+version = "1.4.4"
 
 [deps]
 Compat = "34da2185-b29b-5c13-b0c7-acf172513d20"

src/abstractdataframe/selection.jl

@@ -336,16 +336,7 @@ make_pair_concrete(@nospecialize(x::Pair)) =
     make_pair_concrete(x.first) => make_pair_concrete(x.second)
 make_pair_concrete(@nospecialize(x)) = x
 
-normalize_selection(idx::AbstractIndex, @nospecialize(sel), renamecols::Bool) =
-    try
-        idx[sel]
-    catch e
-        if e isa MethodError && e.f === getindex && e.args === (idx, sel)
-            throw(ArgumentError("Unrecognized column selector $sel in AsTable constructor"))
-        else
-            rethrow(e)
-        end
-    end
+normalize_selection(idx::AbstractIndex, @nospecialize(sel), renamecols::Bool) = idx[sel]
 
 normalize_selection(idx::AbstractIndex, @nospecialize(sel::Base.Callable), renamecols::Bool) = sel
 normalize_selection(idx::AbstractIndex, sel::Colon, renamecols::Bool) = idx[:]
@@ -1807,11 +1798,7 @@ function manipulate(dfv::SubDataFrame, @nospecialize(args...); copycols::Bool, k
                     push!(seen_single_column, ind_idx)
                 end
             else
-                newind = normalize_selection(index(dfv), make_pair_concrete(ind), renamecols)
-                if newind isa Pair
-                    throw(ArgumentError("transforming and renaming columns of a " *
-                                        "SubDataFrame is not allowed when `copycols=false`"))
-                end
+                newind = index(dfv)[ind]
                 push!(newinds, newind)
             end
         end

src/other/index.jl

@@ -182,6 +182,9 @@ end
 
 Base.insert!(x::Index, idx::Integer, nm::AbstractString) = insert!(x, idx, Symbol(nm))
 
+@inline Base.getindex(x::AbstractIndex, idx::T) where {T} =
+    throw(ArgumentError("invalid index: $idx of type $T"))
+
 @inline Base.getindex(x::AbstractIndex, idx::Bool) =
     throw(ArgumentError("invalid index: $idx of type Bool"))
 
@@ -240,7 +243,8 @@ end
     return getindex(x, Vector{Int}(idx))
 end
 
-@inline Base.getindex(x::AbstractIndex, idx::AbstractRange{Bool}) = getindex(x, collect(idx))
+@inline Base.getindex(x::AbstractIndex, idx::AbstractRange{Bool}) =
+    getindex(x, collect(idx))
 
 @inline function Base.getindex(x::AbstractIndex, idx::AbstractVector{Bool})
     length(x) == length(idx) || throw(BoundsError(x, idx))

test/index.jl

@@ -24,7 +24,7 @@ using DataFrames: Index, SubIndex, fuzzymatch
         end
     end
 
-    @test_throws MethodError i[1.0]
+    @test_throws ArgumentError i[1.0]
     @test_throws ArgumentError i[true]
     @test_throws ArgumentError i[false]
     @test_throws ArgumentError i[Union{Bool, Missing}[true, false]]
@@ -477,8 +477,8 @@ end
     @test df[:, Cols(x -> x[1] == 'a')] == df[:, [1, 2]]
     @test df[:, Cols(x -> x[end] == '1')] == df[:, [1, 3]]
     @test df[:, Cols(x -> x[end] == '3')] == DataFrame()
-    @test_throws MethodError df[:, Cols(x -> true, 1)] == DataFrame()
-    @test_throws MethodError df[:, Cols(1, x -> true)] == DataFrame()
+    @test_throws ArgumentError df[:, Cols(x -> true, 1)]
+    @test_throws ArgumentError df[:, Cols(1, x -> true)]
 end
 
 @testset "views" begin

test/select.jl

@@ -315,6 +315,8 @@ end
     @test_throws BoundsError select(df, 6)
     @test_throws ArgumentError select(df, [1, 1])
     @test_throws ArgumentError select(df, :f)
+    @test_throws ArgumentError select(df, 1.0)
+    @test_throws ArgumentError select(df, true)
     @test_throws BoundsError select!(df, [true, false])
 
     @test select(df, 1:0) == DataFrame()
@@ -876,6 +878,7 @@ end
     @test select(sdf, :x1, [:x1], copycols=false) isa SubDataFrame
     @test_throws ArgumentError select(sdf, :x1 => :r1, copycols=false)
     @test_throws ArgumentError select(sdf, :x1 => identity => :r1, copycols=false)
+    @test_throws ArgumentError select(sdf, identity, copycols=false)
 end
 
 @testset "pseudo-broadcasting" begin
@@ -2733,4 +2736,12 @@ end
                          e=[missing, 4, missing, 8])
 end
 
+@testset "selection on a view without copying" begin
+    df = DataFrame(a=1:2)
+    for dfv in (view(df, :, :), view(df, 1:2, 1:1))
+        @test_throws ArgumentError select(dfv, x -> true, copycols=false)
+        @test_throws ArgumentError select(dfv, :a => identity, copycols=false)
+    end
+end
+
 end # module