From 75c0a083f30a1a0992686ee64966e5d15f78ae78 Mon Sep 17 00:00:00 2001 From: Trevor Nederlof Date: Tue, 3 May 2022 09:47:42 -0400 Subject: [PATCH 1/5] Add na.rm argument to nth(), first() and last() --- R/nth-value.R | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/R/nth-value.R b/R/nth-value.R index 8efef2262d..15433eaf18 100644 --- a/R/nth-value.R +++ b/R/nth-value.R @@ -39,12 +39,16 @@ #' #' # These functions always return a single value #' first(integer()) -nth <- function(x, n, order_by = NULL, default = default_missing(x)) { +nth <- function(x, n, order_by = NULL, default = default_missing(x), na.rm = FALSE) { if (length(n) != 1 || !is.numeric(n)) { abort("`n` must be a single integer.") } n <- trunc(n) + if (na.rm) { + x <- x[!is.na(x)] + } + if (n == 0 || n > length(x) || n < -length(x)) { return(default) } @@ -63,14 +67,14 @@ nth <- function(x, n, order_by = NULL, default = default_missing(x)) { #' @export #' @rdname nth -first <- function(x, order_by = NULL, default = default_missing(x)) { - nth(x, 1L, order_by = order_by, default = default) +first <- function(x, order_by = NULL, default = default_missing(x), na.rm = FALSE) { + nth(x, 1L, order_by = order_by, default = default, na.rm = na.rm) } #' @export #' @rdname nth -last <- function(x, order_by = NULL, default = default_missing(x)) { - nth(x, -1L, order_by = order_by, default = default) +last <- function(x, order_by = NULL, default = default_missing(x), na.rm = FALSE) { + nth(x, -1L, order_by = order_by, default = default, na.rm = na.rm) } default_missing <- function(x) { From 0b9ae8780f404753d63ca89b3bec9cebc41f36fa Mon Sep 17 00:00:00 2001 From: Trevor Nederlof Date: Tue, 3 May 2022 10:10:57 -0400 Subject: [PATCH 2/5] Add tests for na.rm = TRUE in nth() --- tests/testthat/test-nth-value.R | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/testthat/test-nth-value.R b/tests/testthat/test-nth-value.R index e21b913140..68d4c07f4c 100644 --- a/tests/testthat/test-nth-value.R +++ b/tests/testthat/test-nth-value.R @@ -42,3 +42,15 @@ test_that("nth() gives meaningful error message (#5466)", { (expect_error(nth(1:10, "x"))) }) }) + +test_that("nth() works with NA values when na.rm = TRUE", { + expect_equal(nth(c(as.numeric(NA), 1:3), 1, na.rm = TRUE), 1) + expect_equal(nth(c(1, as.numeric(NA), 2, 3), 2, na.rm = TRUE), 2) + expect_equal(nth(c(1:3, as.numeric(NA)), -1, na.rm = TRUE), 3) +}) + +test_that("nth() uses default value when na.rm = TRUE and all NA values", { + expect_equal(nth(rep(as.numeric(NA), 3), 1, na.rm = TRUE), as.numeric(NA)) + expect_equal(nth(rep(as.numeric(NA), 3), 2, na.rm = TRUE), as.numeric(NA)) + expect_equal(nth(rep(as.numeric(NA), 3), -1, na.rm = TRUE), as.numeric(NA)) +}) \ No newline at end of file From 9e4628464d2f0f693d165affbfa16b049807e652 Mon Sep 17 00:00:00 2001 From: Trevor Nederlof Date: Tue, 3 May 2022 10:15:03 -0400 Subject: [PATCH 3/5] Revise nth() documentation --- R/nth-value.R | 1 + man/nth.Rd | 8 +++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/R/nth-value.R b/R/nth-value.R index 15433eaf18..bf8e45b8a1 100644 --- a/R/nth-value.R +++ b/R/nth-value.R @@ -19,6 +19,7 @@ #' #' For more complicated objects, you'll need to supply this value. #' Make sure it is the same type as `x`. +#' @param na.rm if `TRUE` missing values don't count #' @return A single value. `[[` is used to do the subsetting. #' @export #' @examples diff --git a/man/nth.Rd b/man/nth.Rd index 57eeb78fef..3740f4c5f5 100644 --- a/man/nth.Rd +++ b/man/nth.Rd @@ -6,11 +6,11 @@ \alias{last} \title{Extract the first, last or nth value from a vector} \usage{ -nth(x, n, order_by = NULL, default = default_missing(x)) +nth(x, n, order_by = NULL, default = default_missing(x), na.rm = FALSE) -first(x, order_by = NULL, default = default_missing(x)) +first(x, order_by = NULL, default = default_missing(x), na.rm = FALSE) -last(x, order_by = NULL, default = default_missing(x)) +last(x, order_by = NULL, default = default_missing(x), na.rm = FALSE) } \arguments{ \item{x}{A vector} @@ -30,6 +30,8 @@ a \code{NULL} is return. For more complicated objects, you'll need to supply this value. Make sure it is the same type as \code{x}.} + +\item{na.rm}{if \code{TRUE} missing values don't count} } \value{ A single value. \code{[[} is used to do the subsetting. From a244ba0e15ab25b7bd1f2768da45d3f9458e94d8 Mon Sep 17 00:00:00 2001 From: Trevor Nederlof Date: Tue, 3 May 2022 10:21:20 -0400 Subject: [PATCH 4/5] NEWS bullet --- NEWS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.md b/NEWS.md index fbdcf8e558..9d304d5cde 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,11 @@ # dplyr 1.0.9 + + +* `nth()`, `first()`, and `last()` gained the `na.rm` argument since they are + summary functions (#6242). + * New `rows_append()` which works like `rows_insert()` but ignores keys and allows you to insert arbitrary rows with a guarantee that the type of `x` won't change (#6249, thanks to @krlmlr for the implementation and @mgirlich From 35b66ef0e8c17879207a2cb03fe7db30575a7361 Mon Sep 17 00:00:00 2001 From: Trevor Nederlof Date: Thu, 2 Jun 2022 08:44:11 -0400 Subject: [PATCH 5/5] Change argument from na.rm to na_rm to be consistent --- NEWS.md | 2 +- R/nth-value.R | 14 +++++++------- man/nth.Rd | 8 ++++---- tests/testthat/test-nth-value.R | 18 +++++++++--------- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/NEWS.md b/NEWS.md index 9d304d5cde..793fe1cc7e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,7 +4,7 @@ -* `nth()`, `first()`, and `last()` gained the `na.rm` argument since they are +* `nth()`, `first()`, and `last()` gained the `na_rm` argument since they are summary functions (#6242). * New `rows_append()` which works like `rows_insert()` but ignores keys and diff --git a/R/nth-value.R b/R/nth-value.R index bf8e45b8a1..4e0fd00ed4 100644 --- a/R/nth-value.R +++ b/R/nth-value.R @@ -19,7 +19,7 @@ #' #' For more complicated objects, you'll need to supply this value. #' Make sure it is the same type as `x`. -#' @param na.rm if `TRUE` missing values don't count +#' @param na_rm if `TRUE` missing values don't count #' @return A single value. `[[` is used to do the subsetting. #' @export #' @examples @@ -40,13 +40,13 @@ #' #' # These functions always return a single value #' first(integer()) -nth <- function(x, n, order_by = NULL, default = default_missing(x), na.rm = FALSE) { +nth <- function(x, n, order_by = NULL, default = default_missing(x), na_rm = FALSE) { if (length(n) != 1 || !is.numeric(n)) { abort("`n` must be a single integer.") } n <- trunc(n) - if (na.rm) { + if (na_rm) { x <- x[!is.na(x)] } @@ -68,14 +68,14 @@ nth <- function(x, n, order_by = NULL, default = default_missing(x), na.rm = FAL #' @export #' @rdname nth -first <- function(x, order_by = NULL, default = default_missing(x), na.rm = FALSE) { - nth(x, 1L, order_by = order_by, default = default, na.rm = na.rm) +first <- function(x, order_by = NULL, default = default_missing(x), na_rm = FALSE) { + nth(x, 1L, order_by = order_by, default = default, na_rm = na_rm) } #' @export #' @rdname nth -last <- function(x, order_by = NULL, default = default_missing(x), na.rm = FALSE) { - nth(x, -1L, order_by = order_by, default = default, na.rm = na.rm) +last <- function(x, order_by = NULL, default = default_missing(x), na_rm = FALSE) { + nth(x, -1L, order_by = order_by, default = default, na_rm = na_rm) } default_missing <- function(x) { diff --git a/man/nth.Rd b/man/nth.Rd index 3740f4c5f5..32fd535858 100644 --- a/man/nth.Rd +++ b/man/nth.Rd @@ -6,11 +6,11 @@ \alias{last} \title{Extract the first, last or nth value from a vector} \usage{ -nth(x, n, order_by = NULL, default = default_missing(x), na.rm = FALSE) +nth(x, n, order_by = NULL, default = default_missing(x), na_rm = FALSE) -first(x, order_by = NULL, default = default_missing(x), na.rm = FALSE) +first(x, order_by = NULL, default = default_missing(x), na_rm = FALSE) -last(x, order_by = NULL, default = default_missing(x), na.rm = FALSE) +last(x, order_by = NULL, default = default_missing(x), na_rm = FALSE) } \arguments{ \item{x}{A vector} @@ -31,7 +31,7 @@ a \code{NULL} is return. For more complicated objects, you'll need to supply this value. Make sure it is the same type as \code{x}.} -\item{na.rm}{if \code{TRUE} missing values don't count} +\item{na_rm}{if \code{TRUE} missing values don't count} } \value{ A single value. \code{[[} is used to do the subsetting. diff --git a/tests/testthat/test-nth-value.R b/tests/testthat/test-nth-value.R index 68d4c07f4c..5c413c830c 100644 --- a/tests/testthat/test-nth-value.R +++ b/tests/testthat/test-nth-value.R @@ -43,14 +43,14 @@ test_that("nth() gives meaningful error message (#5466)", { }) }) -test_that("nth() works with NA values when na.rm = TRUE", { - expect_equal(nth(c(as.numeric(NA), 1:3), 1, na.rm = TRUE), 1) - expect_equal(nth(c(1, as.numeric(NA), 2, 3), 2, na.rm = TRUE), 2) - expect_equal(nth(c(1:3, as.numeric(NA)), -1, na.rm = TRUE), 3) +test_that("nth() works with NA values when na_rm = TRUE", { + expect_equal(nth(c(as.numeric(NA), 1:3), 1, na_rm = TRUE), 1) + expect_equal(nth(c(1, as.numeric(NA), 2, 3), 2, na_rm = TRUE), 2) + expect_equal(nth(c(1:3, as.numeric(NA)), -1, na_rm = TRUE), 3) }) -test_that("nth() uses default value when na.rm = TRUE and all NA values", { - expect_equal(nth(rep(as.numeric(NA), 3), 1, na.rm = TRUE), as.numeric(NA)) - expect_equal(nth(rep(as.numeric(NA), 3), 2, na.rm = TRUE), as.numeric(NA)) - expect_equal(nth(rep(as.numeric(NA), 3), -1, na.rm = TRUE), as.numeric(NA)) -}) \ No newline at end of file +test_that("nth() uses default value when na_rm = TRUE and all NA values", { + expect_equal(nth(rep(as.numeric(NA), 3), 1, na_rm = TRUE), as.numeric(NA)) + expect_equal(nth(rep(as.numeric(NA), 3), 2, na_rm = TRUE), as.numeric(NA)) + expect_equal(nth(rep(as.numeric(NA), 3), -1, na_rm = TRUE), as.numeric(NA)) +})