[R-pkg-devel] S3 generic/method consistency false positive when dplyr imported
Hi all, I'm modifying a package so as to remove its dependence on the superseded packages *reshape2* and *plyr*, in favor of dependence on *dplyr*. In particular, I'm just importing the one function `dplyr::bind_rows`. A curious thing happens, however, as soon as I include this among the imports. I begin to get R CMD check NOTEs alerting me to the presence of S3 generic inconsistencies. These are associated with two functions in the package: `filter.mean` and `filter.traj`, which the checker mistakes for S3 methods based on the generic `stats::filter` function. Actually, these are not S3 methods. In fact, they are S4 generics and declared as such. When *dplyr* is not imported, R CMD check is happy with these functions. When it is imported, R CMD check complains. Now I understand that it may be preferable to move away from functions with names containing dots for this reason, and in fact I am already doing so, but I think the question is a good one: Why does inclusion of *dplyr* in the import list appear to trigger these warnings when they were not there before? Thanks in advance for any insight you can give! Aaron A. King, Ph.D. Nelson G. Hairston Professor of Ecology, Evolutionary Biology, and Complex Systems University of Michigan kinglab.eeb.lsa.umich.edu GPG Public Key: 0x15780975 [[alternative HTML version deleted]] __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel
Re: [R-pkg-devel] S3 generic/method consistency false positive when dplyr imported
Thanks for the fast reply!
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"
Which is not what would happen if my filter.traj function were called. So
the S3 dispatch system is not confused. Thus this really is a false
positive.
It still leaves open the question, why does this suddenly appear when dplyr
is loaded? If I understand you correctly, it would be triggered by the
inclusion of the generic 'filter' in dplyr, which was previously not
present. 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"?
Dr. Aaron A. King
University of Michigan
On Wed, Jan 4, 2023 at 4:22 PM Ivan Krylov wrote:
> On Wed, 4 Jan 2023 15:54:52 -0500
> "Aaron A. King" wrote:
>
> > Why does inclusion of *dplyr* in the import list appear to trigger
> > these warnings when they were not there before?
>
> I think that's because dplyr itself contains an S3 generic named
> "filter", and having a package namespace loaded (not even attached) is
> enough to take its registered S3 methods into consideration for method
> dispatch. Here we have an opposite situation, though, and your
> functions aren't registered as S3 methods. Will your function still be
> called if you artificially construct a wrong object and feed it to
> dplyr::filter while your package namespace is attached?
>
> library(YOURPACKAGE)
> dplyr::filter(structure(list(), class = 'traj'))
>
> Additionally, stats::filter doesn't seem to be an S3 generic, so that's
> one thing that the check gets confused about.
>
> --
> Best regards,
> Ivan
>
[[alternative HTML version deleted]]
__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel
Re: [R-pkg-devel] S3 generic/method consistency false positive when dplyr imported
Excellent suggestion, Ivan. Thanks for pointing out just where to look.
I stepped through tools::checkS3methods("pomp") and it does appear to be
roughly as I conjectured. Namely, all functions are extracted from the
pomp namespace and checked against a list of generics. The check that is
performed is a simple pattern-match (i.e.,
startsWith(paste0(genericname,"."), functionname)). Clearly this is prone
to false positives. I wonder what would be involved in tightening this up
so that it agrees with what the actual dispatch mechanism gives
Interestingly, there is a whitelist, implemented mostly via
tools::nonS3methods(). The function (plyr::rbind.fill) that I had been
importing is there but its replacement (dplyr::bind_rows) is not. So this
answers my original question.
The whitelist appears to be a somewhat arbitrary compilation of functions
from various packages, with no obvious theme. For example, dplyr is not on
the list, but ElectoGraph is; reshape is on the list but reshape2 is not.
The impression of arbitrariness is reinforced by the fact that packages
that Wickham has for whatever reason decided are superseded are present in
that list and packages he considers for the moment to be worth maintaining
are not. Doing a quick 'git blame' I see that the whitelist was mostly put
together in 2013, with a few additions in 2015 and much more occasionally
since.
It occurs to me that there ought to be a mechanism whereby a package author
could add to the list of non-S3 methods. I see from the mailing list
archives that there was discussion of this back in 2015 (
https://stat.ethz.ch/pipermail/r-devel/2015-June/071356.html), with what
looked like consensus that there ought to be such a facility, but nothing
seems to have actually been done about it. I conjecture that the decision
was to just fold false positives into the whitelist on an ad hoc basis.
Dr. Aaron A. King
University of Michigan
On Fri, Jan 6, 2023 at 3:24 AM Ivan Krylov wrote:
> (Sorry for the unfinished message. I've got to do something about my
> mailer to make it harder to send them accidentally.)
>
> On Fri, 6 Jan 2023 11:21:49 +0300
> Ivan Krylov wrote:
>
> > to see where 'filter' appears in the list of generics and where the
>
> filter.traj function gets matched to it as a method?
>
> --
> Best regards,
> Ivan
>
[[alternative HTML version deleted]]
__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel
