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

functions used in formula= argument in defData not available for functions defined out of global namespace #232

Closed
NielsKrarup opened this issue Jul 18, 2024 · 7 comments · Fixed by #233
Assignees
Labels

Comments

@NielsKrarup
Copy link

When using a function in the quoted formula= functionality it seems to me that this expects the function to be available not where defData is called but in global environment, or some other place.

I have added code reproducing the error below.

Thanks,

Niels

# insert reprex here

library(simstudy)
packageVersion("simstudy") #[1] ‘0.6.0’

hello <- function(){
  myinv <- function(x) {
    1/x
  }
  
  def <- defData(varname = "age", formula = 10, variance = 2,
                 dist = "normal")
  def <- defData(def, varname = "loginvage", formula = "log(myinv(age))",
                 variance = 0.1, dist = "normal")
  
  genData(5, def)
}

hello()
@kgoldfeld
Copy link
Owner

Yes - you are right, it does expect the function to be in the global environment. I will need to fix that. Thanks.

@kgoldfeld kgoldfeld self-assigned this Jul 18, 2024
@kgoldfeld kgoldfeld added the bug label Jul 18, 2024
@NielsKrarup
Copy link
Author

Also thanks for a nice package!

@assignUser
Copy link
Collaborator

assignUser commented Jul 19, 2024

Ah, this only works with variables but not with functions because we only use envvir from genData to evaluate the ..vars and don't pass it on to the actual formula evaluation. So we need to pass envvir into .evalWith as a new argument and add that everywhere it's called and add parent = envvir to list2env in .evalWith

e <- list2env(extVars)

@kgoldfeld
Copy link
Owner

That is exactly the approach I took. You can take a look to see if I did this correctly in the branch. It seems to work just fine, though there is some issue with genSurv and one other function that I need to resolve (it failed 4 tests) - none related to genData.

@assignUser
Copy link
Collaborator

Nice! Seems correct to me, maybe something local? You could open a PR and we can see what CI does?

@kgoldfeld
Copy link
Owner

I figured it out - there's an extra argument n, which was not used in some calls to .evalWih, so the environment was getting passed as n. I've fixed and pushed. Tomorrow, I will let you know when it is ready for merging - still need to update news, run checks, etc.

@kgoldfeld
Copy link
Owner

@NielsKrarup This issue has been fixed - and is available in the development version of simstudy. I will be submitting this version to CRAN in the next few days if you'd prefer to wait for that.

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