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