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

Update .assert_package()? #147

Closed
ddsjoberg opened this issue Feb 15, 2022 · 10 comments
Closed

Update .assert_package()? #147

ddsjoberg opened this issue Feb 15, 2022 · 10 comments

Comments

@ddsjoberg
Copy link
Collaborator

Should we update to use rlang::is_installed() and rlang::check_installed() internally? I think it'll improve the user experience when interactive.

  • rlang::is_installed() doesn't interact with the user. It simply returns TRUE or FALSE depending on whether the packages are installed.

  • In interactive sessions, rlang::check_installed() asks the user whether to install missing packages. If the user accepts, the packages are installed with pak::pkg_install() if available, or utils::install.packages() otherwise. If the session is non interactive or if the user chooses not to install the packages, the current evaluation is aborted.

@larmarange
Copy link
Owner

Seems good to me. Could you propose a PR?

@ddsjoberg
Copy link
Collaborator Author

Seems good to me. Could you propose a PR?

Will do!

@ddsjoberg
Copy link
Collaborator Author

ddsjoberg commented Feb 16, 2022

hey hey @larmarange !

I think the issue is pretty strange on R 3.5. The error is stemming from a call to reformulate() in {lme4}, and the error is unused argument (env = environment(f)). The env= argument wasn't added until 3.6.

You can see in the table below, the function was changed in 3.6 and the env arg was added.

image

I looked in the package's GH repo and they don't use a prefix stats::reformulate() when they reference the function (https://github.com/lme4/lme4/search?q=reformulate). So what I think is happening is that the package is installing properly on R 3.5, it's using the old base R of reformulate that doesn't have an env argument.

I think lme4 likely isn't preforming the tests to ensure there are no errors on the older versions of R it claims to support. (they also had a release of a new version last week) What do you think the next best steps are?

@larmarange
Copy link
Owner

larmarange commented Feb 16, 2022

I guess we have two options:

  • decide that now broom.helpers requires 3.6 as a minimum (but it will also impact gtsummary). Some how, we are now version 4.1
  • test R version in examples and test and do not run examples/tests requiring lme4 if R < 3.6

@ddsjoberg
Copy link
Collaborator Author

ddsjoberg commented Feb 16, 2022

I filed an issue on the lme4 repo lme4/lme4#664 . I vote would to just leave it as is for now with the erroring R Cmd Check. Hopefully lme4 will update either the call to reformulate(env=) or update the R versions that are supported, either of which will fix our errors upon the next release.

@larmarange
Copy link
Owner

OK in that case I propose to merge the PR but to keep this issue open. Is it fine with you?

@ddsjoberg
Copy link
Collaborator Author

that sounds good to me! i think they'll update soon enough. i may turn off my 3.5 checks for a while until they do

@ddsjoberg
Copy link
Collaborator Author

hey hey @larmarange ! The lme4 package had a new release 11 days ago, and should solve our R CMD Check problems!
https://cran.r-project.org/web/packages/lme4/index.html

@ddsjoberg
Copy link
Collaborator Author

ddsjoberg commented Apr 18, 2022

FYI, I think I am about 3 to 4 weeks out from a gtsummary release (https://github.com/ddsjoberg/gtsummary/milestone/14). Is there anything we could add to broom.helpers in the interim? Would you be ok with a broom.helpers release in a few weeks?

@larmarange
Copy link
Owner

Is there anything we could add to broom.helpers in the interim?

I do not see anything right now

Would you be ok with a broom.helpers release in a few weeks?

No problem

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