Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 41 additions & 58 deletions tests/testthat/_snaps/pivot-wide.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,47 @@
x Problematic argument:
* name_repair = "check_unique"

# doesn't crash when `id_cols` selects column removed by `names_from` (#1609)

Code
pivot_wider(df, id_cols = x, values_from = y, names_from = x)
Condition
Error in `pivot_wider()`:
! Can't select columns past the end.
i Locations 1, 100, 200, and 300 don't exist.
i There are only 0 columns.

# doesn't crash when `id_cols` selects non-existent column (#1482)

Code
pivot_wider(df, id_cols = c("non", "existent"), names_from = name, values_from = value)
Condition
Error in `pivot_wider()`:
! Can't select columns that don't exist.
x Column `non` doesn't exist.

---

Code
pivot_wider(df2, id_cols = all_of(c("a", "b", "c")), names_from = y,
values_from = z)
Condition
Error in `pivot_wider()`:
i In argument: `all_of(c("a", "b", "c"))`.
Caused by error in `all_of()`:
! Can't subset elements that don't exist.
x Elements `a`, `b`, and `c` don't exist.

---

Code
pivot_wider(df2, id_cols = 1:2, names_from = y, values_from = z)
Condition
Error in `pivot_wider()`:
! Can't select columns past the end.
i Locations 1 and 2 don't exist.
i There are only 0 columns.

# `names_vary` is validated

Code
Expand Down Expand Up @@ -340,61 +381,3 @@
x Problematic argument:
* ids = id

# doesn't crash when `id_cols` selects column removed by `names_from` (#1609)

Code
df %>% dplyr::mutate(y = stringr::str_split(x, "")) %>% unnest(cols = y) %>%
pivot_wider(id_cols = x, values_from = y, names_from = x)
Condition
Warning:
Using an external vector in selections was deprecated in tidyselect 1.1.0.
i Please use `all_of()` or `any_of()` instead.
# Was:
data %>% select(x)

# Now:
data %>% select(all_of(x))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not want to see this warning in the snapshot output


See <https://tidyselect.r-lib.org/reference/faq-external-vector.html>.
Error in `pivot_wider()`:
! Can't select columns past the end.
i Locations 1, 2, 12, ..., 123, and 2341 don't exist.
i There are only 0 columns.

# doesn't crash when `id_cols` selects non-existent column (#1482)

Code
pivot_wider(df, id_cols = c("non", "existent"), names_from = name, values_from = value)
Condition
Error in `pivot_wider()`:
! Can't select columns that don't exist.
x Column `non` doesn't exist.

---

Code
pivot_wider(df, id_cols = c("a", "b", "c"), names_from = name, values_from = value)
Condition
Error in `pivot_wider()`:
! Can't select columns that don't exist.
x Column `a` doesn't exist.

---

Code
pivot_wider(df, id_cols = all_of(c("a", "b", "c")), names_from = y,
values_from = z)
Condition
Error in `pivot_wider()`:
! Can't select columns that don't exist.
x Column `y` doesn't exist.

---

Code
pivot_wider(df, id_cols = 1:2, names_from = y, values_from = z)
Condition
Error in `pivot_wider()`:
! Can't select columns that don't exist.
x Column `y` doesn't exist.

114 changes: 54 additions & 60 deletions tests/testthat/test-pivot-wide.R
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,60 @@ test_that("`pivot_wider_spec()` requires empty dots", {
})
})


test_that("doesn't crash when `id_cols` selects column removed by `names_from` (#1609)", {
local_options(lifecycle_verbosity = "quiet")

# Note how we have an "external vector" here. Ideally tidyselect would error
# on this, but for legacy reasons we currently allow it with a warning, and it
# produces a weird (but correct) tidyselect error
x <- c(1, 100, 200, 300)

df <- tibble(
x = x,
y = c(1, 2, 3, 4)
)

# Should get tidyselect error, not internal error
expect_snapshot(error = TRUE, {
pivot_wider(
df,
id_cols = x,
values_from = y,
names_from = x
)
})
})

test_that("doesn't crash when `id_cols` selects non-existent column (#1482)", {
df <- tibble(name = c("x", "y"), value = c(1, 2))

# Should get tidyselect error, not internal error
expect_snapshot(error = TRUE, {
pivot_wider(
df,
id_cols = c("non", "existent"),
names_from = name,
values_from = value
)
})

df2 <- tibble(y = c("a", "a", "b", "c"), z = c(21, 22, 23, 24))

expect_snapshot(error = TRUE, {
pivot_wider(
df2,
id_cols = all_of(c("a", "b", "c")),
names_from = y,
values_from = z
)
})

expect_snapshot(error = TRUE, {
pivot_wider(df2, id_cols = 1:2, names_from = y, values_from = z)
})
})

# column names -------------------------------------------------------------

test_that("names_glue affects output names", {
Expand Down Expand Up @@ -885,63 +939,3 @@ test_that("`id_cols` compat behavior doesn't trigger if named `...` are supplied
pivot_wider(df, ids = id)
})
})

# Tests for issue #1609 / #1482 -----------------------------------------------

test_that("doesn't crash when `id_cols` selects column removed by `names_from` (#1609)", {
# Original issue scenario
x <- c(1, 2, 12, 31, 123, 2341)
df <- data.frame(x = x)

# This should produce a proper tidyselect error, not an internal error
expect_snapshot(error = TRUE, {
df %>%
dplyr::mutate(y = stringr::str_split(x, "")) %>%
unnest(cols = y) %>%
pivot_wider(
id_cols = x,
values_from = y,
names_from = x
)
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified this pretty dramatically to a minimal reprex with meaningful comments

})

test_that("doesn't crash when `id_cols` selects non-existent column (#1482)", {
# Related issue scenario
df <- tibble(name = c("x", "y"), value = c(1, 2))

# Should get tidyselect error, not internal error
expect_snapshot(error = TRUE, {
pivot_wider(
df,
id_cols = c("non", "existent"),
names_from = name,
values_from = value
)
})

# Character vector case
expect_snapshot(error = TRUE, {
pivot_wider(
df,
id_cols = c("a", "b", "c"),
names_from = name,
values_from = value
)
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a duplicate test


df2 <- tibble(y = c("a", "a", "b", "c"), z = c(21, 22, 23, 24))

expect_snapshot(error = TRUE, {
pivot_wider(
df,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be using df2, the tests aren't useful otherwise, looks like a thinko

id_cols = all_of(c("a", "b", "c")),
names_from = y,
values_from = z
)
})

expect_snapshot(error = TRUE, {
pivot_wider(df, id_cols = 1:2, names_from = y, values_from = z)
})
})
Loading