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

qenv is not reproducible - library calls #93

Closed
Polkas opened this issue Nov 14, 2022 · 5 comments
Closed

qenv is not reproducible - library calls #93

Polkas opened this issue Nov 14, 2022 · 5 comments
Labels

Comments

@Polkas
Copy link
Contributor

Polkas commented Nov 14, 2022

qenv is not reproducible as not taking into account libraries from its environment.

library(ranger)
qq <- teal.code::new_qenv() |> teal.code::eval_code("a <- ranger")
# code is not reproducible
teal.code::get_code(qq)
# But the code is evaluated properly
qq[["a"]]

It will be the best to not take blindly all loaded packages in the Session.
Possible we should not use the parent of Globalenv for default environment, and always demand library calls in the code.

@gogonzo
Copy link
Contributor

gogonzo commented Nov 15, 2022

@Polkas yes I agree that packages from the session info is too much to show and many of the packages in the session are not related to execution within qenv. I think this is more teal issue than teal.code, as I don't really see the option within qenv to either check if @code has relevant library calls or if it's reproducible. So, I'd like to move it to teal if you agree.

qenv is reproducible or not depending on what user push. When one initializes qenv it would be a good idea to include library calls also and this is what we do in teal - but I agree, shole sessionInfo is too much. What to do then?

  • in teal we can add library calls which are relevant to the code we are passing to tlg-modules
  • in tlg-module we can add in the beginning library calls which are needed in the module

Any other ideas?

@Polkas
Copy link
Contributor Author

Polkas commented Nov 15, 2022

TL;DR I do not relate this issue in any way to teal or teal apps. qenv should always provide reproducible code or a proper warnings if not. The internal qenv libraries are NEUTRAL for a qenv search.

Because of the default new_env <- rlang::env_clone(env, parent = parent.env(.GlobalEnv)) we are on purpose offering all loaded in the session packages for the qenv. So we open the door for lack of reproducibility as later are not considered when get_code is used. Then it is a clear qenv problem for me. More than that we could not use libraries inside the qenv.

ranger is not loaded as the primary parent.env(.GlobalEnv) not known the ranger then.

# Restart the session
qq <- teal.code::new_qenv() |> teal.code::eval_code("library(ranger); a <- ranger")
# error
teal.code::get_code(qq)
# error
qq[["a"]]

to make it working we need such trick:

ee <- new.env(parent = parent.env(globalenv()))

ee$library <- function(...) {
  mc <- match.call()
  mc[[1]] <- quote(base::library)
  eval(mc, envir = globalenv())
  this_env <- parent.frame()
  if(!identical(this_env, globalenv())) 
  parent.env(this_env) <- parent.env(globalenv())
}

@gogonzo
Copy link
Contributor

gogonzo commented Nov 15, 2022

@Polkas yes true. If library(xx) call will be really executed in eval_code, then I think we should do the shuffle or we don't have to case as package::xx will be in the search path in the next eval_code.
Let me think more about this so I can prepare comprehensive answer

@Polkas
Copy link
Contributor Author

Polkas commented Nov 16, 2022

PROBLEM 1 lack of library calls in get_code
PROBLEM 2 very limited way of adding library calls to qenv (connected with PROBLEM1)

The simplest solution is to ask end users to add all proper library calls in the one separated eval_code but then they will not be on the top of the code. library calls have to be separated from code which using them, both need to be in different eval_code. Please check examples.

IDEAS to think about:

  • callr::r_bg for isolated environment. callr::r_bg. we investigated it and seems we will lose a lot of performance.
  • A new method like add_libraries which prepend all code, so is at the beginning. We could compare what is added and what is in the sessionInfo so provide a proper warning.
  • A new argument to new_qenv like packages so sb could add a vector of dependencies which could be added and validated.

NOT WORK

# Restart the session
qq <- teal.code::new_qenv() |> teal.code::eval_code("library(ranger); a <- ranger")
# error
teal.code::get_code(qq)
# error
qq[["a"]]

WORK

# Restart the session
qq <- teal.code::new_qenv() |> teal.code::eval_code("library(ranger)") |> teal.code::eval_code("a <- ranger")
teal.code::get_code(qq)
qq[["a"]]

NOT WORK

# Restart the session
qq <- teal.code::new_qenv(code = "library(ranger)", env = new.env(parent = parent.env(.GlobalEnv))) |> teal.code::eval_code("a <- ranger")
# error
teal.code::get_code(qq)
# error
qq[["a"]]

@nikolas-burkoff
Copy link
Contributor

Also related insightsengineering/teal#593

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

No branches or pull requests

3 participants