Dear Martin et al., Thank you for considering so thoroughly the issue that I raised.
Best, John On Sat, 13 Jun 2015 13:35:41 +0200 Martin Maechler <maech...@stat.math.ethz.ch> wrote: > >>>>> Luke Tierney <luke-tier...@uiowa.edu> > >>>>> on Fri, 12 Jun 2015 10:30:29 -0500 writes: > > > 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 > > Thanks a lot Luke, for the extra perspective. > I think the four R core commenters here (Duncan, Kurt, Luke and > me) agree that this is not trivial to implement, but hopefully > not too hard either, and I think we also +- agree that it seems > desirable to try adding a bit more flexibility in how functions > are "made into" S3 methods. > > I had not envisaged to change the S3 method search > algorithm but rather to tweak part of it "database" but am aware > that I don't know enough of the details. > Also, I did not find which notes (from developer.r-project.org) > you were refering to. > > Given the broad agreement that we want to start working / > investigating this, we can well close the thread here for now > (and deal with ideas, issues, details within R-core). > > Martin > > > 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