load_all() glitch with unexported S3 methods

[hongooi73]

I'm wondering if I've messed up my installation somehow, as this seems to be a fairly fundamental issue.

I'm using R 4.0.2 on Windows, with devtools 2.3.0. When I run load_all() on a package under development, R doesn't find S3 methods that aren't exported (ie, aren't present in the NAMESPACE file).

Reproducible example:

• Create a new directory, and in that directory, run devtools::create(".")
• Create a file R/file1.R containing:

func <- function(x, ...)
{
    UseMethod("func")
}

func.cls <- function(x, ...)
{
    x
}

#' @export
make_cls <- function(x)
{
    structure(list(x=x), class="cls")
}

• Run devtools::document() and devtools::load_all()
• Run the following:

obj <- make_cls(42)
func(obj)

Error in UseMethod("func") : 
  no applicable method for 'func' applied to an object of class "cls"

The error goes away if I add @export directives to the func and func.cls functions, redocument and reload. This behaviour wasn't present under R 3.6.x.

[hongooi73]

My full session info:

R version 4.0.2 (2020-06-22)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 10 x64 (build 19041)

Matrix products: default

locale:
[1] LC_COLLATE=English_Australia.1252  LC_CTYPE=English_Australia.1252   
[3] LC_MONETARY=English_Australia.1252 LC_NUMERIC=C
[5] LC_TIME=English_Australia.1252

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] AzureCosmosR_0.0.1 devtools_2.3.0     usethis_1.6.1

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.4.6      compiler_4.0.2    AzureRMR_2.3.6    prettyunits_1.1.1
 [5] remotes_2.1.1     tools_4.0.2       uuid_0.1-4        testthat_2.3.2   
 [9] digest_0.6.25     pkgbuild_1.0.8    pkgload_1.0.2     jsonlite_1.6.1   
[13] memoise_1.1.0     rlang_0.4.6       AzureAuth_1.2.5   cli_2.0.2        
[17] rstudioapi_0.11   xfun_0.13         httr_1.4.1        withr_2.2.0      
[21] stringr_1.4.0     roxygen2_7.1.1    knitr_1.28        xml2_1.3.2       
[25] vctrs_0.3.1       askpass_1.1       desc_1.2.0        fs_1.4.1
[29] rappdirs_0.3.1    rprojroot_1.3-2   glue_1.4.1        R6_2.4.1
[33] processx_3.4.2    fansi_0.4.1       sessioninfo_1.1.1 callr_3.4.3
[37] purrr_0.3.4       magrittr_1.5      backports_1.1.8   ps_1.3.3
[41] ellipsis_0.3.1    assertthat_0.2.1  stringi_1.4.6     openssl_1.4.1
[45] crayon_1.3.4

[jimhester]

This is just how R works with registered methods after R 4.0. You need to explicitly register the S3 methods with #' @export, even if the generic is not exported, or they will not be found by R.

[hongooi73]

Thanks for the info @jimhester. It might be good to mention this in the docs for pkgload::load_all, that S3 methods must still be registered to be found. Right now it just says

load_all by default will copy all objects (not just the ones listed as exports) to the package environment. This is useful during development because it makes all objects easy to access.

which is technically true, but as you can see, a bit misleading in the case of S3 methods.

[snystrom]

This is tripping me up, too. I just want to confirm about some behavior since I'm having issues finding documentation on this change. I get the following behavior:

# generics.R
input <- function(x) {
  UseMethod("input")
}

input.character <- function(x){
  print(x)
}

my_input <- function(x){
  input(x)
}

# Calling input("test") FAILS
# calling my_input("test") WORKS!!!! WTF!?!

devtools::load_all()
input("test")
#> Error in input("test"): could not find function "input"
my_input("test")
#> Error in my_input("test"): could not find function "my_input"

Calling the generic directly fails, but calling a function wrapping the generic succeeds. I guess I'm struggling to reason about when #' @export is needed here. This certainly makes it very difficult to debug internal S3 methods (Obviously, adding #' @export fixes the issue with load_all()).

Anyway, is there some reading material on this? My googling is landing me on pre-R 4.0 references. I understand why input() isn't working, but WRE doesn't quite explain the behavior I'm describing with my_input().

[jimhester]

It is mentioned in the R news for R 4.0

S3 method lookup now by default skips the elements of the search path between the global and base environments.

This would only work previously because unregistered methods defined in the package were on the search path in between the global environment and the base environment. But R 4.0+ does not look there anymore for S3 methods.

[snystrom]

Ah, got it. I misinterpreted that news line. Thanks!

[MilesMcBain]

Just to clarify, internal S3 methods do not need to be exported. They only need an S3method() directive in the NAMESPACE which can be generated by the poorly named @exportS3Method {roxygen2} tag which does not add an export directive for the method.

[lionel-]

The standard tag to use is @export not @exportS3Method.

[MilesMcBain]

But doesn't that export the presumably internal method?

[lionel-]

It only registers it. With roxygen you can't actually "export" an S3 method unless you use a raw namespace directive. The distinction between exporting and registering is important for technical users but not so much for regular users. So @export is a single tag that does the right thing depending on the context.

[jimhester]

yeah to be explicit #' @export when used on a S3 method just registers the method by writing S3method(foo, bar) in the NAMESPACE, it does not write export(foo.bar) as well.

[lionel-]

I think the confusion could be cleared up by adding a warning in roxygen when S3 methods are not registered with @export. I've opened an issue: r-lib/roxygen2#1175.