>>>> "JC" == John Chambers <j...@r-project.org>
>>>> on Mon, 09 Mar 2009 09:53:06 -0700
JC> As Yohan points out, and as we found in testing CRAN packages,
JC> there are
JC> a number of examples where programmers have written S3 methods
JC> for S4
JC> classes, such as print.aTest() below.
JC>
JC> This may well have seemed an easy and convenient mechanism,
JC> but it is a
JC> design error with potentially disastrous consequences.
JC> Note that this
JC> is fundamentally different from an S3 method defined for an
JC> S3 class and
JC> inherited by an S4 class that extends the S3 class.
JC>
JC> We haven't decided whether to re-enable this, but if so it
JC> will be with
JC> a warning at user call and/or at package check time.
JC>
JC> Please turn such methods into legitimate S4 methods.
JC> Usually the change
JC> is a simple call to setMethod(); in some cases, such as this
JC> example you
JC> may need a bit more work, such as a show() method to call
JC> the print.*
JC> function.
JC>
JC> DETAILS:
JC>
JC> There are at least two serious flaws in this mechanism, plus
JC> some minor
JC> defects.
JC>
JC> 1. S3 dispatch does not know about S4 class inheritance.
JC> So if as a user of Yohan's code, for example, I define a class
JC> setClass(bTest, contains = aTest)
JC> then that class will not inherit any of the S3 methods.
JC> In the case of
JC> print, that will be obvious. The disaster waiting to happen
JC> is when the
JC> method involves numerical results, in which case I may be
JC> getting
JC> erroneous results, with no hint of the problem.
JC>
JC> 2. Conversely, S4 dispatch does not know about the S3 method.
JC> So, if my new class was:
JC> setClass(bTest, contains = c(aTest, waffle7)
JC> suppose waffle7 has some huge inheritance, in the midst of
JC> which is a
JC> method for a generic function of importance. I expect to
JC> inherit the
JC> method for aTest because it's for a direct superclass, but I
JC> won't, no
JC> matter how far up the tree the other method occurs. (Even if
JC> point 1
JC> were fixed)
JC>
JC> Again, this would be obvious for a print method, but potentially
JC> disastrous elsewhere.
JC>
JC> There are other minor issues, such as efficiency: the S3 method
JC> requires two dispatches and perhaps may do some extra copying.
JC> But 1
JC> and 2 are the killers.
JC>
JC> Just to anticipate a suggestion: Yes, we could perhaps fix 1
JC> by adding
JC> code to the S3 dispatch, but this would ambiguate the legitimate
JC> attempt
JC> to handle inherited valid S3 methods correctly.
JC>
JC> John
Dear John,
Thank you for the detailed explanation.
I completely understand that it is a design error and that it should be
fixed. As you said it is a matter of using 'setMethod'.
My main concern is that such a change happens one month before a new
release. This is very little time for developers to make their packages
consistent with the new release.
As a side note, I have noticed that it is still possible to define S3
methods for S4 classes which do not contains a super-class like matrix.
But the disastrous consequences that you explained would still be
possible in my opinion.
setClass("aTest", representation(.Data = "matrix", comment = "character"))
a <- new("aTest", .Data = matrix(1:4, ncol = 2), comment = "aTest")
as.matrix.aTest <- function(x, ...) getDataPart(x)
as.matrix(a) # returns same S4 object
# but
setClass("bTest", representation(Data = "matrix", comment = "character"))
b <- new("bTest", Data = matrix(1:4, ncol = 2), comment = "hello")
as.matrix.bTest <- function(x, ...) b...@data
as.matrix(b) # does work
Are you planing to turn this off too?
Again, I understand that developers should conform with the S4 style
but a one month notice is very short in my opinion. Moreover adding a
warning in R CMD check would be the same because developer won't be
able to submit packages since CRAN only accepts packages which pass R
CMD check without warnings.
Best regards,
Yohan
--
PhD student
Swiss Federal Institute of Technology
Zurich
www.ethz.ch
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
