The notes available off the devloper page
https://developer.r-project.org/ describe some of the rationale for
the S3 method search design. One thing that has changed since then is
that all packages now have name spaces. We could change the search
algorithm to skip attached package exports (and package imports and
base), which would require methods defined in packages that are to be
accessible outside the package to be declared. Methods defined inside
a package for internal use or methods defined in scripts not in
packages would still be found. Packages not currently registering
their methods would have to do so -- not sure how many that would
affect. Testing on CRAN/Bioc should show how much of an effect this
would have and whether there are any other issues.
Best,
luke
On Fri, 12 Jun 2015, Duncan Murdoch wrote:
On 12/06/2015 10:53 AM, Hadley Wickham wrote:
To me, it seems like there's actually two problems here:
1) Preventing all() from dispatching to all.effects() for objects of
class effects
2) Eliminating the NOTE in R CMD check
My impression is that 1) actually causes few problems, particularly
since people are mostly now aware of the problem and avoid using `.`
in function names unless they're S3 methods. Fixing this issue seems
like it would be a lot of work for relatively little gain.
However, I think we want to prevent people from writing new functions
with this confusing naming scheme, but equally we want to grandfather
in existing functions, because renaming them all would be a lot of
work (I'm looking at you t.test()!).
Could we have a system similar to globalVariables() where you could
flag a function as definitely not being an S3 method? I'm not sure
what R CMD check should do - ideally you wouldn't be allow to use
method.class for new functions, but still be able suppress the note
for old functions that can't easily be changed.
We have a mechanism for suppressing the warning for existing functions,
it's just not available to users to modify. So it would be possible to
add effects::all.effects to the stop list, and this might be the easiest
action here.
This isn't perfect because all.effects() would still act as a method.
However, it does give the deprecated message if you ever call it, so
nobody would do this unknowingly. The only real risk is that if anyone
ever wrote an all.effects function that *was* supposed to be an S3
method, it might be masked by the one in effects.
Duncan Murdoch
Hadley
On Fri, Jun 12, 2015 at 6:52 AM, Kurt Hornik <kurt.hor...@wu.ac.at> wrote:
Duncan Murdoch writes:
On 12/06/2015 7:16 AM, Kurt Hornik wrote:
Duncan Murdoch writes:
On 12/06/2015 4:12 AM, Martin Maechler wrote:
This is a topic ' "apparent S3 methods" note in R CMD check '
from R-package-devel
https://stat.ethz.ch/pipermail/r-package-devel/2015q2/000126.html
which is relevant to here because some of us have been thinking
about extending R because of the issue.
John Fox, maintainer of the 'effects' package has enquired about
the following output from 'R CMD check effects'
* checking S3 generic/method consistency ... NOTE
Found the following apparent S3 methods exported but not registered:
all.effects
and added
The offending function, all.effects(), is deprecated in favour of
allEffects(), but I'd rather not get rid of it for backwards compatibility.
Is there any way to suppress the note without removing all.effects()?
and I had agreed that this was a "False Positive" in this case.
[.......]
and then
Now I agree .. and have e-talked about this with another R core
member .. that it would be desirable for the package author to
effectively declare the fact that such a function is not an S3
method even though it "looks like it" at least if looked from far.
So, ideally, you could have something like
nonS3method("all.effects")
somewhere in your package source ( in NAMESPACE or R/*.R )
which would tell the package-checking code -- but *ALSO* all the other S3
method code that all.effects should be treated as a regular R
function.
I would very much like such a feature in R, and for that reason,
I'm cross posting this (as one of the famous exceptions that
accompany real-life rules!!) to R-devel.
and actually I did *not* cross post, but have now moved the
relevant part of the thread to R-devel.
It sounds like a good idea. It's a nontrivial amount of work, because
of the "all the other S3 method code" part. There's the question of
functions defined outside of packages: presumably they are still S3
methods, with no way to suppress that.
I am not sure this is the right solution: S3 dispatch will still occur
because we first look at foo.bar exports and then in the S3 registry,
afaicr (the "all the other S3 method code" part).
If we could move to only looking at the registry for dispatch, there
would be no need to declare situations where we should not dispatch on
foo.bar exports.
I think that would break a lot of existing scripts. I think the logic
should be something like this.
For each class in the class list:
Search backwards through the environment chain. If the current location
is a package environment or namespace, look only in the registry. If
not, look at all functions.
If that search failed, try the next class.
Yep---that's what I meant. I forgot to write the "if namespace
semantics applies" part :-)
Best
-k
A variation on the test is: If there's a registry in the current
location, look there. But I think the registry is not on the search
list, so I'm not sure that would work.
This assumes that we keep separate registries in each package; if we
merge them into one big registry, it gets harder. I'm not familiar
enough with the code to know which way we do it.
Duncan Murdoch
______________________________________________
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
--
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa Phone: 319-335-3386
Department of Statistics and Fax: 319-335-3017
Actuarial Science
241 Schaeffer Hall email: luke-tier...@uiowa.edu
Iowa City, IA 52242 WWW: http://www.stat.uiowa.edu
______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
