Thanks for those (very different!) responses. I was about to respond to
Martin's question about doco amendments, when I saw Simon's reply.
Case 1: If Martin completely prevails, then I guess the appropriate amendment
to the documentation would be in R-extensions, section 1.3.1 "Checking
packages", item 10:
<<... In addition, it is checked whether methods have all arguments of the
corresponding generic,...>>
would become
<<... In addition, it is checked whether potential methods[1] have all
arguments of the corresponding generic,...>>
where [1] refers to a footnote:
<<"Potential methods" here includes any function whose name starts with a
known S3 generic followed by a period, e.g. 'subset.blahblah.blah'. This
applies regardless of whether the function is formally registered as an S3
method (see 1.6.2), and even if the function is not intended to be a method at
all. It is bad practice to give method-like names to non-methods.>>
For absolutely strict super-nanny-level consistency, maybe there ought also to
be a change somewhere in the QC/codoc code, to detect "methods" that haven't
been registered with 'S3method', and/or have non-generic-style USAGE sections,
regardless of whether they happen to have arguments compatible with whatever
generic R thinks they belong to. At present, there is a difference in what
'tools::checkDocStyle' and 'tools::checkS3methods' regard as a method. However,
enforcing this extra check does strike me as a bit draconian, and would
certainly break back-compatibility (in other bits of my code, for a start).
You'd have to ask whether it's worth the effort. In my example, I don't think
it's worth worrying that anyone is ever going to define an S3 class called
'with.warning' and decide to call 'subset' on it.
Case 2: If Simon completely prevails, then there should be changes in
'tools::checkS3methods' to exempt "documented non-methods". I think the
appropriate definition of "documented non-methods" would be functions with a
USAGE section that includes the full function name rather than the generic.
That's supposed to cover packages with and without NAMESPACEs.
WRTO Simon's comment: I feel the same negative way about camel-case for the
same reasons, and would want a much stronger reason to change the naming
convention I've been using for 15 years; it's hard enough to remember what name
I gave to a function, letAlone how_I_decided to.spell ItThatWeek.
However, I really really hope that S3 *doesn't* get replaced by S4! S3 is very
easy to use, and for me it "just works" 99% of the time. Just occasionally it
doesn't, but for me at least "the [S4] cure is worse than the [S3] disease".
I'll keep an open mind about R5 ;)
Mark Bravington
CSIRO CMIS
Marine Lab
Hobart
Australia
________________________________________
From: Simon Urbanek [simon.urba...@r-project.org]
Sent: 27 August 2010 02:46
To: Martin Maechler
Cc: Bravington, Mark (CMIS, Hobart); r-devel@r-project.org
Subject: Re: [Rd] RCMD CHECK and non-methods
On Aug 26, 2010, at 11:36 AM, Martin Maechler wrote:
>>>>>> <mark.braving...@csiro.au>
>>>>>> on Wed, 25 Aug 2010 14:06:07 +1000 writes:
>
>> I recently moved a function 'subset.with.warning' into the 'mvbutils'
>> package (a version not yet on CRAN). When I tried RCMD CHECK, I got this
>> warning:
>> * checking S3 generic/method consistency ... WARNING
>> subset:
>> function(x, ...)
>> subset.with.warning:
>> function(x, cond, mess.head, mess.cond, row.info, sub)
>
>> See section 'Generic functions and methods' of the 'Writing R Extensions'
>> manual.
>
>> I know that S3 method arguments need to be compatible with arguments of the
>> generic. However, 'subset.with.warning' is deliberately not a registered S3
>> method,
>
> I think that's your real trouble ... and then your users' if you
> really insist.
> Short answer: "Don't do that!"
>
> There have been a few exceptions of "100 years old" R functions
> which validated this rule,
> the most notable probably t() and t.test(),
> and we (R core) have explicitly listed them in the code base
> as "yes, looks like a method for an S3 generic, but not it ain't!",
> but have basically "forbidden" to commit more such crimes.
> {Also, if you are interested: I think both t() and t.test()
> pre-dated S3}
>
However, one would hope that S3 will be replaced by S4 (or R5? ;)) eventually
so requiring such an arcane rule seems little strong. Personally, I prefer the
use of . to that of camelCase [I find it more readable and faster to type] and
thus I'm with Mark on this one. IMHO it is perfectly legal and clean to declare
a symbol as a function and not an S3 method.
Cheers,
Simon
>
>> and its USAGE section doesn't include a \method{generic}{class} statement. I
>> couldn't see anything in "R Extensions" that says "don't do this", so I'm
>> wondering:
>
> Yes, we should add a such "don't do this" somewhere.
>
> Can you propose a good place to put it in there?
>
>> - should this really be a NOTE not a WARNING (or nothing at all)?
>
> {from the above: definitely a warning, if not "worse"}....
>
> Best regards,
> Martin
>
> Martin Maechler, ETH Zurich
>
>> - if not, shouldn't there be a more explicit statement to the effect that
>> "if R decides it's a method, then it damned well is a method, whether you
>> think it is or not"?
>
>> - and if so, should there also be a check for functions that look like
>> methods but aren't registered and declared as such?
>
>> My preference would be for the first, FWIW. Admittedly, just because I
>> didn't register 'subset.with.warning' as an S3 method, that won't stop
>> 'subset' from trying to use it if it ever encounters an object of class
>> "with.warning". It's a risk that I'm happy to take, though CRAN might not
>> be...
>
>> I made the warning go away by adding a '...' argument to the end of
>> 'subset.with.warning', but that's not pretty.
>
>> Mark Bravington
>> CSIRO
>> Hobart
>> Australia
>
>>> sessionInfo()
>> R version 2.11.1 Patched (2010-06-30 r52418)
>> i386-pc-mingw32
>
>> locale:
>> [1] LC_COLLATE=English_Australia.1252 LC_CTYPE=English_Australia.1252
>> LC_MONETARY=English_Australia.1252 LC_NUMERIC=C
>> [5] LC_TIME=English_Australia.1252
>
>> attached base packages:
>> [1] grDevices tools tcltk stats graphics utils methods
>> base
>
>> other attached packages:
>> [1] ad_1.0 chstuff_1.0 handy2_1.2 tweedie_2.0.2
>> statmod_1.4.1 handy_1.1 debug_1.2.3 mvbutils_2.5.2
>>>
>
>> ______________________________________________
>> R-devel@r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
> ______________________________________________
> R-devel@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
