import icon indicating copy to clipboard operation
import copied to clipboard

`import::from()` not working inside package function

Open brshallo opened this issue 2 years ago • 10 comments

I'm getting the following error when running import::from() from the debugger after loading a personal package.

import::from("purrr", "map", .into = "explictpackage:purrr", .character_only = TRUE)
#> Error in as.environment(where) : 
#>   no item called "explictpackage:purrr" on the search list

Strangely I don't get any error when running that line from the debugger after doing something like debugonce("mean"); mean(1:10). But I had to revert back to using the approach mentioned in #53 for attaching functions here: https://github.com/brshallo/funspotr/blob/issue-5/R/spot-funs.R#L104

brshallo avatar Jul 25 '22 10:07 brshallo

Well, that's a bummer! I hope that this was not a live error while presenting, and that it did not prevent you from submitting funspotter to CRAN.

Not sure what the solution here is though:

  • This works for me without errors
  • There seems to be a typo, "explictpackage:purrr" should probably be "explicitpackage:purr" (but that does not matter in my tests, though)
  • Do you have an expectation that the colon : in the namespace has special meaning? I don't think import has any understanding of colons, is this something that R itself expects and treats in a certain way? Do you get the error also when importing into an environment without the colon in the name?

torfason avatar Jul 26 '22 09:07 torfason

Hah, no, nothing like that. I held off on submitting funspotr for now because was re-writing the API for the talk which I've only just finished. (Though I will want to update this part before before submitting as w/o it I'm stuck with a pesky "note".)

I'll check that out, may have been a typo but yeah wouldn't think that would matter. No I don't have any expectation about colons. In {funspotr} I just use explicitpackage: for my internal labeling to identify packages in a file that are used like pkg::fun() rather than library(pkg); fun() but there's nothing about the colon or syntax in particular.

I was actually noticing the issue initially when using import::from() within callr::r() which first made me suspicious that there may be something weird happening when dealing with nested R processes but I'll try and open a minimal reprex here next week and also make sure that I'm not just doing something wrong in my set-up.

brshallo avatar Jul 26 '22 22:07 brshallo

Sounds like a plan!

Note that main (currently at v1.3.0.9003) has a somewhat deep internal change compared to the CRAN release. If you add a reprex, it would be great if you could note whether it happens with the CRAN release, the main branch, or both.

torfason avatar Jul 27 '22 08:07 torfason

I did not hear more about this. The v.1.3.1 release is now out, and I'm doing a bit of a cleanup. It has seemed to me that in general, the best way forward in complex situations regarding environment is to use import::here(). If that is not working to resolve this case, or if there is a neat solution that you could propose, feel free to reopen the issue.

torfason avatar Sep 25 '23 11:09 torfason

The problem with import::here() is I don't have the flexibility that comes with the .into arg.

This is the function where I'm using import::from() and the intended output:

attach_pkg_fun <- function(pkg_fun){
  pkg_nm <- pkg_fun$pkg
  fun_nm <- pkg_fun$fun
  pkg_nm_exp <- paste0("explicitpackage:", pkg_nm)
  
  import::from(pkg_nm, fun_nm, .into = pkg_nm_exp, .character_only = TRUE)
}

pkg_fun <- list(pkg = "purrr", fun = "map")
attach_pkg_fun(pkg_fun)

search()
#>  [1] ".GlobalEnv"            "explicitpackage:purrr" "package:stats"        
#>  [4] "package:graphics"      "package:grDevices"     "package:utils"        
#>  [7] "package:datasets"      "package:methods"       "Autoloads"            
#> [10] "tools:callr"           "package:base"

The above output is working as intended (when the function is in a standalone script) as you see "explicitpackage:purrr" has been added to the second position on the search path.

However if I try calling this from a (dev version of a) package, you'll see I get an error and the search path is not edited.

pkg_fun <- list(pkg = "purrr", fun = "map")
funspotr:::attach_pkg_fun(pkg_fun)
#> Error in as.environment(where): no item called "explicitpackage:purrr" on the search list

search()
#>  [1] ".GlobalEnv"        "package:stats"     "package:graphics" 
#>  [4] "package:grDevices" "package:utils"     "package:datasets" 
#>  [7] "package:methods"   "Autoloads"         "tools:callr"      
#> [10] "package:base"

This seems like some kind of environment issue that comes from it being in a package but haven't isolated it.

brshallo avatar Oct 06 '23 09:10 brshallo

OK, happy to leave this open. One question, is there a possibility import get around this using environment pointers rather than strings. As in:

> x = import::into(new.env(), "%<>%", .from = magrittr)
> import::into(.GlobalEnv, "%>%", "%$%", .from = magrittr)

I realize this would not solve anything directly, what I was thinking was either:

a. Use base R to get a pointer to the named environment (assign it to a variable), and then use this method to import into it. b. First import into a temporary environment and then use some base R commands to copy to the correct place.

torfason avatar Oct 06 '23 09:10 torfason

Putting things in .GlobalEnv won't work for my use case as I need it to identify the function as coming from that package.

I did try swapping import::from() for import::into() just to see if changing syntax would magically fix things but didn't seem to.

brshallo avatar Oct 06 '23 22:10 brshallo

Sorry, I was not suggesting to use the actual .GlobalEnv, just demonstrating this syntax. My question would be, does the following approach work for you:

my_package_local <- new.env()
import::from(dplyr, select, .into = my_package_local)
attach(my_package_local, name = "my_package") 
search()
#>  [1] ".GlobalEnv"        "my_package"        "package:stats"    
#>  [4] "package:graphics"  "package:grDevices" "package:utils"    
#>  [7] "package:datasets"  "package:methods"   "Autoloads"        
#> [10] "tools:callr"       "package:base"
ls(pos = "my_package")
#> [1] "select"

Knowing the answer would help diagnose whether this is import doing something strange, or whether it is external to that.

torfason avatar Oct 08 '23 18:10 torfason

Yes, that works. That's the approach I've been taking brshallo/funspotr/spot_funs/L119. I'd like to get rid of attach() though so that I no longer get a NOTE when running devtools::check() (even though my use of attach isn't actually dangerous as I do it from a separate R instance).

I think this likely points to some edge case issue with {import} though as well.

brshallo avatar Oct 08 '23 19:10 brshallo

OK, that definitely points to an edge case in import even though it does not give us any blueprint for fixing this. So this will stay open for now.

FWIW, this is how import gets rid of the NOTE:

  make_attach <- attach # Make R CMD check happy.
  if (use_into && !into_exists)
    make_attach(NULL, 2L, name = .into)

torfason avatar Oct 09 '23 10:10 torfason