From bbo|ker @end|ng |rom gm@||@com  Sat Feb 23 15:33:22 2019
From: bbo|ker @end|ng |rom gm@||@com (Ben Bolker)
Date: Sat, 23 Feb 2019 09:33:22 -0500
Subject: [Rd] model.matrix.default() silently ignores bad contrasts.arg
In-Reply-To: <23665.9119.771386.205226@stat.math.ethz.ch>
References: <3553_1550708062_x1L0EMNQ028449_7bf65e6d-839d-9475-79fd-3a8ee341929a@gmail.com>
 <D3DC2A5A-7AE7-4EAE-B6EA-853C85972368@mcmaster.ca>
 <CABghstS8qm9hgbz6Laj-juTu9ra02jPZqhuU41_GkoKKJbzFYw@mail.gmail.com>
 <23664.10276.644277.668486@stat.math.ethz.ch>
 <ACD1644AA6C67E4FBD0C350625508EC836ACBF08@FHSDB2D11-2.csu.mcmaster.ca>
 <23665.9119.771386.205226@stat.math.ethz.ch>
Message-ID: <3472707f-123f-7c42-546d-f5f1596f613d@gmail.com>


 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:maechler at stat.math.ethz.ch] Sent: Friday,
>     >> February 22, 2019 11:50 AM To: Ben Bolker
>     >> <bbolker at gmail.com> Cc: Fox, John <jfox at mcmaster.ca>;
>     >> r-devel at 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 at 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 at 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>______________________________________________
>



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