[Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages

[Michael Lawrence]

That's right.

On Fri, Jan 23, 2015 at 12:22 PM, Mike <wjiang2 at fhcrc.org> wrote:
> Sorry, I just want to clarify some more on this.(maybe useful for others as
> well)
> What you actually mean is callNextMethod() used to drop both ... and the
> other arguments explicitly supplied from the parent call (in my case,
> parameters argument).
> And now after your fix, both gets passed on and that’s why I should
> explicitly select the argument for callNextMethod?
>
> Thanks.
>
> Mike
>
> On 01/23/2015 11:30 AM, Michael Lawrence wrote:
>
> The bug has existed forever. The commit log may be confusing. The
> actual bug (or at least a very undesirable behavior) was in
> match.call(), not callNextMethod().
>
> I think it's still true that callNextMethod() is the natural
> invocation. When adding arguments to initialize that you do not want
> to pass on (and thus set as slots), it's necessary to use explicit
> args. There are other cases where callNextMethod() is exactly what you
> want. Like the case in rtracklayer that motivated this fix.
>
>
> On Fri, Jan 23, 2015 at 11:23 AM, Mike <wjiang2 at fhcrc.org> wrote:
>
> Michael,
>
> Thanks for the confirmation of the issue. I see you were trying to tackle it
> in the latest commits r67467:67472 which apparently haven’t fixed the bug
> yet (instead it triggers the error of the existing legacy code in other R
> packages like flowCore). I can certainly change the code to explicitly pass
> on all the arguments to callNextMethod as you and Martin suggested. I just
> wonder if the documentation should drop the sentence of Calling with no
> arguments is often the natural way to use callNextMethod and change the
> example code (at least before the bug is eventually fixed.) so that users
> won’t be misusing it.
>
>
>
> Mike
>
> On 01/23/2015 10:55 AM, Martin Morgan wrote:
>
> On 01/23/2015 10:52 AM, Michael Lawrence wrote:
>
> First let me apologize for my failure to read emails. There was a
> long-standing bug in the methods package where arguments passed as
> "..." to a method would be dropped by callNextMethod(). It turns out
> that in all known cases this affects calls to initialize(), probably
> because there are many initialize() methods, and new() calls
> initialize with "...".
>
> This case is a very typical one, and Martin's fix is the right one,
> according to the (unchanged) documentation of callNextMethod().
>
> It's in general impossible to detect these cases from static analysis,
> since we do not know how the user is calling a method. But catching
> them in initialize() should be easy, with some false positives. Just
> look for callNextMethod().
>
> Is it OK for me to checkout all of Bioconductor so that I can perform
> that analysis, or will that stress the servers too much? I have a
> package that embeds GNU Global to index and search
> CRAN/Bioconductor-size repositories for these cases.
>
>
> Hi Michael -- there is no problem checking out all
> (https://hedgehog.fhcrc.org/bioconductor/trunk/madman/Rpacks presumably) of
> Bioc.
>
> Thanks! Martin
>
>
> Michael
>
> On Thu, Jan 22, 2015 at 6:15 AM, Martin Morgan <mtmorgan at fredhutch.org>
> wrote:
>
> On 01/22/2015 12:26 AM, Martin Maechler wrote:
>
>
> Mike  <wjiang2 at fhcrc.org>
>       on Tue, 20 Jan 2015 17:16:37 -0800 writes:
>
>
>
>       > I don't think it has been addressed yet in the later commits of
> R-devel.
>       > And that piece of code in flowCore package was written long time
> ago and
>       > there is nothing wrong with it as far as I can see.
>
> You are right.
>
> I thought Michael Lawrence (member of R Core since last summer!)
> was also reading Bioc-devel, so wonder why he has not yet
> replied --> CC'ing him
>
> The changes to R-devel also did break the Matrix package in
> exactly the same way.  You said
>
> Here is the |initialize|method for |parameterFilter| which causes the
> various errors from flow package vignettes.
>
> |setMethod("initialize",
>              signature=signature(.Object="parameterFilter"),
>              definition=function(.Object, parameters,...)
>              {
>                if  (!missing(parameters))
>                  parameters(.Object) <- parameters
>                callNextMethod()
>              })
> |
>
>
>
> and I also had  a  _no argument_ call
>         callNextMethod()
> inside an  initialize method.
>
> I'm pretty sure that if you change (the same as I)
>
>              callNextMethod()
> to
>              callNextMethod(.Object, ...)
>
> you'll have a version of the code that works both in current R 3.1.2
> (and older versions)  *and* in the R-devel version.
>
>
> I also pinged Michael??
>
> What's a precise statement of the problem -- no-argument callNextMethod()
> inside initialize? Any suggestions on ways to discover these without relying
> on package break during build / check / install?
>
> Martin Morgan
>
> Michael L and I were not sure how many other packages or R code this
> would influence and he was expecting very very few.
>
> Best regards,
>
> Martin Maechler, ETH Zurich
>
>
>       > On 01/20/2015 05:06 PM, Dan Tenenbaum wrote:
>       >> I'm not sure if you were implying that this has already been fixed
> in R-devel. Note that the devel build machines currently have r67501
> installed, which is later than all the commits you cite above, yet the flow
> packages are still broken.
>
> _______________________________________________
> Bioc-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/bioc-devel
>
>
>
> --
> Computational Biology / Fred Hutchinson Cancer Research Center
> 1100 Fairview Ave. N.
> PO Box 19024 Seattle, WA 98109
>
> Location: Arnold Building M1 B861
> Phone: (206) 667-2793
>
>
>
>