find calls for box package use

[stefanlinner]

I noticed that the function translation doesn't work (yet) in combination with the box package:

box::use(
  duckplyr
)
#> The duckplyr package is configured to fall back to dplyr when it encounters an
#> incompatibility. Fallback events can be collected and uploaded for analysis to
#> guide future development. By default, no data will be collected or uploaded.
#> → Run `duckplyr::fallback_sitrep()` to review the current settings.

options(duckdb.materialize_message = FALSE)

# Triggers fallback
mtcars_rows <- 
  mtcars |> 
  duckplyr$as_duckplyr_tibble() |> 
  duckplyr$mutate(row_num = duckplyr$row_number())
#> The duckplyr package is configured to fall back to dplyr when it encounters an
#> incompatibility. Fallback events can be collected and uploaded for analysis to
#> guide future development. By default, no data will be collected or uploaded.
#> ℹ A fallback situation just occurred. The following information would have been
#>   recorded:
#>   {"version":"0.4.1","message":"Can't translate function
#>   `duckplyr$row_number`.","name":"mutate","x":{"...1":"numeric","...2":"numeric","...3":"numeric","...4":"numeric","...5":"numeric","...6":"numeric","...7":"numeric","...8":"numeric","...9":"numeric","...10":"numeric","...11":"numeric"},"args":{"dots":{"...12":"...13$...14()"},".by":"NULL",".keep":["all","used","unused","none"]}}
#> → Run `duckplyr::fallback_sitrep()` to review the current settings.
#> → Run `Sys.setenv(DUCKPLYR_FALLBACK_COLLECT = 1)` to enable fallback logging,
#>   and `Sys.setenv(DUCKPLYR_FALLBACK_VERBOSE = TRUE)` in addition to enable
#>   printing of fallback situations to the console.
#> → Run `duckplyr::fallback_review()` to review the available reports, and
#>   `duckplyr::fallback_upload()` to upload them.
#> ℹ See `?duckplyr::fallback()` for details.
#> ℹ This message will be displayed once every eight hours.
#> Error processing with relational.
#> Caused by error in `rel_find_call()`:
#> ! Can't translate function `duckplyr$row_number`.

# Works as intended
mtcars_rows <- 
  mtcars |> 
  duckplyr$as_duckplyr_tibble() |> 
  duckplyr$mutate(row_num = duckplyr::row_number())

^{Created on 2024-08-07 with reprex v2.1.1}

Session info

sessionInfo()
#> R version 4.2.3 (2023-03-15 ucrt)
#> Platform: x86_64-w64-mingw32/x64 (64-bit)
#> Running under: Windows 10 x64 (build 22631)
#> 
#> Matrix products: default
#> 
#> locale:
#> [1] LC_COLLATE=German_Germany.utf8  LC_CTYPE=German_Germany.utf8   
#> [3] LC_MONETARY=German_Germany.utf8 LC_NUMERIC=C                   
#> [5] LC_TIME=German_Germany.utf8    
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> loaded via a namespace (and not attached):
#>  [1] rstudioapi_0.16.0 knitr_1.48        magrittr_2.0.3    tidyselect_1.2.1 
#>  [5] R6_2.5.1          rlang_1.1.4       fastmap_1.2.0     fansi_1.0.6      
#>  [9] dplyr_1.1.4       tools_4.2.3       xfun_0.46         utf8_1.2.4       
#> [13] DBI_1.2.3         cli_3.6.3         withr_3.0.0       htmltools_0.5.8.1
#> [17] yaml_2.3.10       digest_0.6.36     tibble_3.2.1      lifecycle_1.0.4  
#> [21] box_1.2.0         duckdb_1.0.0      vctrs_0.6.5       fs_1.6.4         
#> [25] glue_1.7.0        evaluate_0.24.0   rmarkdown_2.27    reprex_2.1.1     
#> [29] compiler_4.2.3    duckplyr_0.4.1    pillar_1.9.0      generics_0.1.3   
#> [33] collections_0.3.7 jsonlite_1.8.8    pkgconfig_2.0.3

I suggest adding the $ operator to the "fully qualified" package + name case. I also added an extra check that length(name) == 3 to avoid some (unexpected) nested special cases like list$packagename$functionname().

[krlmlr]

Thanks. While this change works, it also opens up too many possibilities for my taste.

Can we, for the case of $, test that pkg$fun actually evaluates to pkg::fun ?

To be fair, the current code only works if :: really is from the base package. Can/should we check this too?

[stefanlinner]

You are right. After playing around a bit, I found some examples that work, but shouldn't work in my opinion:

mtcars_rows <- 
  mtcars |> 
  duckplyr::as_duckplyr_tibble() |> 
  # There is no `test` package
  duckplyr::mutate(row_num = test::row_number())
#> The duckplyr package is configured to fall back to dplyr when it encounters an
#> incompatibility. Fallback events can be collected and uploaded for analysis to
#> guide future development. By default, no data will be collected or uploaded.
#> → Run `duckplyr::fallback_sitrep()` to review the current settings.

mtcars_rows <- 
  mtcars |> 
  duckplyr::as_duckplyr_tibble() |> 
  # There is no `test` package and `box` is not used
  duckplyr::mutate(row_num = test$row_number())

^{Created on 2024-08-19 with reprex v2.1.0}

However, what should be possible, as this is a common use case with box:

box::use(
  test = duckplyr
)
#> The duckplyr package is configured to fall back to dplyr when it encounters an
#> incompatibility. Fallback events can be collected and uploaded for analysis to
#> guide future development. By default, no data will be collected or uploaded.
#> → Run `duckplyr::fallback_sitrep()` to review the current settings.

mtcars_rows <- 
  mtcars |> 
  test$as_duckplyr_tibble() |> 
  test$mutate(row_num = test$row_number())

^{Created on 2024-08-19 with reprex v2.1.0}

So I think we should check for the following things:

1. :: is actually from base
2. pkg namespace / object exists
3. fun is part of the package namespace / object
4. make sure that fun is actually used from the provided namespace / object

What do you think?

[krlmlr]

Thanks. Let's wait a bit, I have a code locally that handles the test:: problem.

For the box::use(test = duckplyr) case, how do you intend to check that test actually maps to duckplyr? Can we make this conditional on isNamespaceLoaded("box") ?

[stefanlinner]

I was thinking of something like this at the top of rel_find_call:

name <- as.character(fun)

  if (length(name) == 3) {

    pkg <- name[[2]]
    fun_name <- name[[3]]

    if (name[[1]] == "::") {

      # ... :: case ...

    } else if (name[[1]] == "$") {

      # Check that box is used
      if(!isNamespaceLoaded("box")){
        cli::cli_abort("Meaningful error")
      }

      # Check that pkg object is available in GlobalEnvironment
      if(!exists(pkg, envir = env)) {
        cli::cli_abort("Meaningful error")
      }

      fun_box <- eval(fun)
      # Which namespace is the box object actually mapped to
      pkg_ns <- getNamespaceName(environment(fun_box))[["name"]]
      fun_mapped <- get(fun_name, envir = asNamespace(pkg_ns))

      # Check that box function and mapped function are identical
      if(!identical(fun_box, fun_mapped)) {
        cli::cli_abort("Meaningful error")
      }

      return(c(pkg_ns, fun_name))

    }

  }

So in the end the package name to which test is mapped to will be returned and not test itself.