From 6d0b6d33bf25cadf676273bcd03c63253dac82df Mon Sep 17 00:00:00 2001 From: Jacob Quinn Date: Sun, 25 Sep 2022 11:00:47 -0600 Subject: [PATCH] Change view to viewhint in Tables.subset (#304) * Change view to viewhint in Tables.subset * fix * fix --- src/Tables.jl | 24 ++++++++++++++---------- src/dicts.jl | 8 ++++++-- src/namedtuples.jl | 8 ++++++-- test/runtests.jl | 26 +++++++++++++------------- 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/src/Tables.jl b/src/Tables.jl index 1bfe17e..a5596c4 100644 --- a/src/Tables.jl +++ b/src/Tables.jl @@ -574,7 +574,7 @@ struct Partitioner{T} end """ - Tables.subset(x, inds; view=nothing) + Tables.subset(x, inds; viewhint=nothing) Return one or more rows from table `x` according to the position(s) specified by `inds`: @@ -584,19 +584,23 @@ Return one or more rows from table `x` according to the position(s) specified by If other types of `inds` are passed than specified above the behavior is undefined. -The `view` argument influences whether the returned object is a view of the original table +The `viewhint` argument tries to influence whether the returned object is a view of the original table or an independent copy: -- If `view=nothing` (the default) then the implementation for a specific table type +- If `viewhint=nothing` (the default) then the implementation for a specific table type is free to decide whether to return a copy or a view. -- If `view=true` then a view is returned and if `view=false` a copy is returned. +- If `viewhint=true` then a view is returned and if `viewhint=false` a copy is returned. This applies both to returning a row or a table. -Any specialized implementation of `subset` must support the `view=nothing` argument. -Support for `view=true` or `view=false` is optional -(i.e. implementations may ignore the keyword argument and return a view or a copy regardless of `view` value). +Any specialized implementation of `subset` must support the `viewhint=nothing` argument. +Support for `viewhint=true` or `viewhint=false` is optional +(i.e. implementations may ignore the keyword argument and return a view or a copy regardless of `viewhint` value). """ -function subset(x::T, inds; view::Union{Bool, Nothing}=nothing) where {T} +function subset(x::T, inds; viewhint::Union{Bool, Nothing}=nothing, view::Union{Bool, Nothing}=nothing) where {T} + if view !== nothing + @warn "`view` keyword argument is deprecated for `Tables.subset`, use `viewhint` instead" + viewhint = view + end # because this method is being called, we know `x` didn't define it's own Tables.subset # first check if it supports column access, and if so, apply inds and wrap columns in a DictColumnTable if columnaccess(x) @@ -604,7 +608,7 @@ function subset(x::T, inds; view::Union{Bool, Nothing}=nothing) where {T} if inds isa Integer return ColumnsRow(cols, inds) else - ret = view === true ? _map(c -> Base.view(c, inds), cols) : _map(c -> c[inds], cols) + ret = viewhint === true ? _map(c -> Base.view(c, inds), cols) : _map(c -> c[inds], cols) return DictColumnTable(schema(cols), ret) end end @@ -612,7 +616,7 @@ function subset(x::T, inds; view::Union{Bool, Nothing}=nothing) where {T} r = rows(x) if r isa AbstractVector inds isa Integer && return r[inds] - ret = view === true ? Base.view(x, inds) : x[inds] + ret = viewhint === true ? Base.view(x, inds) : x[inds] (ret isa AbstractVector) || throw(ArgumentError("`Tables.subset`: invalid `inds` argument, expected `AbstractVector` output, got $(typeof(ret))")) return ret end diff --git a/src/dicts.jl b/src/dicts.jl index ee5e6db..5b38514 100644 --- a/src/dicts.jl +++ b/src/dicts.jl @@ -124,8 +124,12 @@ function Base.iterate(x::DictRowTable, st=1) return DictRow(x.names, x.values[st]), st + 1 end -function subset(x::DictRowTable, inds; view::Union{Bool,Nothing} = nothing) - values = view === true ? Base.view(getfield(x, :values), inds) : getfield(x, :values)[inds] +function subset(x::DictRowTable, inds; viewhint::Union{Bool,Nothing}=nothing, view::Union{Bool,Nothing}=nothing) + if view !== nothing + @warn "`view` keyword argument is deprecated for `Tables.subset`, use `viewhint` instead" + viewhint = view + end + values = viewhint === true ? Base.view(getfield(x, :values), inds) : getfield(x, :values)[inds] if inds isa Integer return DictRow(getfield(x, :names), values) else diff --git a/src/namedtuples.jl b/src/namedtuples.jl index 2245ccc..49e3ea7 100644 --- a/src/namedtuples.jl +++ b/src/namedtuples.jl @@ -110,11 +110,15 @@ end const ColumnTable = NamedTuple{names, T} where {names, T <: NTuple{N, AbstractVector}} where {N} rowcount(c::ColumnTable) = length(c) == 0 ? 0 : length(c[1]) -function subset(x::ColumnTable, inds; view::Union{Bool,Nothing}=nothing) +function subset(x::ColumnTable, inds; viewhint::Union{Bool,Nothing}=nothing, view::Union{Bool,Nothing}=nothing) + if view !== nothing + @warn "`view` keyword argument is deprecated for `Tables.subset`, use `viewhint` instead" + viewhint = view + end if inds isa Integer return map(c -> c[inds], x) else - return view === true ? map(c -> vectorcheck(Base.view(c, inds)), x) : map(c -> vectorcheck(c[inds]), x) + return viewhint === true ? map(c -> vectorcheck(Base.view(c, inds)), x) : map(c -> vectorcheck(c[inds]), x) end end diff --git a/test/runtests.jl b/test/runtests.jl index 317af1e..25206bb 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -147,30 +147,30 @@ end @testset "columntable subset" begin @test Tables.subset(nt, 1) == (a=1, b=4.0, c="7") - @test Tables.subset(nt, 1, view=false) == (a=1, b=4.0, c="7") - @test Tables.subset(nt, 1, view=nothing) == (a=1, b=4.0, c="7") + @test Tables.subset(nt, 1, viewhint=false) == (a=1, b=4.0, c="7") + @test Tables.subset(nt, 1, viewhint=nothing) == (a=1, b=4.0, c="7") @test Tables.subset(nt, 1:2) == (a=[1,2], b=[4.0, 5.0], c=["7","8"]) - @test Tables.subset(nt, 1:2, view=false) == (a=[1,2], b=[4.0, 5.0], c=["7","8"]) - @test Tables.subset(nt, 1:2, view=nothing) == (a=[1,2], b=[4.0, 5.0], c=["7","8"]) + @test Tables.subset(nt, 1:2, viewhint=false) == (a=[1,2], b=[4.0, 5.0], c=["7","8"]) + @test Tables.subset(nt, 1:2, viewhint=nothing) == (a=[1,2], b=[4.0, 5.0], c=["7","8"]) @test_throws ArgumentError Tables.subset(nt, [1:2 1:2]) - @test Tables.subset(nt, 1, view=true) == (a=1, b=4.0, c="7") - rs = Tables.subset(nt, 1:2, view=true) + @test Tables.subset(nt, 1, viewhint=true) == (a=1, b=4.0, c="7") + rs = Tables.subset(nt, 1:2, viewhint=true) @test rs == (a=[1,2], b=[4.0, 5.0], c=["7","8"]) @test rs.a.parent === nt.a end @testset "rowtable subset" begin @test Tables.subset(rt, 1) == (a=1, b=4.0, c="7") - @test Tables.subset(rt, 1, view=false) == (a=1, b=4.0, c="7") - @test Tables.subset(rt, 1, view=nothing) == (a=1, b=4.0, c="7") + @test Tables.subset(rt, 1, viewhint=false) == (a=1, b=4.0, c="7") + @test Tables.subset(rt, 1, viewhint=nothing) == (a=1, b=4.0, c="7") @test Tables.subset(rt, 1:2) == [(a=1, b=4.0, c="7"), (a=2, b=5.0, c="8")] - @test Tables.subset(rt, 1:2, view=false) == [(a=1, b=4.0, c="7"), (a=2, b=5.0, c="8")] - @test Tables.subset(rt, 1:2, view=nothing) == [(a=1, b=4.0, c="7"), (a=2, b=5.0, c="8")] + @test Tables.subset(rt, 1:2, viewhint=false) == [(a=1, b=4.0, c="7"), (a=2, b=5.0, c="8")] + @test Tables.subset(rt, 1:2, viewhint=nothing) == [(a=1, b=4.0, c="7"), (a=2, b=5.0, c="8")] @test_throws ArgumentError Tables.subset(rt, [1:2 1:2]) - @test Tables.subset(rt, 1, view=true) == (a=1, b=4.0, c="7") - rs = Tables.subset(rt, 1:2, view=true) + @test Tables.subset(rt, 1, viewhint=true) == (a=1, b=4.0, c="7") + rs = Tables.subset(rt, 1:2, viewhint=true) @test rs == [(a=1, b=4.0, c="7"), (a=2, b=5.0, c="8")] @test rs.parent === rt end @@ -794,7 +794,7 @@ end @test drow.a == 1 && drow.b == 2 && drow.c == 3 drows = Tables.subset(drt, [1, 2]) @test length(drows) == 2 - drowsv = Tables.subset(drt, [1, 2]; view=true) + drowsv = Tables.subset(drt, [1, 2]; viewhint=true) @test length(drowsv) == 2 end