Skip to content

Commit

Permalink
Merge pull request #1770 from olivroy/issue1535
Browse files Browse the repository at this point in the history
Refactor resolver to nudge to adding row names to columns + improve error message + refactor tests
  • Loading branch information
rich-iannone authored Jul 12, 2024
2 parents 7dae195 + 7c0dc67 commit 37ad5a6
Show file tree
Hide file tree
Showing 11 changed files with 232 additions and 215 deletions.
4 changes: 3 additions & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# gt (development version)

* Fixed an issue where `md("")` would fail in Quarto (@olivroy, #1769).
* `tab_row_group()` gives a more precise error message when `rows` can't be resolved correctly (#1535). (@olivroy, #1770)

* Fixed an issue where `md("")` would fail in Quarto. (@olivroy, #1769)

# gt 0.11.0

## New features
Expand Down
20 changes: 18 additions & 2 deletions R/resolver.R
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,23 @@ normalize_resolved <- function(

unknown_resolved <- setdiff(resolved, item_names)
if (length(unknown_resolved) != 0) {
resolver_stop_on_character(item_label = item_label, unknown_resolved = unknown_resolved, call = call)
if (all(is.na(item_names)) && item_label == "row") {
# Send a more informative message when the gt table has no rows
# rows need to be initialized with `rownames_to_stub = TRUE` or with `rowname_col = <column>`
# Issue #1535 (Override the resolver default error message.)

cli::cli_abort(c(
"Can't find named rows in the table",
"i" = "In {.help [gt()](gt::gt)}, use {.code rownames_to_stub = TRUE} or specify {.arg rowname_col} to initialize row names in the table."
), call = call)
}

# Potentially use arg_match() when rlang issue is solved?
resolver_stop_on_character(
item_label = item_label,
unknown_resolved = unknown_resolved,
call = call
)
}
resolved <- item_names %in% resolved

Expand Down Expand Up @@ -693,7 +709,7 @@ resolver_stop_on_character <- function(
# Specify cli pluralization
l <- length(unknown_resolved)
cli::cli_abort(
"{item_label}{cli::qty(l)}{?s} {.code {unknown_resolved}}
"{item_label}{cli::qty(l)}{?s} {.str {unknown_resolved}}
do{?es/} not exist in the data.",
call = call
)
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/tab_footnote.md
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@
Error in `tab_footnote()`:
! Can't add footnote "First data cell.".
Caused by error in `cells_column_spanners()`:
! Spanner `valuer` does not exist in the data.
! Spanner "valuer" does not exist in the data.
Code
tab_footnote(start_gt, footnote = "First data cell.", locations = cells_column_spanners(
3))
Expand All @@ -595,7 +595,7 @@
Error in `tab_footnote()`:
! Can't add footnote "Footnote error.".
Caused by error in `cells_body()`:
! Row `Mazda RX7` does not exist in the data.
! Row "Mazda RX7" does not exist in the data.

# tab_footnote() errors well when it can't resolve location

Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/tab_remove.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,12 @@
rm_spanners(t_sp, "span2")
Condition
Error in `rm_spanners()`:
! Spanner `span2` does not exist in the data.
! Spanner "span2" does not exist in the data.
Code
rm_spanners(t_sp, c("span1", "span2", "span3"))
Condition
Error in `rm_spanners()`:
! Spanners `span2` and `span3` do not exist in the data.
! Spanners "span2" and "span3" do not exist in the data.

# Table footnotes can be removed using `rm_footnotes()`

Expand Down
31 changes: 6 additions & 25 deletions tests/testthat/_snaps/tab_style.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
# tab_style errors if problems occur
# tab_style() errors if locations can't be resolved

Code
data %>% tab_style(style = list(cell_fill(color = "green"), cell_text(color = "white")),
tab_style(data, style = list(cell_fill(color = "green"), cell_text(color = "white")),
locations = cells_summary(groups = "Mercs", columns = starts_with("x"), rows = 2))
Condition
Error in `tab_style()`:
! Failed to style the summary of the table.
Caused by error in `cells_summary()`:
! The location requested could not be resolved.
* Review the expression provided as `columns`.

---

Code
data %>% tab_style(style = list(cell_fill(color = "green"), cell_text(color = "white")),
tab_style(data, style = list(cell_fill(color = "green"), cell_text(color = "white")),
locations = cells_summary(groups = "Mercs", columns = starts_with("m"), rows = starts_with(
"x")))
Condition
Expand All @@ -22,23 +19,17 @@
Caused by error in `cells_summary()`:
! The location requested could not be resolved.
* Review the expression provided as `rows`.

---

Code
data %>% tab_style(style = list(cell_fill(color = "green"), cell_text(color = "white")),
tab_style(data, style = list(cell_fill(color = "green"), cell_text(color = "white")),
locations = cells_column_labels(`non existent`))
Condition
Error in `tab_style()`:
! Failed to style the column labels of the table.
Caused by error in `cells_column_labels()`:
! Can't select columns that don't exist.
x Column `non existent` doesn't exist.

---

Code
data %>% tab_style(style = list(cell_fill(color = "green"), cell_text(color = "white")),
tab_style(data, style = list(cell_fill(color = "green"), cell_text(color = "white")),
locations = cells_column_spanners(2))
Condition
Error in `tab_style()`:
Expand Down Expand Up @@ -80,17 +71,7 @@
Error in `tab_style()`:
! Failed to style the body of the table.
Caused by error in `cells_body()`:
! Row `Mazda RX7` does not exist in the data.

# tab_row_group warns when others_label is not empty

Code
a_gt <- data %>% tab_row_group(others_label = "Others1")
Condition
Warning:
Since gt v0.3.0 the `others_label` argument has been deprecated.
* Use `tab_options(row_group.default_label = <label>)` to set this label.
This warning is displayed once every 8 hours.
! Row "Mazda RX7" does not exist in the data.

# Using fonts in `from_column()` works within `cell_*()` fns

Expand Down
31 changes: 31 additions & 0 deletions tests/testthat/_snaps/table_parts.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,34 @@
Output
[1] "<table class=\"gt_table\" data-quarto-disable-processing=\"false\" data-quarto-bootstrap=\"false\">\n <thead>\n <tr class=\"gt_heading\">\n <td colspan=\"11\" class=\"gt_heading gt_title gt_font_normal gt_bottom_border\" style>test title</td>\n </tr>\n \n <tr class=\"gt_col_headings\">\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"mpg\">mpg</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"cyl\">cyl</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"disp\">disp</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"hp\">hp</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"drat\">drat</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"wt\">wt</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"qsec\">qsec</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"vs\">vs</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"am\">am</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"gear\">gear</th>\n <th class=\"gt_col_heading gt_columns_bottom_border gt_right\" rowspan=\"1\" colspan=\"1\" scope=\"col\" id=\"carb\">carb</th>\n </tr>\n </thead>\n <tbody class=\"gt_table_body\">\n <tr><td headers=\"mpg\" class=\"gt_row gt_right\">21.0</td>\n<td headers=\"cyl\" class=\"gt_row gt_right\">6</td>\n<td headers=\"disp\" class=\"gt_row gt_right\">160</td>\n<td headers=\"hp\" class=\"gt_row gt_right\">110</td>\n<td headers=\"drat\" class=\"gt_row gt_right\">3.90</td>\n<td headers=\"wt\" class=\"gt_row gt_right\">2.620</td>\n<td headers=\"qsec\" class=\"gt_row gt_right\">16.46</td>\n<td headers=\"vs\" class=\"gt_row gt_right\">0</td>\n<td headers=\"am\" class=\"gt_row gt_right\">1</td>\n<td headers=\"gear\" class=\"gt_row gt_right\">4</td>\n<td headers=\"carb\" class=\"gt_row gt_right\">4</td></tr>\n <tr><td headers=\"mpg\" class=\"gt_row gt_right\">21.0</td>\n<td headers=\"cyl\" class=\"gt_row gt_right\">6</td>\n<td headers=\"disp\" class=\"gt_row gt_right\">160</td>\n<td headers=\"hp\" class=\"gt_row gt_right\">110</td>\n<td headers=\"drat\" class=\"gt_row gt_right\">3.90</td>\n<td headers=\"wt\" class=\"gt_row gt_right\">2.875</td>\n<td headers=\"qsec\" class=\"gt_row gt_right\">17.02</td>\n<td headers=\"vs\" class=\"gt_row gt_right\">0</td>\n<td headers=\"am\" class=\"gt_row gt_right\">1</td>\n<td headers=\"gear\" class=\"gt_row gt_right\">4</td>\n<td headers=\"carb\" class=\"gt_row gt_right\">4</td></tr>\n <tr><td headers=\"mpg\" class=\"gt_row gt_right\">22.8</td>\n<td headers=\"cyl\" class=\"gt_row gt_right\">4</td>\n<td headers=\"disp\" class=\"gt_row gt_right\">108</td>\n<td headers=\"hp\" class=\"gt_row gt_right\">93</td>\n<td headers=\"drat\" class=\"gt_row gt_right\">3.85</td>\n<td headers=\"wt\" class=\"gt_row gt_right\">2.320</td>\n<td headers=\"qsec\" class=\"gt_row gt_right\">18.61</td>\n<td headers=\"vs\" class=\"gt_row gt_right\">1</td>\n<td headers=\"am\" class=\"gt_row gt_right\">1</td>\n<td headers=\"gear\" class=\"gt_row gt_right\">4</td>\n<td headers=\"carb\" class=\"gt_row gt_right\">1</td></tr>\n <tr><td headers=\"mpg\" class=\"gt_row gt_right\">21.4</td>\n<td headers=\"cyl\" class=\"gt_row gt_right\">6</td>\n<td headers=\"disp\" class=\"gt_row gt_right\">258</td>\n<td headers=\"hp\" class=\"gt_row gt_right\">110</td>\n<td headers=\"drat\" class=\"gt_row gt_right\">3.08</td>\n<td headers=\"wt\" class=\"gt_row gt_right\">3.215</td>\n<td headers=\"qsec\" class=\"gt_row gt_right\">19.44</td>\n<td headers=\"vs\" class=\"gt_row gt_right\">1</td>\n<td headers=\"am\" class=\"gt_row gt_right\">0</td>\n<td headers=\"gear\" class=\"gt_row gt_right\">3</td>\n<td headers=\"carb\" class=\"gt_row gt_right\">1</td></tr>\n <tr><td headers=\"mpg\" class=\"gt_row gt_right\">18.7</td>\n<td headers=\"cyl\" class=\"gt_row gt_right\">8</td>\n<td headers=\"disp\" class=\"gt_row gt_right\">360</td>\n<td headers=\"hp\" class=\"gt_row gt_right\">175</td>\n<td headers=\"drat\" class=\"gt_row gt_right\">3.15</td>\n<td headers=\"wt\" class=\"gt_row gt_right\">3.440</td>\n<td headers=\"qsec\" class=\"gt_row gt_right\">17.02</td>\n<td headers=\"vs\" class=\"gt_row gt_right\">0</td>\n<td headers=\"am\" class=\"gt_row gt_right\">0</td>\n<td headers=\"gear\" class=\"gt_row gt_right\">3</td>\n<td headers=\"carb\" class=\"gt_row gt_right\">2</td></tr>\n </tbody>\n \n \n</table>"

# tab_row_group() warns for deprecated args, but respects output.

Code
gt_tbl <- gt(exibble, rowname_col = "row") %>% tab_row_group(label = "group_prioritized",
group = "group", rows = 1:3)
Condition
Warning:
Since gt v0.3.0 the `group` argument has been deprecated.
* Use the `label` argument to specify the group label.
This warning is displayed once every 8 hours.

---

Code
gt_tbl <- (gt(exibble, rowname_col = "row") %>% tab_row_group(label = "one",
rows = 1:3) %>% tab_row_group(others_label = "foo"))
Condition
Warning:
Since gt v0.3.0 the `others_label` argument has been deprecated.
* Use `tab_options(row_group.default_label = <label>)` to set this label.
This warning is displayed once every 8 hours.

# tab_row_group() errors when named rows are supplied (#1535)

Code
gt_tbl %>% tab_row_group("Mazda", c("Mazda RX4", "Mazda RX4 Wag"))
Condition
Error in `tab_row_group()`:
! Can't find named rows in the table
i In gt() (`?gt::gt()`), use `rownames_to_stub = TRUE` or specify `rowname_col` to initialize row names in the table.

9 changes: 9 additions & 0 deletions tests/testthat/helper.R
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ expect_merge_locale_sep <- function(locale = NULL, global_locale = NULL, sep = N
expect_equal(actual_merge_sep, expected_sep, label = paste0("locale = ", locale))
}

# Gets the text from a row group label
get_row_group_text <- function(tbl_html) {
gsub(
"\n\\s+",
"",
selection_text(tbl_html, "[class='gt_group_heading_row']")
)
}

# Create a shortened version of `mtcars`
mtcars_short <- datasets::mtcars[1:5, ]

Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/test-fmt_percent.R
Original file line number Diff line number Diff line change
Expand Up @@ -414,10 +414,10 @@ test_that("fmt_percent() can render in the Indian numbering system", {
})

test_that("fmt_percent() works correctly with stubs", {
tbl <- tibble::tibble(
tbl <- dplyr::tibble(
raw = c("[shiny](https://shiny.posit.co/)", "<a href='https://gt.rstudio.com/'>gt</a>"),
markdown = purrr::map(c("[shiny](https://shiny.posit.co/)", "[gt](https://gt.rstudio.com/)"), gt::md),
html = purrr::map(c("<a href='https://shiny.posit.co/'>shiny</a>", "<a href='https://gt.rstudio.com/'>gt</a>"), gt::html),
markdown = lapply(c("[shiny](https://shiny.posit.co/)", "[gt](https://gt.rstudio.com/)"), md),
html = lapply(c("<a href='https://shiny.posit.co/'>shiny</a>", "<a href='https://gt.rstudio.com/'>gt</a>"), html),
str_col = c("shiny", "gt"),
pct_col = c(0.75, 0.25)
)
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-tab_spanner.R
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ cli::test_that_cli("multiple levels of `tab_spanner()` are not compatible with i
}
)

test_that("multiple levels of `tab_spanner()` are not compatible with interactive tables and only use 1st level", {
test_that("tab_spanner() can't render multiple spanners in interactive tables and only use 1st level", {
check_suggests()
skip_if_not_installed("jsonlite")

Expand Down
Loading

0 comments on commit 37ad5a6

Please sign in to comment.