Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
Thanks, not much need for convincing here, just wanted a big enough push to get me over that 'kinda works so why bother` hurdle. I need to do other code refactoring in Gviz soon, and will take the opportunity to kick out all initialisers in the process. So one less package to worry about. Florian On 26/01/15 14:28, Michael Lawrence lawrence.mich...@gene.com wrote: In the S4Vectors/IRanges infrastructure, we have never defined an initialize method, so it is certainly possible to use the constructor pattern in complex frameworks, and IRanges serves as a good example of doing so. The basic pattern is to delegate to the superclass constructor and pass that object as an unnamed argument to new(). We stayed away from changing the formals of initialize, because (a) initialize() has a special contract of no-arg invocation that is a pain to support and may not be consistent with the user-facing interface and (b) more importantly, we wanted to preserve the ability (at the low level) to re-initialize based purely on slots. Once you have a custom initialize, it is no longer possible to call new() consistently (just with slots) across all classes. And (c), since in general we want constructors anyway (the user calling new() directly would sacrifice abstraction), this policy greatly reduced complexity by keeping the logic at a single layer. Others might have more to add; I stopped thinking after I got to three ;) Hope that helps, Michael On Mon, Jan 26, 2015 at 12:38 AM, Hahne, Florian florian.ha...@novartis.com wrote: Hi Michael, I'll take a look. In order to improve my code: what exactly do you think should be part of the initialiser, and what should be in the constructor? There don't seem to bee any clear guidelines out there anywhere. And if all logic goes in the constructor, how does one deal with more complex nested inheritance? And what's the use for the initialiser in the first place? Florian On 24/01/15 03:03, Michael Lawrence lawrence.mich...@gene.com wrote: I have found and fixed the affected initialize cases and will begin checking in fixes. Affected packages: RDAVIDWebService, flowCore (as we know), Gviz, ChIPseqR, Pviz, VanillaICE, flowFit. As an aside, in all of these cases, initialize is implementing logic that really belongs in a constructor function. Treating new() as a high-level constructor should be discouraged in my opinion. Has nothing to do with this bug though. I have also begun looking through cases outside of initialize. There are only about 300 cases of callNextMethod() in the codebase. Great majority seem OK so far. On Fri, Jan 23, 2015 at 12:46 PM, Michael Lawrence micha...@gene.com wrote: That's right. On Fri, Jan 23, 2015 at 12:22 PM, Mike wjia...@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 wjia...@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
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
In the S4Vectors/IRanges infrastructure, we have never defined an initialize method, so it is certainly possible to use the constructor pattern in complex frameworks, and IRanges serves as a good example of doing so. The basic pattern is to delegate to the superclass constructor and pass that object as an unnamed argument to new(). We stayed away from changing the formals of initialize, because (a) initialize() has a special contract of no-arg invocation that is a pain to support and may not be consistent with the user-facing interface and (b) more importantly, we wanted to preserve the ability (at the low level) to re-initialize based purely on slots. Once you have a custom initialize, it is no longer possible to call new() consistently (just with slots) across all classes. And (c), since in general we want constructors anyway (the user calling new() directly would sacrifice abstraction), this policy greatly reduced complexity by keeping the logic at a single layer. Others might have more to add; I stopped thinking after I got to three ;) Hope that helps, Michael On Mon, Jan 26, 2015 at 12:38 AM, Hahne, Florian florian.ha...@novartis.com wrote: Hi Michael, I'll take a look. In order to improve my code: what exactly do you think should be part of the initialiser, and what should be in the constructor? There don't seem to bee any clear guidelines out there anywhere. And if all logic goes in the constructor, how does one deal with more complex nested inheritance? And what's the use for the initialiser in the first place? Florian On 24/01/15 03:03, Michael Lawrence lawrence.mich...@gene.com wrote: I have found and fixed the affected initialize cases and will begin checking in fixes. Affected packages: RDAVIDWebService, flowCore (as we know), Gviz, ChIPseqR, Pviz, VanillaICE, flowFit. As an aside, in all of these cases, initialize is implementing logic that really belongs in a constructor function. Treating new() as a high-level constructor should be discouraged in my opinion. Has nothing to do with this bug though. I have also begun looking through cases outside of initialize. There are only about 300 cases of callNextMethod() in the codebase. Great majority seem OK so far. On Fri, Jan 23, 2015 at 12:46 PM, Michael Lawrence micha...@gene.com wrote: That's right. On Fri, Jan 23, 2015 at 12:22 PM, Mike wjia...@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 wjia...@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
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
I might add that the old (very old) style in Biobase was to use new as constructor. That one should avoid this all the time, is a newer insight, so you'll find plenty of old code violating this. Kasper On Mon, Jan 26, 2015 at 8:35 AM, Hahne, Florian florian.ha...@novartis.com wrote: Thanks, not much need for convincing here, just wanted a big enough push to get me over that 'kinda works so why bother` hurdle. I need to do other code refactoring in Gviz soon, and will take the opportunity to kick out all initialisers in the process. So one less package to worry about. Florian On 26/01/15 14:28, Michael Lawrence lawrence.mich...@gene.com wrote: In the S4Vectors/IRanges infrastructure, we have never defined an initialize method, so it is certainly possible to use the constructor pattern in complex frameworks, and IRanges serves as a good example of doing so. The basic pattern is to delegate to the superclass constructor and pass that object as an unnamed argument to new(). We stayed away from changing the formals of initialize, because (a) initialize() has a special contract of no-arg invocation that is a pain to support and may not be consistent with the user-facing interface and (b) more importantly, we wanted to preserve the ability (at the low level) to re-initialize based purely on slots. Once you have a custom initialize, it is no longer possible to call new() consistently (just with slots) across all classes. And (c), since in general we want constructors anyway (the user calling new() directly would sacrifice abstraction), this policy greatly reduced complexity by keeping the logic at a single layer. Others might have more to add; I stopped thinking after I got to three ;) Hope that helps, Michael On Mon, Jan 26, 2015 at 12:38 AM, Hahne, Florian florian.ha...@novartis.com wrote: Hi Michael, I'll take a look. In order to improve my code: what exactly do you think should be part of the initialiser, and what should be in the constructor? There don't seem to bee any clear guidelines out there anywhere. And if all logic goes in the constructor, how does one deal with more complex nested inheritance? And what's the use for the initialiser in the first place? Florian On 24/01/15 03:03, Michael Lawrence lawrence.mich...@gene.com wrote: I have found and fixed the affected initialize cases and will begin checking in fixes. Affected packages: RDAVIDWebService, flowCore (as we know), Gviz, ChIPseqR, Pviz, VanillaICE, flowFit. As an aside, in all of these cases, initialize is implementing logic that really belongs in a constructor function. Treating new() as a high-level constructor should be discouraged in my opinion. Has nothing to do with this bug though. I have also begun looking through cases outside of initialize. There are only about 300 cases of callNextMethod() in the codebase. Great majority seem OK so far. On Fri, Jan 23, 2015 at 12:46 PM, Michael Lawrence micha...@gene.com wrote: That's right. On Fri, Jan 23, 2015 at 12:22 PM, Mike wjia...@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 wjia...@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
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
Hi Michael, I'll take a look. In order to improve my code: what exactly do you think should be part of the initialiser, and what should be in the constructor? There don't seem to bee any clear guidelines out there anywhere. And if all logic goes in the constructor, how does one deal with more complex nested inheritance? And what's the use for the initialiser in the first place? Florian On 24/01/15 03:03, Michael Lawrence lawrence.mich...@gene.com wrote: I have found and fixed the affected initialize cases and will begin checking in fixes. Affected packages: RDAVIDWebService, flowCore (as we know), Gviz, ChIPseqR, Pviz, VanillaICE, flowFit. As an aside, in all of these cases, initialize is implementing logic that really belongs in a constructor function. Treating new() as a high-level constructor should be discouraged in my opinion. Has nothing to do with this bug though. I have also begun looking through cases outside of initialize. There are only about 300 cases of callNextMethod() in the codebase. Great majority seem OK so far. On Fri, Jan 23, 2015 at 12:46 PM, Michael Lawrence micha...@gene.com wrote: That's right. On Fri, Jan 23, 2015 at 12:22 PM, Mike wjia...@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 wjia...@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 mtmor...@fredhutch.org wrote: On 01/22/2015 12:26 AM, Martin Maechler wrote: Mike wjia...@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,
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
On Fri, Jan 23, 2015 at 6:03 PM, Michael Lawrence micha...@gene.com wrote: I have found and fixed the affected initialize cases and will begin checking in fixes. Affected packages: RDAVIDWebService, flowCore (as we know), Gviz, ChIPseqR, Pviz, VanillaICE, flowFit. As an aside, in all of these cases, initialize is implementing logic that really belongs in a constructor function. Treating new() as a high-level constructor should be discouraged in my opinion. Has nothing to do with this bug though. I have also begun looking through cases outside of initialize. There are only about 300 cases of callNextMethod() in the codebase. Great majority seem OK so far. I looked at 60 or so that occur inside functions with ... in their formals. Gviz had a bunch of questionable cases, so I have cleaned those up. I saw some cases where the Gviz authors had to do extra work to avoid the bug, so the code is simpler now. There was one other case in qpgraph. Will check in my changes now. Maintainers of the mentioned packages should keep an eye on things, because who knows, I could just be breaking as many things as I am fixing. On Fri, Jan 23, 2015 at 12:46 PM, Michael Lawrence micha...@gene.com wrote: That's right. On Fri, Jan 23, 2015 at 12:22 PM, Mike wjia...@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 wjia...@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 mtmor...@fredhutch.org wrote: On 01/22/2015 12:26 AM, Martin Maechler wrote: Mike wjia...@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
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
That's right.
On Fri, Jan 23, 2015 at 12:22 PM, Mike wjia...@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 wjia...@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 mtmor...@fredhutch.org
wrote:
On 01/22/2015 12:26 AM, Martin Maechler wrote:
Mike wjia...@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
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
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 mtmor...@fredhutch.org wrote:
On 01/22/2015 12:26 AM, Martin Maechler wrote:
Mike wjia...@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@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
--
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
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
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 wjia...@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 mtmor...@fredhutch.org
wrote:
On 01/22/2015 12:26 AM, Martin Maechler wrote:
Mike wjia...@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
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
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.
Michael
On Thu, Jan 22, 2015 at 6:15 AM, Martin Morgan mtmor...@fredhutch.org wrote:
On 01/22/2015 12:26 AM, Martin Maechler wrote:
Mike wjia...@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@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
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
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 wjia...@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 mtmor...@fredhutch.org
wrote:
On 01/22/2015 12:26 AM, Martin Maechler wrote:
Mike wjia...@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@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
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
I have found and fixed the affected initialize cases and will begin
checking in fixes.
Affected packages: RDAVIDWebService, flowCore (as we know), Gviz,
ChIPseqR, Pviz, VanillaICE, flowFit.
As an aside, in all of these cases, initialize is implementing logic
that really belongs in a constructor function. Treating new() as a
high-level constructor should be discouraged in my opinion. Has
nothing to do with this bug though.
I have also begun looking through cases outside of initialize. There
are only about 300 cases of callNextMethod() in the codebase. Great
majority seem OK so far.
On Fri, Jan 23, 2015 at 12:46 PM, Michael Lawrence micha...@gene.com wrote:
That's right.
On Fri, Jan 23, 2015 at 12:22 PM, Mike wjia...@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 wjia...@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 mtmor...@fredhutch.org
wrote:
On 01/22/2015 12:26 AM, Martin Maechler wrote:
Mike wjia...@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, ...)
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
On 01/22/2015 12:26 AM, Martin Maechler wrote:
Mike wjia...@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@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
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
- Original Message -
From: Dan Tenenbaum dtene...@fredhutch.org
To: Mike wjia...@fhcrc.org
Cc: Bioc-devel@r-project.org
Sent: Tuesday, January 20, 2015 5:01:52 PM
Subject: Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other
flow packages
Hi Mike,
- Original Message -
From: Mike wjia...@fhcrc.org
To: Bioc-devel@r-project.org
Cc: Dan Tenenbaum dtene...@fhcrc.org
Sent: Tuesday, January 20, 2015 4:56:43 PM
Subject: [Bioc-devel] Latest R-devel revision breaks flowCore and
other flow packages
Dan,
First of all, Thanks to the bioconductor |docker container| project
that
allows me to quickly troubleshoot within the exactly same
environment
that |bioc| has.
I want to be very clear that the docker containers we maintain
do NOT have exactly the same environment as the build systems. Maybe
a little closer, but not the same.
We are not attempting to replicate the build environment, just to
create an environment that
is useful to users. In particular, we try and build the docker
containers daily with the latest R-devel, whereas
we do not update R-devel daily on the build systems. There are many
other differences.
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()
})
|
The error goes away after |callNextMethod| call is removed.
I suspect it has to do with a bug introduced by the latest
|R-devel|
based on its svn history.
| ~/R/r-devel/src$ svn log |grep callNextMethod -C3
r67472 | lawrence |2015-01-14 20:11:30 -0800 (Wed,14 Jan2015)
|3 lines
callNextMethod no longer drops arguments named... (bug
introduced
in r67468)
--
r67468 | lawrence |2015-01-14 09:57:53 -0800 (Wed,14 Jan2015)
|3 lines
remove hack from C part of callNextMethod that should no longer
be
necessary given the dots fixes
r67467 | lawrence |2015-01-14 07:38:41 -0800 (Wed,14 Jan2015)
|3 lines
fix PR#16141 and other issues related to ... forwarding in
callGeneric() and callNextMethod()
|
If so, it should get fixed by the next run of updating |R-devel|
from
|bioc| (hopefully coming soon)
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.
Dan
OK, thanks for letting us know. Michael, if/when this is addressed
can you let us know?
Thanks,
Dan
Best,
Mike
[[alternative HTML version deleted]]
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
Hi Mike,
- Original Message -
From: Mike wjia...@fhcrc.org
To: Bioc-devel@r-project.org
Cc: Dan Tenenbaum dtene...@fhcrc.org
Sent: Tuesday, January 20, 2015 4:56:43 PM
Subject: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow
packages
Dan,
First of all, Thanks to the bioconductor |docker container| project
that
allows me to quickly troubleshoot within the exactly same environment
that |bioc| has.
I want to be very clear that the docker containers we maintain
do NOT have exactly the same environment as the build systems. Maybe a little
closer, but not the same.
We are not attempting to replicate the build environment, just to create an
environment that
is useful to users. In particular, we try and build the docker containers daily
with the latest R-devel, whereas
we do not update R-devel daily on the build systems. There are many other
differences.
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()
})
|
The error goes away after |callNextMethod| call is removed.
I suspect it has to do with a bug introduced by the latest |R-devel|
based on its svn history.
| ~/R/r-devel/src$ svn log |grep callNextMethod -C3
r67472 | lawrence |2015-01-14 20:11:30 -0800 (Wed,14 Jan2015)
|3 lines
callNextMethod no longer drops arguments named... (bug
introduced
in r67468)
--
r67468 | lawrence |2015-01-14 09:57:53 -0800 (Wed,14 Jan2015)
|3 lines
remove hack from C part of callNextMethod that should no longer be
necessary given the dots fixes
r67467 | lawrence |2015-01-14 07:38:41 -0800 (Wed,14 Jan2015)
|3 lines
fix PR#16141 and other issues related to ... forwarding in
callGeneric() and callNextMethod()
|
If so, it should get fixed by the next run of updating |R-devel| from
|bioc| (hopefully coming soon)
OK, thanks for letting us know. Michael, if/when this is addressed can you let
us know?
Thanks,
Dan
Best,
Mike
[[alternative HTML version deleted]]
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
[Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
Dan,
First of all, Thanks to the bioconductor |docker container| project that
allows me to quickly troubleshoot within the exactly same environment
that |bioc| has.
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()
})
|
The error goes away after |callNextMethod| call is removed.
I suspect it has to do with a bug introduced by the latest |R-devel|
based on its svn history.
| ~/R/r-devel/src$ svn log |grep callNextMethod -C3
r67472 | lawrence |2015-01-14 20:11:30 -0800 (Wed,14 Jan2015) |3 lines
callNextMethod no longer drops arguments named... (bug introduced
in r67468)
--
r67468 | lawrence |2015-01-14 09:57:53 -0800 (Wed,14 Jan2015) |3 lines
remove hack from C part of callNextMethod that should no longer be
necessary given the dots fixes
r67467 | lawrence |2015-01-14 07:38:41 -0800 (Wed,14 Jan2015) |3 lines
fix PR#16141 and other issues related to ... forwarding in
callGeneric() and callNextMethod()
|
If so, it should get fixed by the next run of updating |R-devel| from
|bioc| (hopefully coming soon)
Best,
Mike
[[alternative HTML version deleted]]
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
Re: [Bioc-devel] Latest R-devel revision breaks flowCore and other flow packages
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. Mike 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@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel
