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

Thoughts/Updates for {gtsummary} themes #1580

Open
ddsjoberg opened this issue Dec 12, 2023 · 2 comments
Open

Thoughts/Updates for {gtsummary} themes #1580

ddsjoberg opened this issue Dec 12, 2023 · 2 comments

Comments

@ddsjoberg
Copy link
Owner

@larmarange and I recently had a chat about gtsummary themes. @larmarange indicated some students have confusion with gtsummary themes because they are specified differently than ggplot2 themes.

Admittedly, I do prefer the ggplot2 specification using ggplot() + geom_line() + themes(). But I am not sure how I could make this work in gtsummary. Essentially we would need to delay the calculation of everything until the print (or as_gt(), as_kable() etc) is called. That would require much more complex environment handling and a major re-write. While not impossible, I dont know if that added complexity is worth trying to unify ggplot and gtsummary notation. I'll keep thinking on it...maybe it will be worth it!

There was another thing I was thinking about regarding themes' current implementation that I do not love: If I set a theme at the top of a script and create a gtsummary table and save out the object, the theme is only active for the parts that are used during the construction of the table. If I were to load the saved gtsummary table in another script, and print it, any theme elements that control the defaults in the as_gt() function would not be activated unless I were to load the same theme into the env before printing. This is something that has confused users. Perhaps, we could instead save the "theme env" with the gtsummary object when the tbl_*() functions are called and the themes would be available for that tbl_*() object in any environment.

@larmarange
Copy link
Collaborator

larmarange commented Dec 13, 2023

If I may, some additional thoughts.

If the "theme env" is saved with the tbl object, you could consider two different options: set_gtsummary_theme() to change overall the theme elements for the future gtsummary objects that will be created, and a modify_gtsummary_theme() that could be applied on an existing table, e.g. tbl %>% modify_gtsummary_theme().

Another thought is the question of inheritance of theme elements. It exists in the code in many places, without being explicit. An example:

missing_text <- missing_text %||%
get_theme_element("tbl_svysummary-arg:missing_text") %||%
get_theme_element("tbl_summary-arg:missing_text",
default = translate_text("Unknown")
)

We could think about a better naming of the theme elements, showing the inheritance. An example:

  • pvalue_fun (overall default)
    • pvalue_fun.regression (value for tbl_regression())
    • pvalue_fun.summary (value for all summary functions)
      • pvalue_fun.summary.regular (value for tbl_summary() only)
      • pvalue_fun.summary.survey (value for tbl_svysummary() only)
      • pvalue_fun.summary.custom (value for tbl_custom_summary() only)

Then the code could be simplify in many places. We could simply call get_theme_element("pvalue_fun.summary.custom"). If this element is not defined (i.e. NULL) then get_theme_element() will look at "pvalue_fun.summary" and eventually at "pvalue_fun". With such naming (also used by ggplot2), inheritance will be explicit.

Another thought coming from ggplot::theme(): each theme element should be a named argument of set_gtsummary_theme() and modify_gtsummary_theme(). It would allow to use autocompletion to easily identify the name of a theme element, and improve the documentation of all of them.

@larmarange
Copy link
Collaborator

Also, an internal function called when package is loaded to fix the default values at once would be relevant, to avoid indicating default values within each function.

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