[Rd] model.matrix.default() silently ignores bad contrasts.arg

[Fox, John]

Dear Martin and Ben,

I agree that a warning is a good idea (and perhaps that wasn't clear in my response to Ben's post). 

Also, it would be nice to correct the omission in the help file, which as far as I could see doesn't mention that a contrast-generating function (as opposed to its quoted name) can be an element of the contrasts.arg list.

Best,
 John

> -----Original Message-----
> From: Martin Maechler [mailto:maechler using stat.math.ethz.ch]
> Sent: Friday, February 22, 2019 11:50 AM
> To: Ben Bolker <bbolker using gmail.com>
> Cc: Fox, John <jfox using mcmaster.ca>; r-devel using r-project.org
> Subject: Re: [Rd] model.matrix.default() silently ignores bad contrasts.arg
> 
> >>>>> Ben Bolker
> >>>>>     on Thu, 21 Feb 2019 08:18:51 -0500 writes:
> 
>     > On Thu, Feb 21, 2019 at 7:49 AM Fox, John <jfox using mcmaster.ca> wrote:
>     >>
>     >> Dear Ben,
>     >>
>     >> Perhaps I'm missing the point, but contrasts.arg is documented to be a
> list. From ?model.matrix: "contrasts.arg: A list, whose entries are values
> (numeric matrices or character strings naming functions) to be used as
> replacement values for the contrasts replacement function and whose
> names are the names of columns of data containing factors."
> 
>     > I absolutely agree that this is not a bug/behaves as documented (I
>     > could have said that more clearly).  It's just that (for reasons I
>     > attempted to explain) this is a really easy mistake to make.
> 
>     >> This isn't entirely accurate because a function also works as a named
> element of the list (in addition to a character string naming a function and a
> contrast matrix), as your example demonstrates, but nowhere that I'm
> aware of is it suggested that a non-list should work.
>     >>
>     >> It certainly would be an improvement if specifying contrast.arg as a non-
> list generated an error or warning message, and it at least arguably would be
> convenient to allow a general contrast specification such as contrasts.arg-
> "contr.sum", but I don't see a bug here.
> 
>     > I agree.  That's what my patch does (throws a warning message if
>     > contrasts.arg is non-NULL and not a list).
> 
> I currently do think this is a good idea... "even though" I'm 99% sure that this
> will make work for package maintainers and others whose code may
> suddenly show warnings.
> I hope they would know better than suppressWarnings(.) ...
> 
> I see a version of the patch using old style indentation which makes the diff
> even "considerably" smaller -- no need to submit this different, though --
> and I plan to test that a bit, and commit eventually to R-devel, possibly in a 5
> days or so.
> 
> Thank you Ben for the suggestion and patch !
> Martin
> 
>     > cheers
>     > Ben Bolker
> 
>     >> Best,
>     >> John
>     >>
>     >> -------------------------------------------------
>     >> John Fox, Professor Emeritus
>     >> McMaster University
>     >> Hamilton, Ontario, Canada
>     >> Web: http::/socserv.mcmaster.ca/jfox
>     >>
>     >> > On Feb 20, 2019, at 7:14 PM, Ben Bolker <bbolker using gmail.com> wrote:
>     >> >
>     >> > An lme4 user pointed out
> <https://github.com/lme4/lme4/issues/491> that
>     >> > passing contrasts as a string or symbol to [g]lmer (which would work if
>     >> > we were using `contrasts<-` to set contrasts on a factor variable) is
>     >> > *silently ignored*. This goes back to model.matrix(), and seems bad
>     >> > (this is a very easy mistake to make, because of the multitude of ways
>     >> > to specify contrasts for factors in R  - e.g. options(contrasts=...);
>     >> > setting contrasts on the specific factors; passing contrasts as a list
>     >> > to the model function ... )
>     >> >
>     >> > The relevant code is here:
>     >> >
>     >> > https://github.com/wch/r-
> source/blob/trunk/src/library/stats/R/models.R#L578-L603
>     >> >
>     >> > The following code shows the problem: a plain-vanilla model.matrix()
>     >> > call with no contrasts argument, followed by two wrong contrasts
>     >> > arguments, followed by a correct contrasts argument.
>     >> >
>     >> > data(cbpp, package="lme4")
>     >> > mf1 <- model.matrix(~period, data=cbpp)
>     >> > mf2 <- model.matrix(~period, contrasts.arg="contr.sum", data=cbpp)
>     >> > all.equal(mf1,mf2) ## TRUE
>     >> > mf3 <- model.matrix(~period, contrasts.arg=contr.sum, data=cbpp)
>     >> > all.equal(mf1,mf3)  ## TRUE
>     >> > mf4 <- model.matrix(~period, contrasts.arg=list(period=contr.sum),
>     >> > data=cbpp)
>     >> > isTRUE(all.equal(mf1,mf4))  ## FALSE
>     >> >
>     >> >
>     >> >  I've attached a potential patch for this, which is IMO the mildest
>     >> > possible case (if contrasts.arg is non-NULL and not a list, it produces
>     >> > a warning).  I haven't been able to test it because of some mysterious
>     >> > issues I'm having with re-making R properly ...
>     >> >
>     >> >  Thoughts?  Should I submit this as a bug report/patch?
>     >> >
>     >> >  cheers
>     >> >   Ben Bolker
>     >>
>     >> >
> <models.R.diff>______________________________________________