From maechler at stat.math.ethz.ch  Fri Jun 12 17:09:32 2015
From: maechler at stat.math.ethz.ch (Martin Maechler)
Date: Fri, 12 Jun 2015 17:09:32 +0200
Subject: [Rd] RFC: Declaring "foo.bar" as nonS3method() ?!
In-Reply-To: <CABdHhvFTb=erfo6T+83pBqG5b0y0hQLgO3u=Ywh6F2WoZu8crw@mail.gmail.com>
References: <000e01d0a387$885ad130$99107390$@mcmaster.ca>
	<21882.36966.429889.227076@stat.math.ethz.ch>
	<21882.37975.726216.982364@stat.math.ethz.ch>
	<557ABD04.1040205@gmail.com>
	<21882.49031.599800.412740@fangorn.hornik.net>
	<557AC6FC.8060206@gmail.com>
	<21882.51223.144401.149543@fangorn.hornik.net>
	<CABdHhvFTb=erfo6T+83pBqG5b0y0hQLgO3u=Ywh6F2WoZu8crw@mail.gmail.com>
Message-ID: <21882.63020.988582.597510@stat.math.ethz.ch>

>>>>> Hadley Wickham <h.wickham at gmail.com>
>>>>>     on Fri, 12 Jun 2015 09:53:20 -0500 writes:

    > 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

Sure.  ... and that what I said in my OP

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

Well, we may disagree here.  I did mean to improve the system
"deep down", and Kurt and Duncan where basically also blowing
the same trumpet.

We are not re-inventing all the wheels on all the cars that have
been racing around - sometimes for decennia... and so some names
have been there for a long time and will remain.

    > 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()!).

exactly.

    > Could we have a system similar to globalVariables() where you could
    > flag a function as definitely not being an S3 method? 

did you read what I wrote originally?  
Exactly that - with a working-proposition   
	nonS3method("<name>")

And BTW  ---- I'm diverting and I  seem to "preach" in vain
here, but in my view 

    globalVariables("xy") 

is really a very very poor "solution" and far from what one would desire: 
If you use it in your package, say because one function uses
non-standard evaluation and hence 'xy',
*all* erronous global uses of 'xy in all other functions in your
package will *not* be flagged by the nice codetools package
functionality which is behind that part of R CMD check.

Martin

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


    > Hadley

    > On Fri, Jun 12, 2015 at 6:52 AM, Kurt Hornik <Kurt.Hornik at 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 at r-project.org mailing list
    >> https://stat.ethz.ch/mailman/listinfo/r-devel



    > -- 
    > http://had.co.nz/



[Rd] RFC: Declaring "foo.bar" as nonS3method() ?!
Martin Maechler