[Rd] Improving user-friendliness of S4 dispatch failure when mis-naming arguments?

[Michael Chirico]

> I'm not entirely sure about the extra call. = FALSE

My thinking is the signature of the internal .InheritForDispatch() is
not likely to help anyone,
in fact having the opposite effect for beginners unsure how to use that info.

> Now I'd like to find an example that only uses packages with priority base | Recommended

Sure, here are a few.

library(Matrix)
# searching for Matrix-owned generics
matrix_generics <- getGenerics(where = asNamespace("Matrix"))
matrix_generics using .Data[matrix_generics using package == "Matrix"]

# simple signature, one argument 'x'
symmpart()
# Error: unable to find an inherited method for function ‘symmpart’
for signature ‘x="missing"’

# more complicated signature, especially including ...
Cholesky(a = 1)
# Error: unable to find an inherited method for function ‘Cholesky’
for signature ‘A="missing"’
Cholesky(a = 1, perm = TRUE)
# Error: unable to find an inherited method for function ‘Cholesky’
for signature ‘A="missing"’
Cholesky(a = 1, perm = TRUE, IMult = 2)
# Error: unable to find an inherited method for function ‘Cholesky’
for signature ‘A="missing"’

---

'base' is a bit harder since stats4 just provides classes over stats
functions, so the missigness error comes from non-S4 code.

library(stats4)
coef()
# Error in coef.default() : argument "object" is missing, with no default

Defining our own generic:

setGeneric("BaseGeneric", \(a, ...) standardGeneric("BaseGeneric"))
BaseGeneric()
# Error: unable to find an inherited method for function ‘BaseGeneric’
for signature ‘a="missing"’

# getting multiple classes to show up requires setting the signature:
setMethod("BaseGeneric", signature(x = "double", y = "double"), \(x,
y, ...) x + y)
BaseGeneric(X = 1, Y = 2)
# Error: unable to find an inherited method for function ‘BaseGeneric’
for signature ‘x="missing", y="missing"’


On Fri, Aug 11, 2023 at 2:26 AM Martin Maechler
<maechler using stat.math.ethz.ch> wrote:
>
> >>>>> Michael Chirico via R-devel
> >>>>>     on Thu, 10 Aug 2023 23:56:45 -0700 writes:
>
> > Here's a trivial patch that offers some improvement:
>
>     > Index: src/library/methods/R/methodsTable.R
>     > ===================================================================
>     > --- src/library/methods/R/methodsTable.R (revision 84931)
>     > +++ src/library/methods/R/methodsTable.R (working copy)
>     > @@ -752,11 +752,12 @@
>     > if(length(methods) == 1L)
>     > return(methods[[1L]]) # the method
>     > else if(length(methods) == 0L) {
>     > -    cnames <- paste0("\"", vapply(classes, as.character, ""), "\"",
>     > +    cnames <- paste0(head(fdef using signature, length(classes)), "=\"",
>     > vapply(classes, as.character, ""), "\"",
>     > collapse = ", ")
>     > stop(gettextf("unable to find an inherited method for function %s
>     > for signature %s",
>     > sQuote(fdef using generic),
>     > sQuote(cnames)),
>     > +         call. = FALSE,
>     > domain = NA)
>     > }
>     > else
>
>     > Here's the upshot for the example on DBI:
>
>     > dbGetQuery(connection = conn, query = query)
>     > Error: unable to find an inherited method for function ‘dbGetQuery’
>     > for signature ‘conn="missing", statement="missing"’
>
>     > I don't have any confidence about edge cases / robustness of this
>     > patch for generic S4 use cases (make check-all seems fine),
>
> Good you checked, but you are right that that's not all enough to be sure:
>
> Checking error *messages* is not something we do often {not
> the least because you'd need to consider message translations
> and hence ensure you only check in case of English ...}.
>
>     > but I don't suppose a full patch would be dramatically different from the
>     > above.
>
> I agree: The patch looks to make sense to me, too,
> while I'm not entirely sure about the extra   call. = FALSE
>  (which I of course understand you'd prefer for the above example)
>
> Now I'd like to find an example that only uses packages with priority
>  base | Recommended
>
> Martin
>
> --
> Martin Maechler
> ETH Zurich  and  R Core team
>
>
>     > Mike C
>
>     > On Thu, Aug 10, 2023 at 12:39 PM Gabriel Becker <gabembecker using gmail.com> wrote:
>     >>
>     >> I just want to add my 2 cents that I think it would be very useful and beneficial to improve S4 to surface that information as well.
>     >>
>     >> More information about the way that the dispatch failed would be of great help in situations like the one Michael pointed out.
>     >>
>     >> ~G
>     >>
>     >> On Thu, Aug 10, 2023 at 9:59 AM Michael Chirico via R-devel <r-devel using r-project.org> wrote:
>     >>>
>     >>> I forwarded that along to the original reporter with positive feedback
>     >>> -- including the argument names is definitely a big help for cuing
>     >>> what exactly is missing.
>     >>>
>     >>> Would a patch to do something similar for S4 be useful?
>     >>>
>     >>> On Thu, Aug 10, 2023 at 6:46 AM Hadley Wickham <h.wickham using gmail.com> wrote:
>     >>> >
>     >>> > Hi Michael,
>     >>> >
>     >>> > I can't help with S4, but I can help to make sure this isn't a problem
>     >>> > with S7. What do you think of the current error message? Do you see
>     >>> > anything obvious we could do to improve?
>     >>> >
>     >>> > library(S7)
>     >>> >
>     >>> > dbGetQuery <- new_generic("dbGetQuery", c("conn", "statement"))
>     >>> > dbGetQuery(connection = NULL, query = NULL)
>     >>> > #> Error: Can't find method for generic `dbGetQuery(conn, statement)`:
>     >>> > #> - conn     : MISSING
>     >>> > #> - statement: MISSING
>     >>> >
>     >>> > Hadley
>     >>> >
>     >>> > On Wed, Aug 9, 2023 at 10:02 PM Michael Chirico via R-devel
>     >>> > <r-devel using r-project.org> wrote:
>     >>> > >
>     >>> > > I fielded a debugging request from a non-expert user today. At root
>     >>> > > was running the following:
>     >>> > >
>     >>> > > dbGetQuery(connection = conn, query = query)
>     >>> > >
>     >>> > > The problem is that they've named the arguments incorrectly -- it
>     >>> > > should have been [1]:
>     >>> > >
>     >>> > > dbGetQuery(conn = conn, statement = query)
>     >>> > >
>     >>> > > The problem is that the error message "looks" highly confusing to the
>     >>> > > untrained eye:
>     >>> > >
>     >>> > > Error in (function (classes, fdef, mtable)  :   unable to find an
>     >>> > > inherited method for function ‘dbGetQuery’ for signature ‘"missing",
>     >>> > > "missing"’
>     >>> > >
>     >>> > > In retrospect, of course, this makes sense -- the mis-named arguments
>     >>> > > are getting picked up by '...', leaving the required arguments
>     >>> > > missing.
>     >>> > >
>     >>> > > But I was left wondering how we could help users right their own ship here.
>     >>> > >
>     >>> > > Would it help to mention the argument names? To include some code
>     >>> > > checking for weird combinations of missing arguments? Any other
>     >>> > > suggestions?
>     >>> > >
>     >>> > > Mike C
>     >>> > >
>     >>> > > [1] https://github.com/r-dbi/DBI/blob/97934c885749dd87a6beb10e8ccb6a5ebea3675e/R/dbGetQuery.R#L62-L64