Deprecate un-used base= kwarg for SeqDiffCoding (#238)

* remove base field from seqdiffcoding and depwarn when base= kwarg

* typo

* suggestion from code review: clearer deprecation warning

* also error message in test

Author: kleinschmidt

src/contrasts.jl

@@ -234,7 +234,7 @@ Base.getindex(contrasts::ContrastsMatrix{C,T}, rowinds, colinds) where {C,T} =
 # Making a contrast type T only requires that there be a method for
 # contrasts_matrix(T,  baseind, n) and optionally termnames(T, levels, baseind)
 # The rest is boilerplate.
-for contrastType in [:DummyCoding, :EffectsCoding, :HelmertCoding, :SeqDiffCoding]
+for contrastType in [:DummyCoding, :EffectsCoding, :HelmertCoding]
     @eval begin
         mutable struct $contrastType <: AbstractContrasts
             base::Any
@@ -448,7 +448,22 @@ julia> StatsModels.hypothesis_matrix(seqdiff)
 """
 SeqDiffCoding
 
-function contrasts_matrix(C::SeqDiffCoding, baseind, n)
+mutable struct SeqDiffCoding <: AbstractContrasts
+    levels::Union{AbstractVector,Nothing}
+end
+function SeqDiffCoding(; base=nothing, levels::Union{AbstractVector,Nothing}=nothing)
+    if base !== nothing
+        Base.depwarn("`base=` kwarg for `SeqDiffCoding` has no effect and is deprecated. " *
+                     "Specify full order of levels using `levels=` instead",
+                     :SeqDiffCoding)
+    end
+    return SeqDiffCoding(levels)
+end
+
+baselevel(c::SeqDiffCoding) = c.levels === nothing ? nothing : c.levels[1]
+DataAPI.levels(c::SeqDiffCoding) = c.levels
+
+function contrasts_matrix(C::SeqDiffCoding, _, n)
     mat = zeros(n, n-1)
     for col in 1:n-1
         mat[1:col, col] .= col-n

test/contrasts.jl

@@ -289,8 +289,8 @@
         using StatsModels: baselevel, FullDummyCoding, ContrastsCoding
 
         levs = [:a, :b, :c, :d]
-        base = [:a]
-        for C in [DummyCoding, EffectsCoding, SeqDiffCoding, HelmertCoding]
+        base = [:c]
+        for C in [DummyCoding, EffectsCoding, HelmertCoding]
             c = C()
             @test levels(c) == nothing
             @test baselevel(c) == nothing
@@ -308,6 +308,25 @@
             @test baselevel(c) == base
         end
 
+        c = SeqDiffCoding()
+        @test baselevel(c) == nothing
+        @test levels(c) == nothing
+
+        c = SeqDiffCoding(levels=levs)
+        @test baselevel(c) == levs[1]
+        @test levels(c) == levs
+
+        c = @test_logs((:warn,
+                        "`base=` kwarg for `SeqDiffCoding` has no effect and is deprecated. " *
+                        "Specify full order of levels using `levels=` instead"),
+                       SeqDiffCoding(base=base))
+        @test baselevel(c) == nothing
+        @test levels(c) == nothing
+
+        c = SeqDiffCoding(base=base, levels=levs)
+        @test baselevel(c) == levs[1]
+        @test levels(c) == levs
+
         c = FullDummyCoding()
         @test baselevel(c) == nothing
         @test levels(c) == nothing