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:
>>
>>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
