rticulate / import

An Import Mechanism For R
https://import.rticulate.org
Other
222 stars 14 forks source link

Errors in imported script requires manual `detach` or R session restart #79

Closed hutch3232 closed 11 months ago

hutch3232 commented 11 months ago

When using import::from on an R script, an environment for that script is created within import:::scripts. That environment stores, among other things, a variable called __last_modified__ which is the modified data of the script. On subsequent calls to import::from on that same script, it will only source it again if the modify date of the script exceeds __last_modified__ (i.e., it has been changed/updated). This makes sense from an efficiency standpoint.

However, if an error is thrown while sourcing the script, the __last_modified__ variable remains, but the symbols from the sourced script do not. Therefore, correcting the issue that threw the error, and then re-running import::from will silently do nothing.

The possible use-case for this: if my script has some environment checking like, is a particular environmental variable set or a package loaded, then the error will be thrown, a user will make corrections to remedy it (add the env var, load the package, etc.) and then will attempt to re-run import::from unsuccessfully.

I'm working on a pull request, but I propose that, if there is an error when sourcing the script, to remove the environment from import:::scripts[.from].

Example: main.R

if(is.na(Sys.getenv("CONFIG", NA))) stop("CONFIG not set!")
foo <- function(){
  message("hi")
}
import::from("main.R", .all = TRUE)
# Error in eval(exprs[i], envir) : an error was thrown!
# "fix error" and re-run
Sys.setenv(CONFIG = "default")
import::from("main.R", .into = "new", .all = TRUE)
# nothing happens, no error and no foo
exists("foo")
# [1] FALSE
rm("main.R", envir = import:::scripts)
import::from("main.R", .into = "newtwo", .all = TRUE)
# this actually fails and I'm not sure why:
# Error in as.environment(where) : 
#   no item called "newtwo" on the search list
# restart R session
Sys.setenv(CONFIG = "default")
import::from("main.R", .into = "newtwo", .all = TRUE)
foo()
# hi
torfason commented 11 months ago

Fixed in dev and main, will be released to CRAN shortly.