On Sat, Jan 11, 2014 at 12:11 PM, Martin Morgan <mtmor...@fhcrc.org> wrote:
> Comments inline below. Two bugs and a feature request would seem to be > steps in the right direction, but repeat previous ideas that seem not to > have gained traction / appropriate notice. > > bug 1. showMethods apparently ignores the default value for 'where' > > http://r.789695.n4.nabble.com/problem-with-showMethods-td4676667.html > > bug 2. getGeneric("c") has an argument 'x' (no longer necessary?) that > interferes with concentration of named S4 instances (illustrated below). I > think this has been reported to R-devel before, but I couldn't find the > thread... > > feature request 1. unify S3 and S4 method discovery. I think Martin > Maechler has expressed interest in this in the past. > > > > On 01/11/2014 06:39 AM, Michael Lawrence wrote: > >> Hi, >> >> Defining S3 methods for our data types is obviously useful, because it >> helps them work with code that only sees the S3 generic. We should >> encourage the definition of S3 methods whenever there is an existing >> generic. Given that, some questions: >> >> Is it still worth it to define the corresponding S4 method? Now that S3 >> > > I think the 'best practices' are laid out in the ?Methods documentation > under 'Methods for S3 Generic Functions' which suggests (and provides > justification for) writing both S3 and S4 methods. > > Yes, thanks, I see: someone could override my S3 method simply by defining an S4 method on a superclass. Of course, if we all decided to *only* use S3 methods for S3 generics, this would not be a problem. But there's no way to enforce that, so we're stuck with the overhead of defining so many fundamental methods *twice*. Seems like the methods package could somehow be smart enough to define (and handle the namespacing) of an S3 method when defining a method on an S4 generic that has a corresponding S3 generic (obeying certain constraints, like the S4 generic only dispatching on the argument considered by S3). > > dispatch accounts for S4 inheritance, I'm not sure there is a functional >> difference. Obviously, it would be nice if there were one interface for >> discovering methods. Right now, we're split between >> selectMethod()/showMethods() and methods(). >> > > This really does seem unfortunate and not impossible to correct, but the > current reality is that discovering / seeking help requires facility with > both S3 and S4 approaches; I believe Martin Maechler solicited comments > about this on the R-devel mailing list several years ago, so there is some > hope for progress. Providing users with consistent guidance is helpful. For > showMethods this includes using where=search() when the command is invoked. > For help I find method?"coerc<tab>" or ?"coer<tab>" to be helpful. I don't > know how well Rstudio plays with this, though. > > > In particular, there are many setAs() calls where "to" is an S3 class. For >> many of the basic classes, the methods package already defines a coerce >> method that delegates to the as.X S3 generic. So those definitions are >> redundant. For example: >> >> showMethods(coerce, classes="list") >>> >> Function: coerce (package methods) >> from="ANY", to="list" >> from="CompressedAtomicList", to="list" >> from="Hits", to="list" >> from="List", to="list" >> from="Rle", to="list" >> >> Those four methods are unneeded if there is an as.list method. I've >> noticed >> that there is no coerce,ANY,data.frame so perhaps that could be added to >> BiocGenerics or requested of the methods package? >> > > I think that the need for both S3 and S4 methods are covered by the > ?Methods documentation mentioned above? > > > Finally, the "c" function. There's no need for an S3 method here, since >> "c" >> is a primitive. But there are notes in IRanges-class.R about how the S4 >> generic for "c" does not correctly account for inheritance when the >> arguments are named. This is probably a bug in the methods package. But it >> does seem that the S3 generic for 'c' correctly dispatches (on its first >> argument) regardless of whether the arguments are named. The only downside >> is that it does not enforce that all arguments in '...' are of the same >> type. >> > > The challenge with names and 'c' is that it can disrupt order of the > concatenation > > > c(IRanges(1, 2), IRanges(2, 3)) > IRanges of length 2 > start end width > [1] 1 2 2 > [2] 2 3 2 > > c(a=IRanges(1, 2), IRanges(2, 3)) > IRanges of length 2 > start end width > [1] 2 3 2 > [2] 1 2 2 > > base::c(a=IRanges(1, 2), IRanges(2, 3)) > IRanges of length 2 > start end width > [1] 2 3 2 > [2] 1 2 2 > > Yea, there are multiple problems with "c". Was just a thought that we could define S3 methods, since the S3 generic does not have the "x" artifact. But it's a bug in the methods package that should be addressed. Might be hard though, considering it's a primitive, and the C-level dispatch is likely not smart enough to correctly handle variadic generics (is 'c' the only one in this category?). > Martin Morgan > > >> Michael >> >> [[alternative HTML version deleted]] >> >> _______________________________________________ >> 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 > [[alternative HTML version deleted]] _______________________________________________ Bioc-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/bioc-devel
