Skip to content

Fix compact_list() for labelled + other vctrs classes #880

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.

Sign up for GitHub

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

Open
bwiernik wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Fix compact_list() for labelled + other vctrs classes #880

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5253441
Prepare CRAN submission
strengejacke May 31, 2024
9a1c851
cran comments
strengejacke May 31, 2024
6c86c36
comments
strengejacke May 31, 2024
85eb328
Fix compact_list() for labelled + other vctrs classes
bwiernik Jun 1, 2024
584bca9
fix
strengejacke Jun 2, 2024
45499b1
fix vctrs issue
strengejacke Jun 3, 2024
819c578
news
strengejacke Jun 3, 2024
40c4fbc
typo
strengejacke Jun 3, 2024
300ef47
submitted
strengejacke Jun 3, 2024
f062fd7
Fix compact_list() for labelled + other vctrs classes
bwiernik Jun 1, 2024
d70f73b
Remove character "NULL" comparison to avoid vctrs errors
bwiernik Jun 3, 2024
399c9e4
Update utils_compact.R
bwiernik Jun 3, 2024
64a1cce
Remove unnecessary test
bwiernik Jun 3, 2024
dca73b7
Preserve check against character "NULL"
bwiernik Jun 3, 2024
c6d1394
Fix failing test
bwiernik Jun 3, 2024
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
4 changes: 2 additions & 2 deletions R/utils_compact.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
#' @export
compact_list <- function(x, remove_na = FALSE) {
if (remove_na) {
x[!sapply(x, function(i) !is_model(i) && !inherits(i, c("Formula", "gFormula")) && (length(i) == 0L || is.null(i) || (length(i) == 1L && is.na(i)) || all(is.na(i)) || any(is.character(i) && i == "NULL", na.rm = TRUE)))]
x[!sapply(x, function(i) !is_model(i) && !inherits(i, c("Formula", "gFormula")) && (length(i) == 0L || is.null(i) || (length(i) == 1L && is.na(i)) || all(is.na(i)) || all(sapply(i, is.null)) || any(sapply(i, \(j) length(j) == 1 && is.character(j) && j == "NULL"), na.rm = TRUE)))]
Copy link
Member

@strengejacke strengejacke Jun 3, 2024

Choose a reason for hiding this comment

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

We shouldn't use lambdas here due to backwards compatibility.

} else {
x[!sapply(x, function(i) !is_model(i) && !inherits(i, c("Formula", "gFormula")) && (length(i) == 0L || is.null(i) || any(is.character(i) && i == "NULL", na.rm = TRUE)))]
x[!sapply(x, function(i) !is_model(i) && !inherits(i, c("Formula", "gFormula")) && (length(i) == 0L || is.null(i) || all(sapply(i, is.null)) || any(sapply(i, \(j) length(j) == 1 && is.character(j) && j == "NULL"), na.rm = TRUE)))]
}
}

Expand Down