Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

import::from() not working inside package function #71

Open
brshallo opened this issue Jul 25, 2022 · 10 comments
Open

import::from() not working inside package function #71

brshallo opened this issue Jul 25, 2022 · 10 comments

Comments

@brshallo
Copy link
Contributor

brshallo commented Jul 25, 2022

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

@torfason
Copy link
Collaborator

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?

@brshallo
Copy link
Contributor Author

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.

@torfason
Copy link
Collaborator

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
Copy link
Collaborator

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 torfason closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2023
@brshallo
Copy link
Contributor Author

brshallo commented Oct 6, 2023

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.

@torfason torfason reopened this Oct 6, 2023
@torfason
Copy link
Collaborator

torfason commented Oct 6, 2023

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.

@brshallo
Copy link
Contributor Author

brshallo commented Oct 6, 2023

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.

@torfason
Copy link
Collaborator

torfason commented Oct 8, 2023

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.

@brshallo
Copy link
Contributor Author

brshallo commented Oct 8, 2023

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.

@torfason
Copy link
Collaborator

torfason commented Oct 9, 2023

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants