From e9e43217f69cc0d18f81c16e0e7ab47a93ba27b2 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Mon, 19 Jul 2021 16:30:03 -0400 Subject: [PATCH 01/14] Add basic implementation --- r/R/dplyr-functions.R | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index 11efb7f26d2..8b52bac97c0 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -57,6 +57,14 @@ nse_funcs$cast <- function(x, target_type, safe = TRUE, ...) { Expression$create("cast", x, options = opts) } +nse_funcs$coalesce <- function(...) { + if (missing(..1)) { + abort("At least one argument must be supplied to coalesce()") + } + args <- list2(...) + build_expr("coalesce", args = args) +} + nse_funcs$is.na <- function(x) { # TODO: if an option is added to the is_null kernel to treat NaN as NA, # use that to simplify the code here (ARROW-13367) From a271bc71e1dd3ad29a9103fae7cc9657b88a30a8 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Mon, 19 Jul 2021 16:33:10 -0400 Subject: [PATCH 02/14] Handle NaN consistent with dplyr::coalesce() --- r/R/dplyr-functions.R | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index 8b52bac97c0..231bf8743b8 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -61,8 +61,24 @@ nse_funcs$coalesce <- function(...) { if (missing(..1)) { abort("At least one argument must be supplied to coalesce()") } - args <- list2(...) - build_expr("coalesce", args = args) + # TODO: if an option is added to the coalesce kernel to treat NaN as NA, + # use that to simplify the code here (ARROW-13389) + args <- lapply(list2(...), function(arg) { + if (!inherits(arg, "Expression")) { + arg <- Expression$scalar(arg) + } + if (arg$type_id() %in% TYPES_WITH_NAN) { + # replace NaN with NA, using Arrow's smallest float type to avoid casting + # smaller float types to larger float types + NA_expr <- Expression$scalar(Scalar$create(NA_real_, type = arg$type())) + # TODO: Figure out why this doesn't work: + #Expression$create("replace_with_mask", arg, Expression$create("is_nan", arg), NA_expr) + Expression$create("if_else", Expression$create("is_nan", arg), NA_expr, arg) + } else { + arg + } + }) + Expression$create("coalesce", args = args) } nse_funcs$is.na <- function(x) { From 9617573dcb1651d2e1faa41ad8da489c21389cc0 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Mon, 19 Jul 2021 16:34:24 -0400 Subject: [PATCH 03/14] Add tests --- r/tests/testthat/test-dplyr.R | 97 +++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/r/tests/testthat/test-dplyr.R b/r/tests/testthat/test-dplyr.R index ac36c5a1bc9..047dae886a8 100644 --- a/r/tests/testthat/test-dplyr.R +++ b/r/tests/testthat/test-dplyr.R @@ -1359,3 +1359,100 @@ test_that("case_when()", { tbl ) }) + +test_that("coalesce()", { + # character + df <- tibble( + w = c(NA_character_, NA_character_, NA_character_), + x = c(NA_character_, NA_character_, "c"), + y = c(NA_character_, "b", "c"), + z = c("a", "b", "c") + ) + expect_dplyr_equal( + input %>% + mutate( + cw = coalesce(w), + cz = coalesce(z), + cwx = coalesce(w, x), + cwxy = coalesce(w, x, y), + cwxyz = coalesce(w, x, y, z) + ) %>% + collect(), + df + ) + + # integer + df <- tibble( + w = c(NA_integer_, NA_integer_, NA_integer_), + x = c(NA_integer_, NA_integer_, 3L), + y = c(NA_integer_, 2L, 3L), + z = 1:3 + ) + expect_dplyr_equal( + input %>% + mutate( + cw = coalesce(w), + cz = coalesce(z), + cwx = coalesce(w, x), + cwxy = coalesce(w, x, y), + cwxyz = coalesce(w, x, y, z) + ) %>% + collect(), + df + ) + + # double with NaN + df <- tibble( + w = c(NA_real_, NA_real_, NA_real_), + x = c(NA_real_, NaN, 3.3), + y = c(NA_real_, 2.2, 3.3), + z = c(1.1, 2.2, 3.3) + ) + expect_dplyr_equal( + input %>% + mutate( + cw = coalesce(w), + cz = coalesce(z), + cwx = coalesce(w, x), + cwxy = coalesce(w, x, y), + cwxyz = coalesce(w, x, y, z) + ) %>% + collect(), + df + ) + # singles stay single + expect_equal( + (df %>% + Table$create(schema = schema( + w = float32(), + x = float32(), + y = float32(), + z = float32() + )) %>% + transmute(c = coalesce(w, x, y, z)) %>% + compute() + )$schema[[1]]$type, + float32() + ) + # with R literal values + expect_dplyr_equal( + input %>% + mutate( + c1 = coalesce(4.4), + c2 = coalesce(NA_real_), + c3 = coalesce(NaN), + c4 = coalesce(w, x, y, 5.5), + c5 = coalesce(w, x, y, NA_real_), + c6 = coalesce(w, x, y, NaN) + ) %>% + collect(), + df + ) + + # no arguments + expect_error( + nse_funcs$coalesce(), + "At least one argument must be supplied to coalesce()", + fixed = TRUE + ) +}) From 3a91ee911b2367abafaf3c96d45d95ed456d5bb3 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Mon, 19 Jul 2021 17:45:12 -0400 Subject: [PATCH 04/14] Fix NaN handling --- r/R/dplyr-functions.R | 14 +++++++++++--- r/tests/testthat/test-dplyr.R | 10 ++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index 231bf8743b8..7be76fb078d 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -61,20 +61,28 @@ nse_funcs$coalesce <- function(...) { if (missing(..1)) { abort("At least one argument must be supplied to coalesce()") } + + # treat NaN like NA for consistency with dplyr::coalesce() # TODO: if an option is added to the coalesce kernel to treat NaN as NA, # use that to simplify the code here (ARROW-13389) - args <- lapply(list2(...), function(arg) { + args <- list2(...) + # if *all* the values are NaN, we should return NaN, not NA, so don't replace + # NaN in the final argument with NA + attr(args[-1][[1]], "last") <- TRUE + args <- lapply(args, function(arg) { if (!inherits(arg, "Expression")) { arg <- Expression$scalar(arg) } - if (arg$type_id() %in% TYPES_WITH_NAN) { - # replace NaN with NA, using Arrow's smallest float type to avoid casting + + if (is.null(attr(arg, "last")) && arg$type_id() %in% TYPES_WITH_NAN) { + # store the NA_real_ in Arrow's smallest float type to avoid casting # smaller float types to larger float types NA_expr <- Expression$scalar(Scalar$create(NA_real_, type = arg$type())) # TODO: Figure out why this doesn't work: #Expression$create("replace_with_mask", arg, Expression$create("is_nan", arg), NA_expr) Expression$create("if_else", Expression$create("is_nan", arg), NA_expr, arg) } else { + attr(arg, "last") <- NULL arg } }) diff --git a/r/tests/testthat/test-dplyr.R b/r/tests/testthat/test-dplyr.R index 047dae886a8..4f39141ba9b 100644 --- a/r/tests/testthat/test-dplyr.R +++ b/r/tests/testthat/test-dplyr.R @@ -1401,9 +1401,9 @@ test_that("coalesce()", { df ) - # double with NaN + # double with NaNs df <- tibble( - w = c(NA_real_, NA_real_, NA_real_), + w = c(NA_real_, NaN, NA_real_), x = c(NA_real_, NaN, 3.3), y = c(NA_real_, 2.2, 3.3), z = c(1.1, 2.2, 3.3) @@ -1420,6 +1420,12 @@ test_that("coalesce()", { collect(), df ) + # NaNs stay NaN and are not converted to NA in the results + # (testing this requires expect_identical()) + expect_identical( + df %>% mutate(cwx = coalesce(w, x)), + df %>% Table$create() %>% mutate(cwx = coalesce(w, x)) %>% collect() + ) # singles stay single expect_equal( (df %>% From ede440d545c8cc5f4efb68f3a4d6e088a1f284e9 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Mon, 19 Jul 2021 17:45:33 -0400 Subject: [PATCH 05/14] Warn on factors --- r/R/dplyr-functions.R | 6 ++++++ r/tests/testthat/test-dplyr.R | 19 ++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index 7be76fb078d..6d3915c9213 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -74,6 +74,12 @@ nse_funcs$coalesce <- function(...) { arg <- Expression$scalar(arg) } + # coalesce doesn't yet support factors/dictionaries + # TODO: remove this after ARROW-13390 is merged + if (nse_funcs$is.factor(arg)) { + warning("Dictionaries (in R: factors) are currently converted to strings (characters) in coalesce", call. = FALSE) + } + if (is.null(attr(arg, "last")) && arg$type_id() %in% TYPES_WITH_NAN) { # store the NA_real_ in Arrow's smallest float type to avoid casting # smaller float types to larger float types diff --git a/r/tests/testthat/test-dplyr.R b/r/tests/testthat/test-dplyr.R index 4f39141ba9b..b6f81b0a726 100644 --- a/r/tests/testthat/test-dplyr.R +++ b/r/tests/testthat/test-dplyr.R @@ -1207,7 +1207,7 @@ test_that("if_else and ifelse", { mutate( y = if_else(int > 5, fct, factor("a")) ) %>% collect() %>% - # This is a no-op on the Arrow side, but necesary to make the results equal + # This is a no-op on the Arrow side, but necessary to make the results equal mutate(y = as.character(y)), tbl, warning = "Dictionaries .* are currently converted to strings .* in if_else and ifelse" @@ -1455,6 +1455,23 @@ test_that("coalesce()", { df ) + # factors + # TODO: remove the mutate + warning after ARROW-13390 is merged and Arrow + # supports factors in coalesce + df <- tibble( + x = factor("a", levels = c("a", "z")), + y = factor("b", levels = c("a", "b", "c")) + ) + expect_dplyr_equal( + input %>% + mutate(c = coalesce(x, y)) %>% + collect() %>% + # This is a no-op on the Arrow side, but necessary to make the results equal + mutate(c = as.character(c)), + df, + warning = "Dictionaries .* are currently converted to strings .* in coalesce" + ) + # no arguments expect_error( nse_funcs$coalesce(), From 63636a8df3a82bf4565067a642d67a1e4f6a37da Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Mon, 19 Jul 2021 17:51:38 -0400 Subject: [PATCH 06/14] Bugfix --- r/R/dplyr-functions.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index 6d3915c9213..a79fadcadae 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -68,7 +68,7 @@ nse_funcs$coalesce <- function(...) { args <- list2(...) # if *all* the values are NaN, we should return NaN, not NA, so don't replace # NaN in the final argument with NA - attr(args[-1][[1]], "last") <- TRUE + attr(args[[length(args)]], "last") <- TRUE args <- lapply(args, function(arg) { if (!inherits(arg, "Expression")) { arg <- Expression$scalar(arg) From 8a308135c95ce071ce4bcf515dc5479ab1e295c7 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Mon, 19 Jul 2021 18:04:43 -0400 Subject: [PATCH 07/14] Fix handling of NaN literals --- r/R/dplyr-functions.R | 8 +++++--- r/tests/testthat/test-dplyr.R | 12 ++++++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index a79fadcadae..3102925638a 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -67,9 +67,12 @@ nse_funcs$coalesce <- function(...) { # use that to simplify the code here (ARROW-13389) args <- list2(...) # if *all* the values are NaN, we should return NaN, not NA, so don't replace - # NaN in the final argument with NA + # NaN with NA in the final (or only) argument attr(args[[length(args)]], "last") <- TRUE args <- lapply(args, function(arg) { + last_arg <- is.null(attr(arg, "last")) + attr(arg, "last") <- NULL + if (!inherits(arg, "Expression")) { arg <- Expression$scalar(arg) } @@ -80,7 +83,7 @@ nse_funcs$coalesce <- function(...) { warning("Dictionaries (in R: factors) are currently converted to strings (characters) in coalesce", call. = FALSE) } - if (is.null(attr(arg, "last")) && arg$type_id() %in% TYPES_WITH_NAN) { + if (last_arg && arg$type_id() %in% TYPES_WITH_NAN) { # store the NA_real_ in Arrow's smallest float type to avoid casting # smaller float types to larger float types NA_expr <- Expression$scalar(Scalar$create(NA_real_, type = arg$type())) @@ -88,7 +91,6 @@ nse_funcs$coalesce <- function(...) { #Expression$create("replace_with_mask", arg, Expression$create("is_nan", arg), NA_expr) Expression$create("if_else", Expression$create("is_nan", arg), NA_expr, arg) } else { - attr(arg, "last") <- NULL arg } }) diff --git a/r/tests/testthat/test-dplyr.R b/r/tests/testthat/test-dplyr.R index b6f81b0a726..0a7ea8da89b 100644 --- a/r/tests/testthat/test-dplyr.R +++ b/r/tests/testthat/test-dplyr.R @@ -1423,8 +1423,16 @@ test_that("coalesce()", { # NaNs stay NaN and are not converted to NA in the results # (testing this requires expect_identical()) expect_identical( - df %>% mutate(cwx = coalesce(w, x)), - df %>% Table$create() %>% mutate(cwx = coalesce(w, x)) %>% collect() + df %>% Table$create() %>% mutate(cwx = coalesce(w, x)) %>% collect(), + df %>% mutate(cwx = coalesce(w, x)) + ) + expect_identical( + df %>% Table$create() %>% transmute(cw = coalesce(w)) %>% collect(), + df %>% transmute(cw = coalesce(w)) + ) + expect_identical( + df %>% Table$create() %>% transmute(cn = coalesce(NaN)) %>% collect(), + df %>% transmute(cn = coalesce(NaN)) ) # singles stay single expect_equal( From 5bf11819da4c4c669d4317e4d12e228847e3cc79 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Mon, 19 Jul 2021 18:13:57 -0400 Subject: [PATCH 08/14] Update NEWS --- r/NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/NEWS.md b/r/NEWS.md index b05cc131902..0b6c9647f9a 100644 --- a/r/NEWS.md +++ b/r/NEWS.md @@ -26,7 +26,7 @@ * String operations: `strsplit()` and `str_split()`; `strptime()`; `paste()`, `paste0()`, and `str_c()`; `substr()` and `str_sub()`; `str_like()`; `str_pad()`; `stri_reverse()` * Date/time operations: `lubridate` methods such as `year()`, `month()`, `wday()`, and so on * Math: logarithms (`log()` et al.); trigonometry (`sin()`, `cos()`, et al.); `abs()`; `sign()`; `pmin()` and `pmax()`; `ceiling()`, `floor()`, and `trunc()` - * Conditional: `ifelse()` and `if_else()` (fixed-precision decimal numbers do not yet work and factors/dictionaries are converted to character strings); `case_when()` (currently works with numeric data types but not character strings, factors/dictionaries, or lists/structs) + * Conditional: `ifelse()` and `if_else()` (fixed-precision decimal numbers do not yet work and factors/dictionaries are converted to character strings); `case_when()` (currently works with numeric data types but not character strings, factors/dictionaries, or lists/structs); `coalesce()` (lists/structs do not work and factors/dictionaries are converted to character strings) * `is.*` functions are supported and can be used inside `relocate()` * The print method for `arrow_dplyr_query` now includes the expression and the resulting type of columns derived by `mutate()`. From 09d740447290e1429d955911c1fcb059f1ebcdd6 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Tue, 20 Jul 2021 09:08:04 -0400 Subject: [PATCH 09/14] Improve NEWS Co-authored-by: Nic --- r/NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/NEWS.md b/r/NEWS.md index 0b6c9647f9a..99754a3929f 100644 --- a/r/NEWS.md +++ b/r/NEWS.md @@ -26,7 +26,7 @@ * String operations: `strsplit()` and `str_split()`; `strptime()`; `paste()`, `paste0()`, and `str_c()`; `substr()` and `str_sub()`; `str_like()`; `str_pad()`; `stri_reverse()` * Date/time operations: `lubridate` methods such as `year()`, `month()`, `wday()`, and so on * Math: logarithms (`log()` et al.); trigonometry (`sin()`, `cos()`, et al.); `abs()`; `sign()`; `pmin()` and `pmax()`; `ceiling()`, `floor()`, and `trunc()` - * Conditional: `ifelse()` and `if_else()` (fixed-precision decimal numbers do not yet work and factors/dictionaries are converted to character strings); `case_when()` (currently works with numeric data types but not character strings, factors/dictionaries, or lists/structs); `coalesce()` (lists/structs do not work and factors/dictionaries are converted to character strings) + * Conditional: `ifelse()` and `if_else()` (fixed-precision decimal numbers not yet implemented and factors/dictionaries are converted to character strings); `case_when()` (currently works with numeric data types but not character strings, factors/dictionaries, or lists/structs); `coalesce()` (lists/structs not yet implemented and factors/dictionaries are converted to character strings) * `is.*` functions are supported and can be used inside `relocate()` * The print method for `arrow_dplyr_query` now includes the expression and the resulting type of columns derived by `mutate()`. From 66d742be92bb9d90738f89b0eb0453f22c7e2dd4 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Tue, 20 Jul 2021 16:40:38 -0400 Subject: [PATCH 10/14] Make NEWS bullet more concise Co-authored-by: Neal Richardson --- r/NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/NEWS.md b/r/NEWS.md index 99754a3929f..5ec8492a9ca 100644 --- a/r/NEWS.md +++ b/r/NEWS.md @@ -26,7 +26,7 @@ * String operations: `strsplit()` and `str_split()`; `strptime()`; `paste()`, `paste0()`, and `str_c()`; `substr()` and `str_sub()`; `str_like()`; `str_pad()`; `stri_reverse()` * Date/time operations: `lubridate` methods such as `year()`, `month()`, `wday()`, and so on * Math: logarithms (`log()` et al.); trigonometry (`sin()`, `cos()`, et al.); `abs()`; `sign()`; `pmin()` and `pmax()`; `ceiling()`, `floor()`, and `trunc()` - * Conditional: `ifelse()` and `if_else()` (fixed-precision decimal numbers not yet implemented and factors/dictionaries are converted to character strings); `case_when()` (currently works with numeric data types but not character strings, factors/dictionaries, or lists/structs); `coalesce()` (lists/structs not yet implemented and factors/dictionaries are converted to character strings) + * Conditional functions, with some limitations on input type in this release: `ifelse()` and `if_else()` for all but `Decimal` types; `case_when()` for logical, numeric, and temporal types only; `coalesce()` for all but lists/structs. Note also that in this release, factors/dictionaries are converted to strings in these functions. * `is.*` functions are supported and can be used inside `relocate()` * The print method for `arrow_dplyr_query` now includes the expression and the resulting type of columns derived by `mutate()`. From fd444557c74fb6c96f42888ac88f5f7120aa22de Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Tue, 20 Jul 2021 16:43:56 -0400 Subject: [PATCH 11/14] Reorganize for readability Co-authored-by: Neal Richardson --- r/R/dplyr-functions.R | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index 3102925638a..d3b92456801 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -58,16 +58,16 @@ nse_funcs$cast <- function(x, target_type, safe = TRUE, ...) { } nse_funcs$coalesce <- function(...) { - if (missing(..1)) { + args <- list2(...) + if (length(args) < 1) { abort("At least one argument must be supplied to coalesce()") } - # treat NaN like NA for consistency with dplyr::coalesce() - # TODO: if an option is added to the coalesce kernel to treat NaN as NA, - # use that to simplify the code here (ARROW-13389) - args <- list2(...) + # Treat NaN like NA for consistency with dplyr::coalesce(): # if *all* the values are NaN, we should return NaN, not NA, so don't replace # NaN with NA in the final (or only) argument + # TODO: if an option is added to the coalesce kernel to treat NaN as NA, + # use that to simplify the code here (ARROW-13389) attr(args[[length(args)]], "last") <- TRUE args <- lapply(args, function(arg) { last_arg <- is.null(attr(arg, "last")) From 0110cef6ca929480b508e351a5444a1dc1c699ef Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Tue, 20 Jul 2021 17:09:27 -0400 Subject: [PATCH 12/14] Remove misguided TODO comment Co-authored-by: Benjamin Kietzman --- r/R/dplyr-functions.R | 2 -- 1 file changed, 2 deletions(-) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index d3b92456801..fa8dedddb3a 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -87,8 +87,6 @@ nse_funcs$coalesce <- function(...) { # store the NA_real_ in Arrow's smallest float type to avoid casting # smaller float types to larger float types NA_expr <- Expression$scalar(Scalar$create(NA_real_, type = arg$type())) - # TODO: Figure out why this doesn't work: - #Expression$create("replace_with_mask", arg, Expression$create("is_nan", arg), NA_expr) Expression$create("if_else", Expression$create("is_nan", arg), NA_expr, arg) } else { arg From 04a8e5e4309c3efb583252d6c2809d9c93b0f1cc Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Tue, 20 Jul 2021 17:13:36 -0400 Subject: [PATCH 13/14] Fix comment --- r/R/dplyr-functions.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index fa8dedddb3a..7b7ffcecfb6 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -84,7 +84,7 @@ nse_funcs$coalesce <- function(...) { } if (last_arg && arg$type_id() %in% TYPES_WITH_NAN) { - # store the NA_real_ in Arrow's smallest float type to avoid casting + # store the NA_real_ in the same type as arg to avoid avoid casting # smaller float types to larger float types NA_expr <- Expression$scalar(Scalar$create(NA_real_, type = arg$type())) Expression$create("if_else", Expression$create("is_nan", arg), NA_expr, arg) From 4b399f771d64ac3b528fb8c583208b3c0e20c378 Mon Sep 17 00:00:00 2001 From: Ian Cook Date: Tue, 20 Jul 2021 17:15:51 -0400 Subject: [PATCH 14/14] Improve comment --- r/R/dplyr-functions.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/r/R/dplyr-functions.R b/r/R/dplyr-functions.R index 7b7ffcecfb6..8406de1ba8f 100644 --- a/r/R/dplyr-functions.R +++ b/r/R/dplyr-functions.R @@ -63,8 +63,8 @@ nse_funcs$coalesce <- function(...) { abort("At least one argument must be supplied to coalesce()") } - # Treat NaN like NA for consistency with dplyr::coalesce(): - # if *all* the values are NaN, we should return NaN, not NA, so don't replace + # Treat NaN like NA for consistency with dplyr::coalesce(), but if *all* + # the values are NaN, we should return NaN, not NA, so don't replace # NaN with NA in the final (or only) argument # TODO: if an option is added to the coalesce kernel to treat NaN as NA, # use that to simplify the code here (ARROW-13389)