[R-pkg-devel] S3 generic/method consistency false positive when dplyr imported

[Ivan Krylov]

On Thu, 5 Jan 2023 17:58:47 -0500
"Aaron A. King" <kingaa using umich.edu> wrote:

> However, since the S3 dispatch system is not itself confused, I
> suppose it must be the case that the S3 generic/method consistency
> checker itself is confused.  I wonder, how does the checker decide
> when to throw a NOTE?  Is it a simple pattern-matching system,
> something like "If X.Y is exported, where X is a generic and Y is any
> string and X.Y is not declared as an S3 method, then throw NOTE"?

The aim of the check is a bit different: "if an S3 method for generic X
and class Y exists, and the formal arguments of X.Y don't extend the
formal arguments of X in a certain way, then throw a NOTE", but then
something gets wrong about deciding whether X.Y is an S3 method, and
once filter.mean is decided to be an S3 method, of course it doesn't
match the formals of filter.

This function is even a public API of the tools package, see
?tools::checkS3methods:
https://github.com/r-devel/r-svn/blob/b57918fd104962415a752ea7db12dcf4f3f5219f/src/library/tools/R/QC.R#L2310-L2560

> When I follow your suggestion above, I get
> 
> library(pomp)
> dplyr::filter(structure(list(), class = 'traj'))
> 
> Error in UseMethod("filter") :
> no applicable method for 'filter' applied to an object of class "traj"

Interesting. I wonder if there's are other remaining ways to confuse
the S3 dispatch system into thinking your function is an S3 method that
the check is supposed to guard against. For instance, the following is
likely to work with your package:

library(pomp)
library(dplyr)
identical(filter.traj, getS3method('filter', 'traj'))
# as long as there's a visible 'filter' generic, getS3method will
# locate filter.traj as the method

...despite the method lookup seems to (correctly) fail, as you've just
demonstrated. But on older versions of R, this used to wrongly resolve
the call:

library(Ropj)
read <- function(x, ...) UseMethod('read')
identical(read.opj, getS3method('read','opj')) # works in R-devel
# [1] TRUE
read(structure(0, class = 'opj')) # calls read.opj in old R
# Error in read_opj(file, encoding, tree) :
#   Expecting a single string value: [type=double; extent=1].

I'm afraid I'm out of my depth here. The old, wrong behaviour seems to
match the R language definition
<https://cran.r-project.org/doc/manuals/r-release/R-lang.html#Definition>.
The current behaviour seems to do the right thing™ by letting the
functions opt into being methods, but then getS3method() seems to
contradict what actually gets called. Should we be fixing the
documentation, the implementation, or the check?

Can you try stepping through tools::checkS3methods('pomp') (or
tools::checkS3methods(dir = 'path/to/source/directory')) to see where
'filter' appears in the list of generics and where the 

-- 
Best regards,
Ivan