Change view to viewhint in Tables.subset (#304)

* Change view to viewhint in Tables.subset

* fix

* fix

Author: quinnj

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,35 +584,39 @@ 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)
         cols = columns(x)
         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
     # otherwise, let's get the rows and see if we can apply inds to them
     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

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

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
 

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