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

Running README example requires extra dependency #31

Closed
erikcs opened this issue Dec 11, 2021 · 5 comments
Closed

Running README example requires extra dependency #31

erikcs opened this issue Dec 11, 2021 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@erikcs
Copy link

erikcs commented Dec 11, 2021

After I installed medoutcon from github, a big handful of dependencies, and dependencies' dependencies were updated/installed. When I run the readme example I get the following, which may seem a bit quirky for a new user:

> # compute targeted minimum loss estimate of the interventional direct effect
> tmle_de <- medoutcon(W = example_data[, ..w_names],
+                      A = example_data$A,
+                      Z = example_data$Z,
+                      M = example_data[, ..m_names],
+                      Y = example_data$Y,
+                      effect = "direct",
+                      estimator = "tmle")
Error in requirePackages(private$.required_packages, why = class(self)[1],  : 
  For Lrnr_glm_fast please install the following packages: speedglm
@nhejazi nhejazi self-assigned this Dec 11, 2021
@nhejazi nhejazi added the bug Something isn't working label Dec 11, 2021
@nhejazi
Copy link
Owner

nhejazi commented Dec 11, 2021

Thanks for catching this and pointing it out. This seems to happen because the default algorithms used for nuisance estimation wrap the speedglm package via sl3 (https://github.com/nhejazi/medoutcon/blob/master/R/medoutcon.R#L81-L85), while speedglm is only listed in the Suggests of this package. This could be fixed by moving speedglm to Depends, I believe. The same is true of hal9001 also (https://github.com/nhejazi/medoutcon/blob/master/R/medoutcon.R#L86-L87).

@nhejazi
Copy link
Owner

nhejazi commented Dec 20, 2021

Following up on this to note that 4d5d15a in #34 moved both hal9001 and speedglm to the Depends slot. In testing the README example in a clean R session (R --vanilla), the example appears to work without issue; that is, I cannot reproduce the error. If you could spare a minute to try with that package version and confirm, that'd be helpful. If not, I'll try to run on another machine just to double check. Thanks again for your review of this package @erikcs.

@erikcs
Copy link
Author

erikcs commented Dec 21, 2021

Hi @nhejazi, confirming this works here.

@nhejazi
Copy link
Owner

nhejazi commented Dec 21, 2021

OK, I've now tried the example again on a completely fresh machine and believe that this issue has been resolved.

@nhejazi nhejazi closed this as completed Dec 21, 2021
@nhejazi
Copy link
Owner

nhejazi commented Dec 21, 2021

Great, thanks for confirming @erikcs!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants