Repository: JuliaData/Tables.jl
Pull request #319 (2022-11-29 06:35:13)

If AbstractMatrix defines Tables APIs, make fallbacks work
Alternative fix for #316.

codecov[bot]: # [Codecov](https://codecov.io/gh/JuliaData/Tables.jl/pull/319?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData) Report
Base: **94.32**% // Head: **94.33**% // Increases project coverage by **`+0.01%`** :tada:
> Coverage data is based on head [(`446862d`)](https://codecov.io/gh/JuliaData/Tables.jl/pull/319?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData) compared to base [(`db5e090`)](https://codecov.io/gh/JuliaData/Tables.jl/commit/db5e0903cf40a0803ec23ebd92753b70feeab169?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData).
> Patch coverage: 100.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #319      +/-   ##
==========================================
+ Coverage   94.32%   94.33%   +0.01%     
==========================================
  Files           7        7              
  Lines         722      724       +2     
==========================================
+ Hits          681      683       +2     
  Misses         41       41              
```


| [Impacted Files](https://codecov.io/gh/JuliaData/Tables.jl/pull/319?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData) | Coverage Δ | |
|---|---|---|
| [src/fallbacks.jl](https://codecov.io/gh/JuliaData/Tables.jl/pull/319/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData#diff-c3JjL2ZhbGxiYWNrcy5qbA==) | `97.33% <100.00%> (+0.03%)` | :arrow_up: |
| [src/matrix.jl](https://codecov.io/gh/JuliaData/Tables.jl/pull/319/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData#diff-c3JjL21hdHJpeC5qbA==) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/JuliaData/Tables.jl/pull/319?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaData).


Patch:
diff --git a/src/fallbacks.jl b/src/fallbacks.jl
index 48ea6ad..a46978e 100644
--- a/src/fallbacks.jl
+++ b/src/fallbacks.jl
@@ -82,7 +82,10 @@ end
 # this is our generic Tables.rows fallback definition
 @noinline nodefault(T) = throw(ArgumentError("no default `Tables.rows` implementation for type: $T"))
 
-function rows(x::T) where {T}
+rows(x::T) where {T} = _rows(x)
+
+# split out so we can re-use it in the matrix fallback
+function _rows(x::T) where {T}
     isrowtable(x) && return x
     # because this method is being called, we know `x` didn't define it's own Tables.rows
     # first check if it supports column access, and if so, wrap it in a RowIterator
@@ -252,7 +255,9 @@ getcolumn(x::CopiedColumns, nm::Symbol) = getcolumn(source(x), nm)
 columnnames(x::CopiedColumns) = columnnames(source(x))
 
 # here's our generic fallback Tables.columns definition
-@inline function columns(x::T) where {T}
+@inline columns(x::T) where {T} = _columns(x)
+
+@inline function _columns(x::T) where {T}
     # because this method is being called, we know `x` didn't define it's own Tables.columns method
     # first check if it explicitly supports row access, and if so, build up the desired columns
     if rowaccess(x)
diff --git a/src/matrix.jl b/src/matrix.jl
index 679b43c..12a609b 100644
--- a/src/matrix.jl
+++ b/src/matrix.jl
@@ -1,7 +1,9 @@
 istable(::Type{<:AbstractMatrix}) = false
 
-rows(m::T) where {T <: AbstractMatrix} = throw(ArgumentError("a '$T' is not a table; see `?Tables.table` for ways to treat an AbstractVecOrMat as a table"))
-columns(m::T) where {T <: AbstractMatrix} = throw(ArgumentError("a '$T' is not a table; see `?Tables.table` for ways to treat an AbstractVecOrMat as a table"))
+# for AbstractMatrix types, we don't want to automatically treat them as tables
+# *unless* someone has defined their own w/ appropriate Tables definitions
+rows(m::T) where {T <: AbstractMatrix} = istable(m) ? _rows(m) : throw(ArgumentError("a '$T' is not a table; see `?Tables.table` for ways to treat an AbstractVecOrMat as a table"))
+columns(m::T) where {T <: AbstractMatrix} = istable(m) ? _columns(m) : throw(ArgumentError("a '$T' is not a table; see `?Tables.table` for ways to treat an AbstractVecOrMat as a table"))
 
 struct MatrixTable{T <: AbstractVecOrMat} <: AbstractColumns
     names::Vector{Symbol}


Test patch:
diff --git a/test/runtests.jl b/test/runtests.jl
index 84a87a7..25d03c8 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -246,6 +246,16 @@ Tables.getcolumn(x::MockTable, ::Symbol) = (1, 2, 3)
 Tables.getcolumn(x::MockTable, ::Int) = (1, 2, 3)
 Tables.schema(x::MockTable) = Tables.Schema((:a, :b, :c), NTuple{3, Int})
 
+struct TestMatrixTable <: AbstractMatrix{Int}
+end
+Tables.istable(::Type{TestMatrixTable}) = true
+Tables.columnaccess(::Type{TestMatrixTable}) = true
+Tables.columnnames(x::TestMatrixTable) = (:a, :b, :c)
+Tables.columns(x::TestMatrixTable) = x
+Tables.getcolumn(x::TestMatrixTable, ::Symbol) = (1, 2, 3)
+Tables.getcolumn(x::TestMatrixTable, ::Int) = (1, 2, 3)
+Tables.schema(x::TestMatrixTable) = Tables.Schema((:a, :b, :c), NTuple{3, Int})
+
 @testset "Matrix integration" begin
     rt = [(a=1, b=4.0, c="7"), (a=2, b=5.0, c="8"), (a=3, b=6.0, c="9")]
     nt = (a=[1,2,3], b=[4.0, 5.0, 6.0])
@@ -339,6 +349,16 @@ Tables.schema(x::MockTable) = Tables.Schema((:a, :b, :c), NTuple{3, Int})
     # For the case that `Tables.getcolumn` doesn't return an `AbstractVector`
     # e.g Tuple, see #263
     @test Tables.matrix(MockTable()) == repeat([1, 2, 3], 1, 3)
+
+    tbl = TestMatrixTable()
+    ctbl = columntable(tbl)
+    @test ctbl.a == [1, 2, 3]
+    @test ctbl.b == [1, 2, 3]
+    @test ctbl.c == [1, 2, 3]
+    rtbl = rowtable(tbl)
+    @test rtbl[1].a == 1
+    @test rtbl[1].b == 1
+    @test rtbl[1].c == 1
 end
 
 import Base: ==