Make modelcols(::Term, d) an error (#137)

* reproducer for #136

* throw argument error for modelcols on Term

* (broken) test for lack of fallback

* fallback guard in modelcols(::Any, ::Any)

* bump version (patch)

* add comment in source about fallback error

Project.toml

@@ -1,6 +1,6 @@
 name = "StatsModels"
 uuid = "3eaba693-59b7-5ba5-a881-562e759f1c8d"
-version = "0.6.2"
+version = "0.6.3"
 
 [deps]
 CategoricalArrays = "324d7699-5711-5eae-9e2f-1d82baa6b597"

src/terms.jl

@@ -419,6 +419,13 @@ converted to a `NamedTuple` of `Vectors` (e.g., a `Tables.ColumnTable`).
 """
 function modelcols(t, d::D) where D
     Tables.istable(d) || throw(ArgumentError("Data of type $D is not a table!"))
+    ## throw an error for t which don't have a more specific modelcols method defined
+    ## TODO: this seems like it ought to be handled by dispatching on something
+    ## like modelcols(::Any, ::NamedTuple) or modelcols(::AbstractTerm, ::NamedTuple)
+    ## but that causes ambiguity errors or under-constrained modelcols methods for
+    ## custom term types...
+    d isa NamedTuple && throw(ArgumentError("don't know how to generate modelcols for " *
+                                            "term $t. Did you forget to call apply_schema?"))
     modelcols(t, columntable(d))
 end
 
@@ -468,6 +475,10 @@ julia> modelcols(MatrixTerm(ts), d)
 """
 modelcols(ts::TupleTerm, d::NamedTuple) = modelcols.(ts, Ref(d))
 
+modelcols(t::Term, d::NamedTuple) =
+    throw(ArgumentError("can't generate modelcols for un-typed term $t. " *
+                        "Use apply_schema to create concrete terms first"))
+
 # TODO: @generated to unroll the getfield stuff
 modelcols(ft::FunctionTerm{Fo,Fa,Names}, d::NamedTuple) where {Fo,Fa,Names} =
     ft.fanon.(getfield.(Ref(d), Names)...)

test/extension.jl

@@ -24,6 +24,9 @@ StatsModels.apply_schema(t::NonMatrixTerm, sch, Mod::Type) =
     NonMatrixTerm(apply_schema(t.term, sch, Mod))
 StatsModels.modelcols(t::NonMatrixTerm, d) = modelcols(t.term, d)
 
+struct DummyTerm <: AbstractTerm
+end
+
 @testset "Extended formula/models" begin
     d = (z = rand(10), y = rand(10), x = collect(1:10))
     sch = schema(d)
@@ -87,5 +90,7 @@ StatsModels.modelcols(t::NonMatrixTerm, d) = modelcols(t.term, d)
 
     end
 
+    @testset "Fallback" begin
+        @test_throws ArgumentError modelcols(DummyTerm(), (a=[1], ))
+    end
 end
-

test/modelmatrix.jl

@@ -374,4 +374,10 @@
         end
     end
 
+    @testset "#136" begin
+        t = (x = rand(100), y = randn(100));
+        f = @formula(y ~ x)
+        @test_throws ArgumentError modelcols(f, t)
+    end
+
 end