ddsjoberg / gtsummary

Presentation-Ready Data Summary and Analytic Result Tables
http://www.danieldsjoberg.com/gtsummary
Other
1.04k stars 114 forks source link

Failure with dev glue #1989

Closed jennybc closed 1 hour ago

jennybc commented 3 hours ago

This issue is inspired by doing revdep checks for glue. In the next release of glue, which is very soon, glue::glue() will error when .envir is not an actual environment. .envir has always been documented to be an environment and I've been working to make this actually true.

I last did revdep checks for this in December 2023 and I opened issue or PRs everywhere that I could. So gtsummary's usage of this pattern must have arisen more recently.

glue_data() does officially accept something "list-ish" as .x. So you should switch to glue_data(.x = list_ish_thing, ...) as opposed to glue(.envir = list_ish_thing, ...) in many places here in this package. I haven't made a PR since there seem to be so many locations that need tweaking; I don't trust myself to get this right, quickly, in someone else's package.

Relevant links in glue:

https://github.com/tidyverse/glue/issues/308 https://github.com/tidyverse/glue/commit/e2b74ff17704261b5a7da25b4ebd2dec94740764 https://github.com/tidyverse/glue/issues/341

Here's what I'm currently seeing for gtsummary in glue's revdep checks: https://github.com/tidyverse/glue/blob/main/revdep/problems.md#gtsummary

I think this breakage has some other indirect effects on gtsummary's revdeps, such as gtreg (cc @shannonpileggi ).

ddsjoberg commented 2 hours ago

Thank you @jennybc for the heads up!

ddsjoberg commented 1 hour ago

Welp, that was a surprisingly large number of off-label uses of glue(.envir) 😬 😆

jennybc commented 1 hour ago

Ha! Yeah that's why you didn't get a PR from me 😄