Skip to content

Commit

Permalink
Don't convert implicit NA to explicit in fct_count
Browse files Browse the repository at this point in the history
Requiring some gymnastics to do the counting correctly :|. Fixes #151
  • Loading branch information
hadley committed Feb 28, 2020
1 parent e13df2a commit 4220934
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 5 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# forcats (development version)

* `fct_count()` no longer converts implicit NAs into explicit NAs (#151).

* `fct_inseq()` behaves more robustly when factor levels aren't all numbers
(#221).

Expand Down
8 changes: 4 additions & 4 deletions R/count.R
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
#' fct_count(f, sort = TRUE)
#' fct_count(f, sort = TRUE, prop = TRUE)
fct_count <- function(f, sort = FALSE, prop = FALSE) {
f <- check_factor(f)
f2 <- addNA(f, ifany = TRUE)
f2 <- check_factor(f)
n_na <- sum(is.na(f))

df <- tibble::tibble(
f = fct_unique(f2),
n = as.integer(table(f2))
f = fct_inorder(c(levels(f2), if (n_na > 0) NA)),
n = c(tabulate(f2), if (n_na > 0) n_na)
)

if (sort) {
Expand Down
3 changes: 2 additions & 1 deletion tests/testthat/test-count.R
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@ test_that("0 count for empty levels", {
expect_equal(out$n, c(0, 0))
})


test_that("counts NA even when not in levels", {
f <- factor(c("a", "a", NA))
out <- fct_count(f)

expect_equal(out$n, c(2, 1))
# and doesn't change levels
expect_equal(levels(out$f), levels(f))
})

test_that("returns marginal table", {
Expand Down

0 comments on commit 4220934

Please sign in to comment.