>>>>> Fox, John >>>>> on Fri, 22 Feb 2019 17:40:15 +0000 writes:
> 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 Thank you John for the clarification and the reminder about filling the omission there! Prepared to go (into the sources) now. Martin >> -----Original Message----- From: Martin Maechler >> [mailto:maech...@stat.math.ethz.ch] Sent: Friday, >> February 22, 2019 11:50 AM To: Ben Bolker >> <bbol...@gmail.com> Cc: Fox, John <j...@mcmaster.ca>; >> r-devel@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 >> <j...@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 >> <bbol...@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>______________________________________________ ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel