thanks!
On 2019-02-23 5:42 a.m., Martin Maechler wrote: >>>>>> 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
