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

Support use of non-existent cells_*() within tab_style(locations = ...) #1608

Open
2 tasks done
christopherkenny opened this issue Mar 19, 2024 · 0 comments
Open
2 tasks done
Assignees

Comments

@christopherkenny
Copy link
Contributor

christopherkenny commented Mar 19, 2024

Prework

Motivation

Like for ggplot2, it would be nice to have full theme() style functions. It's not hard to do that for some features, like column labels or footnotes. However, others error out when certain locations are passed to tab_style() via calls to cells_*() that don't currently exist.

Ideally, this could be made consistent, allowing for more relaxed passing of these themes. A while back, I started working on some examples of this, but the errors mean we have to (1) do some fancy checking of what elements to pass, (2) break it up into many subfunctions, or (3) ignore elements which must be present to include styling. A table showing the type of thing that would be nice to have is below.

Example table corresponding to link

This code runs now, but would be ideal to style all elements.

gt::exibble |>
  gt::gt() |>
  gt_theme_barbie()

image

The specifics of this issue are laid out below, with reproducible examples. The below solution fixes the issue, as far as I can tell, and I don't see anything breaking. Happy to provide a PR if that would be more helpful. For what it's worth, I know this could be intentional, so I hope this doesn't waste your time if it is.

Proposal

Currently, tab_style() is inconsistent with formatting non-existent elements of a gt table. For some calls to cells_*(), it runs whether that element exists or not. For example, the following runs without issue:

library(gt)
gt::exibble |> 
  gt::gt() |> 
  gt::tab_style(
    style = gt::cell_text(color = 'orange'),
    locations = gt::cells_column_spanners()
  ) 

Nothing visibly changes, but the call to gt::cells_column_spanners() runs without issue.

In contrast, running the following fails:

gt::exibble |> 
  gt::gt() |> 
  #gt::grand_summary_rows(columns = where(is.numeric), fns = 'mean') |> 
  gt::tab_style(
    style = gt::cell_borders(sides = c('top', 'bottom'), color = 'black'),
    location = gt::cells_grand_summary()
  )
rlang::trace(drop = FALSE) for above
<error/rlang_error>
Error:
! `everything()` must be used within a *selecting* function.
ℹ See ?tidyselect::faq-selection-context for details.
---
Backtrace:
     ▆
  1. ├─gt::tab_style(...)
  2. │ ├─gt:::set_style(loc = loc, data = data, style = style) at gt/R/tab_create_modify.R:3448:5
  3. │ └─gt:::set_style.cells_grand_summary(loc = loc, data = data, style = style) at gt/R/tab_create_modify.R:3498:3
  4. │   └─gt:::add_grand_summary_location_row(...) at gt/R/tab_create_modify.R:3676:3
  5. │     └─gt:::resolve_vector_i(expr = !!loc$rows, vector = id_vals, item_label = "grand summary row") at gt/R/location_methods.R:214:3
  6. │       ├─base::which(...) at gt/R/resolver.R:475:3
  7. │       └─gt:::resolve_vector_l(...)
  8. │         ├─tidyselect::with_vars(...) at gt/R/resolver.R:458:3
  9. │         └─rlang::eval_tidy(expr = quo, data = NULL)
 10. └─tidyselect::everything()
 11.   ├─vars %||% peek_vars(fn = "everything")
 12.   └─tidyselect::peek_vars(fn = "everything")
 13.     └─cli::cli_abort(...)
 14.       └─rlang::abort(...)

(As expected, uncommenting the above line does fix the issue and it runs without issue.)

Only four cells_*() functions fail when the targeted piece does not exist:

  • cells_grand_summary()
  • cells_stub_grand_summary()
  • cells_stub_summary()
  • cells_summary()

Upon investigation, this is tied to two issues.

  1. resolve_vector_i() cannot evaluate on a NULL vector. This causes a tidyselection error re everything()
  2. Upon updating resolve_vector_i() to return integer(0) when is.null(vector), there is a specific check for 0 rows to abort.

If we patch (1) and remove the following code for (2), then things seem to work without issue.

 if (length(rows) == 0) {
   cli::cli_abort(c(
     "The location requested could not be resolved.",
     "*" = "Review the expression provided as `rows`."
   ))
 }

In context, I've put these minimal changes in one commit here. I can create a PR in (and actually remove the comments rather than commenting them like that).

Related issues

There is no perfectly related issues here, but #1438 suggests to me that the order of evaluation here could be meaningful, so the check in (2) above is intentional and possibly necessary. (Though I'm not deep enough into the weeds here to say if it really matters or is just a "oh we should check that" type check.)

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

No branches or pull requests

2 participants