From eaf2fe96a1d9cb93c1ebe7084226270c65c3b8b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Mon, 24 Jan 2022 16:01:42 +0000 Subject: [PATCH 01/55] added failing tests for timestamps --- r/tests/testthat/test-Array.R | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 15d6d79247a..812d7890419 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -1077,3 +1077,11 @@ test_that("Array to C-interface", { delete_arrow_schema(schema_ptr) delete_arrow_array(array_ptr) }) + +test_that("Array coverts timestamps with missing timezone /assumed local tz correctly", { + a <- as.POSIXct("1970-01-01 00:00:00") + attr(a, "tzone") <- Sys.timezone() + expect_equal( + Array$create(a), + Array$create(0, int64())$cast(timestamp(unit = "us", timezone = Sys.timezone()))) +}) From d7fee58915c7c395c7fd7b9129bbbcc416484870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Mon, 24 Jan 2022 16:37:01 +0000 Subject: [PATCH 02/55] a more robust test --- r/tests/testthat/test-Array.R | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 812d7890419..2efca180f38 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -1079,9 +1079,12 @@ test_that("Array to C-interface", { }) test_that("Array coverts timestamps with missing timezone /assumed local tz correctly", { - a <- as.POSIXct("1970-01-01 00:00:00") - attr(a, "tzone") <- Sys.timezone() - expect_equal( - Array$create(a), - Array$create(0, int64())$cast(timestamp(unit = "us", timezone = Sys.timezone()))) + withr::with_envvar(c(TZ = "America/Chicago"), { + a <- as.POSIXct("1970-01-01 00:00:00") + attr(a, "tzone") <- Sys.getenv("TZ") + expect_equal( + Array$create(a), + Array$create(as.integer(a), int64())$cast(timestamp(unit = "us", timezone = Sys.getenv("TZ"))) + ) + }) }) From 9f6e2ba7fd6421c6f7f208734902a973660d1944 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 25 Jan 2022 10:03:35 +0000 Subject: [PATCH 03/55] added with and without tz examples --- r/tests/testthat/test-Array.R | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 2efca180f38..770ee6c3d18 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -1079,12 +1079,13 @@ test_that("Array to C-interface", { }) test_that("Array coverts timestamps with missing timezone /assumed local tz correctly", { - withr::with_envvar(c(TZ = "America/Chicago"), { - a <- as.POSIXct("1970-01-01 00:00:00") - attr(a, "tzone") <- Sys.getenv("TZ") + withr::with_envvar(c(TZ = NA), { + a <- as.POSIXct("1970-01-01 00:00:15") + attr(a, "tzone") <- Sys.timezone() + b <- as.POSIXct("1970-01-01 00:00:15") expect_equal( Array$create(a), - Array$create(as.integer(a), int64())$cast(timestamp(unit = "us", timezone = Sys.getenv("TZ"))) + Array$create(b) ) }) }) From eba6e74d50b2a856812e4ef56c89358505d95af7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 25 Jan 2022 16:14:50 +0000 Subject: [PATCH 04/55] add the timezone if missing --- r/R/array.R | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/r/R/array.R b/r/R/array.R index 965e3bfc33d..b35d236e827 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -190,6 +190,10 @@ Array$create <- function(x, type = NULL) { return(out) } + if (inherits(x, "POSIXct") && attr(x, "tzone") == "") { + attr(x, "tzone") <- Sys.timezone() + } + if (is.null(type)) { return(vec_to_Array(x, type)) } From ec5e3a3a75d83d3bd720f2b7777c7c8dc4e0a82b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 09:24:59 +0000 Subject: [PATCH 05/55] simplify unit test --- r/R/array.R | 1 + r/tests/testthat/test-Array.R | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/r/R/array.R b/r/R/array.R index b35d236e827..75d100bd19f 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -192,6 +192,7 @@ Array$create <- function(x, type = NULL) { if (inherits(x, "POSIXct") && attr(x, "tzone") == "") { attr(x, "tzone") <- Sys.timezone() + inform("You have not supplied a timezone to x") } if (is.null(type)) { diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 770ee6c3d18..9b268cd0d66 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -1079,9 +1079,8 @@ test_that("Array to C-interface", { }) test_that("Array coverts timestamps with missing timezone /assumed local tz correctly", { - withr::with_envvar(c(TZ = NA), { - a <- as.POSIXct("1970-01-01 00:00:15") - attr(a, "tzone") <- Sys.timezone() + withr::with_envvar(c(TZ = "America/Chicago"), { + a <- as.POSIXct("1970-01-01 00:00:15", tz = "America/Chicago") b <- as.POSIXct("1970-01-01 00:00:15") expect_equal( Array$create(a), From 2dd042a6371bdb75df9517dc094f755ec54264de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 10:27:39 +0000 Subject: [PATCH 06/55] import rlang::inform --- r/R/arrow-package.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/R/arrow-package.R b/r/R/arrow-package.R index 47b10b1bb10..02718927177 100644 --- a/r/R/arrow-package.R +++ b/r/R/arrow-package.R @@ -23,7 +23,7 @@ #' @importFrom rlang eval_tidy new_data_mask syms env new_environment env_bind set_names exec #' @importFrom rlang is_bare_character quo_get_expr quo_get_env quo_set_expr .data seq2 is_interactive #' @importFrom rlang expr caller_env is_character quo_name is_quosure enexpr enexprs as_quosure -#' @importFrom rlang is_list call2 is_empty +#' @importFrom rlang is_list call2 is_empty inform #' @importFrom tidyselect vars_pull vars_rename vars_select eval_select #' @useDynLib arrow, .registration = TRUE #' @keywords internal From 4cf3f4ab7babfac92d077c490474e7cf578d8bd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 10:56:10 +0000 Subject: [PATCH 07/55] add rlang::import to namespace --- r/NAMESPACE | 1 + 1 file changed, 1 insertion(+) diff --git a/r/NAMESPACE b/r/NAMESPACE index 72f71b98f69..feaf2db4990 100644 --- a/r/NAMESPACE +++ b/r/NAMESPACE @@ -338,6 +338,7 @@ importFrom(rlang,env_bind) importFrom(rlang,eval_tidy) importFrom(rlang,exec) importFrom(rlang,expr) +importFrom(rlang,inform) importFrom(rlang,is_bare_character) importFrom(rlang,is_character) importFrom(rlang,is_empty) From 3d9534e4d65bb2861844cbcd57715f75989f8951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 11:11:26 +0000 Subject: [PATCH 08/55] account for POSIXct vectors without the `"tzone"` attribute --- r/R/array.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/r/R/array.R b/r/R/array.R index 75d100bd19f..d3973b4bf2d 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -190,7 +190,10 @@ Array$create <- function(x, type = NULL) { return(out) } - if (inherits(x, "POSIXct") && attr(x, "tzone") == "") { + if (inherits(x, "POSIXct") && + # the POSIXct vector may not have the "tzone" attribute (e.g. it was + # created with `strptime()`) + (attr(x, "tzone") == "" | is.null(attr(x, "tzone")))) { attr(x, "tzone") <- Sys.timezone() inform("You have not supplied a timezone to x") } From f38a2938fdeb1302c3f7682d42081b4d5384bc15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 11:35:21 +0000 Subject: [PATCH 09/55] use `missing()` instead of `is.null()` to check for a missing attribute --- r/R/array.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/R/array.R b/r/R/array.R index d3973b4bf2d..7afae3fa1c2 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -193,7 +193,7 @@ Array$create <- function(x, type = NULL) { if (inherits(x, "POSIXct") && # the POSIXct vector may not have the "tzone" attribute (e.g. it was # created with `strptime()`) - (attr(x, "tzone") == "" | is.null(attr(x, "tzone")))) { + (attr(x, "tzone") == "" | missing(attr(x, "tzone")))) { attr(x, "tzone") <- Sys.timezone() inform("You have not supplied a timezone to x") } From 0238aa598b5b081fdbd5ed2c63730e182243e583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 11:52:35 +0000 Subject: [PATCH 10/55] add a test for message and do not use `missing()` to check for missing attributes --- r/R/array.R | 4 ++-- r/tests/testthat/test-Array.R | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/r/R/array.R b/r/R/array.R index 7afae3fa1c2..ef9eb71eb46 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -190,10 +190,10 @@ Array$create <- function(x, type = NULL) { return(out) } - if (inherits(x, "POSIXct") && + if (inherits(x, "POSIXct") && (attr(x, "tzone") == "" | # the POSIXct vector may not have the "tzone" attribute (e.g. it was # created with `strptime()`) - (attr(x, "tzone") == "" | missing(attr(x, "tzone")))) { + !("tzone" %in% names(attributes(x))))) { attr(x, "tzone") <- Sys.timezone() inform("You have not supplied a timezone to x") } diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 9b268cd0d66..38ad382a36a 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -1087,4 +1087,8 @@ test_that("Array coverts timestamps with missing timezone /assumed local tz corr Array$create(b) ) }) + expect_message( + Array$create(b), + regexp = "You have not supplied a timezone" + ) }) From 1a5fe71e59a0af8ff5fa7cb890bc23f1df5928dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 13:27:50 +0000 Subject: [PATCH 11/55] use `any()` instead of `|` when looking for a missing `"tzone"` attribute --- r/R/array.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/r/R/array.R b/r/R/array.R index ef9eb71eb46..d730482501a 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -190,10 +190,10 @@ Array$create <- function(x, type = NULL) { return(out) } - if (inherits(x, "POSIXct") && (attr(x, "tzone") == "" | + if (inherits(x, "POSIXct") && any(attr(x, "tzone") == "", # the POSIXct vector may not have the "tzone" attribute (e.g. it was # created with `strptime()`) - !("tzone" %in% names(attributes(x))))) { + is.null(attr(x, "tzone")))) { attr(x, "tzone") <- Sys.timezone() inform("You have not supplied a timezone to x") } From 2a2065abd54ab87515290ce9378380e9f03d0422 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 14:08:05 +0000 Subject: [PATCH 12/55] remove tests for array from POSIXct without timezone --- r/tests/testthat/test-Array.R | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 38ad382a36a..fb3542a06a2 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -260,11 +260,14 @@ test_that("array supports POSIXct (ARROW-3340)", { expect_array_roundtrip(times2, timestamp("us", "US/Eastern")) }) -test_that("array supports POSIXct without timezone", { - # Make sure timezone is not set - withr::with_envvar(c(TZ = ""), { +test_that("array uses local timezone for POSIXct without timezone", { + expect_message( + Array$create(as.POSIXct("1970-01-01 00:00:15")), + regexp = "You have not supplied a timezone" + ) + withr::with_envvar(c(TZ = "Asia/Ulaanbaatar"), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 - expect_array_roundtrip(times, timestamp("us", "")) + expect_array_roundtrip(times, timestamp("us", "Asia/Ulaanbaatar")) # Also test the INTSXP code path skip("Ingest_POSIXct only implemented for REALSXP") @@ -1077,18 +1080,3 @@ test_that("Array to C-interface", { delete_arrow_schema(schema_ptr) delete_arrow_array(array_ptr) }) - -test_that("Array coverts timestamps with missing timezone /assumed local tz correctly", { - withr::with_envvar(c(TZ = "America/Chicago"), { - a <- as.POSIXct("1970-01-01 00:00:15", tz = "America/Chicago") - b <- as.POSIXct("1970-01-01 00:00:15") - expect_equal( - Array$create(a), - Array$create(b) - ) - }) - expect_message( - Array$create(b), - regexp = "You have not supplied a timezone" - ) -}) From 69df9270025fdd65123e0a613a6ca48fab14baf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 15:01:13 +0000 Subject: [PATCH 13/55] no longer inform --- r/NAMESPACE | 1 - r/R/array.R | 1 - 2 files changed, 2 deletions(-) diff --git a/r/NAMESPACE b/r/NAMESPACE index feaf2db4990..72f71b98f69 100644 --- a/r/NAMESPACE +++ b/r/NAMESPACE @@ -338,7 +338,6 @@ importFrom(rlang,env_bind) importFrom(rlang,eval_tidy) importFrom(rlang,exec) importFrom(rlang,expr) -importFrom(rlang,inform) importFrom(rlang,is_bare_character) importFrom(rlang,is_character) importFrom(rlang,is_empty) diff --git a/r/R/array.R b/r/R/array.R index d730482501a..f7429fea974 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -195,7 +195,6 @@ Array$create <- function(x, type = NULL) { # created with `strptime()`) is.null(attr(x, "tzone")))) { attr(x, "tzone") <- Sys.timezone() - inform("You have not supplied a timezone to x") } if (is.null(type)) { From 135af60cd8e6876b97020005168143f4584c3762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 16:22:22 +0000 Subject: [PATCH 14/55] remove message expectation --- r/tests/testthat/test-Array.R | 4 ---- 1 file changed, 4 deletions(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index fb3542a06a2..71bd721f4b5 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -261,10 +261,6 @@ test_that("array supports POSIXct (ARROW-3340)", { }) test_that("array uses local timezone for POSIXct without timezone", { - expect_message( - Array$create(as.POSIXct("1970-01-01 00:00:15")), - regexp = "You have not supplied a timezone" - ) withr::with_envvar(c(TZ = "Asia/Ulaanbaatar"), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 expect_array_roundtrip(times, timestamp("us", "Asia/Ulaanbaatar")) From ea4ef8798694a8804170e89f0585910626aab92a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 16:25:25 +0000 Subject: [PATCH 15/55] remove inaccurate comment --- r/R/array.R | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/r/R/array.R b/r/R/array.R index f7429fea974..67f6730896b 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -190,10 +190,8 @@ Array$create <- function(x, type = NULL) { return(out) } - if (inherits(x, "POSIXct") && any(attr(x, "tzone") == "", - # the POSIXct vector may not have the "tzone" attribute (e.g. it was - # created with `strptime()`) - is.null(attr(x, "tzone")))) { + if (inherits(x, "POSIXct") && + any(attr(x, "tzone") == "", is.null(attr(x, "tzone")))) { attr(x, "tzone") <- Sys.timezone() } From d9aa10a3376a482ee8d686a230a5593ee5b47486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 20:55:52 +0000 Subject: [PATCH 16/55] additional unit tests for POSIXct -> array --- r/tests/testthat/test-Array.R | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 71bd721f4b5..2c66d5b87a0 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -261,9 +261,9 @@ test_that("array supports POSIXct (ARROW-3340)", { }) test_that("array uses local timezone for POSIXct without timezone", { - withr::with_envvar(c(TZ = "Asia/Ulaanbaatar"), { + withr::with_envvar(c(TZ = ""), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 - expect_array_roundtrip(times, timestamp("us", "Asia/Ulaanbaatar")) + expect_array_roundtrip(times, timestamp("us", Sys.timezone())) # Also test the INTSXP code path skip("Ingest_POSIXct only implemented for REALSXP") @@ -271,6 +271,14 @@ test_that("array uses local timezone for POSIXct without timezone", { attributes(times_int) <- attributes(times) expect_array_roundtrip(times_int, timestamp("us", "")) }) + withr::with_envvar(c(TZ = "Asia/Ulaanbaatar"), { + times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 + expect_array_roundtrip(times, timestamp("us", "Asia/Ulaanbaatar")) + }) + withr::with_envvar(c(TZ = NA), { + times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 + expect_array_roundtrip(times, timestamp("us", Sys.timezone())) + }) }) test_that("Timezone handling in Arrow roundtrip (ARROW-3543)", { From 0bd4eabece896526b29206f9029de5786392fcdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 26 Jan 2022 21:12:15 +0000 Subject: [PATCH 17/55] switch test to `"Pacific/Marquesas"` as improbable timezone --- r/tests/testthat/test-Array.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 2c66d5b87a0..7dee40dbea9 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -273,7 +273,7 @@ test_that("array uses local timezone for POSIXct without timezone", { }) withr::with_envvar(c(TZ = "Asia/Ulaanbaatar"), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 - expect_array_roundtrip(times, timestamp("us", "Asia/Ulaanbaatar")) + expect_array_roundtrip(times, timestamp("us", "Pacific/Marquesas")) }) withr::with_envvar(c(TZ = NA), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 From e1b1199466d0d45cce6e4a1d325109c29693855c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 1 Feb 2022 10:51:37 +0000 Subject: [PATCH 18/55] remove handling of missing or empty tzone from Array --- r/R/array.R | 5 ----- 1 file changed, 5 deletions(-) diff --git a/r/R/array.R b/r/R/array.R index 67f6730896b..965e3bfc33d 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -190,11 +190,6 @@ Array$create <- function(x, type = NULL) { return(out) } - if (inherits(x, "POSIXct") && - any(attr(x, "tzone") == "", is.null(attr(x, "tzone")))) { - attr(x, "tzone") <- Sys.timezone() - } - if (is.null(type)) { return(vec_to_Array(x, type)) } From 5466d39da4907e6df1ac9961a96776151d213729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 1 Feb 2022 10:57:26 +0000 Subject: [PATCH 19/55] changed rare timezone in tests to `"Pacific/Marquesas"` --- r/tests/testthat/test-Array.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 7dee40dbea9..b11d7e0d385 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -271,7 +271,7 @@ test_that("array uses local timezone for POSIXct without timezone", { attributes(times_int) <- attributes(times) expect_array_roundtrip(times_int, timestamp("us", "")) }) - withr::with_envvar(c(TZ = "Asia/Ulaanbaatar"), { + withr::with_envvar(c(TZ = "Pacific/Marquesas"), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 expect_array_roundtrip(times, timestamp("us", "Pacific/Marquesas")) }) From 2742681d1f3162629551ea3cd7c04e3064d4c213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 1 Feb 2022 10:57:47 +0000 Subject: [PATCH 20/55] added systzone symbol --- r/src/arrow_cpp11.h | 1 + r/src/symbols.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/r/src/arrow_cpp11.h b/r/src/arrow_cpp11.h index f1338c02ca6..43be1e3c8cd 100644 --- a/r/src/arrow_cpp11.h +++ b/r/src/arrow_cpp11.h @@ -176,6 +176,7 @@ struct symbols { static SEXP new_; static SEXP create; static SEXP arrow; + static SEXP systzone; }; struct data { diff --git a/r/src/symbols.cpp b/r/src/symbols.cpp index 0cb32c462d3..eb46647736d 100644 --- a/r/src/symbols.cpp +++ b/r/src/symbols.cpp @@ -34,6 +34,7 @@ SEXP symbols::arrow_attributes = Rf_install("arrow_attributes"); SEXP symbols::new_ = Rf_install("new"); SEXP symbols::create = Rf_install("create"); SEXP symbols::arrow = Rf_install("arrow"); +SEXP symbols::systzone= Rf_install("Sys.timezone"); // persistently protect `x` and return it SEXP precious(SEXP x) { From a1540dcaa54aadf5ac24f9b126a8b75a96b340cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 1 Feb 2022 10:58:47 +0000 Subject: [PATCH 21/55] update the definition of `InferArrowTypeFromVector` --- r/src/type_infer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/r/src/type_infer.cpp b/r/src/type_infer.cpp index 2568757aa2d..ca1e154e8f5 100644 --- a/r/src/type_infer.cpp +++ b/r/src/type_infer.cpp @@ -72,7 +72,8 @@ std::shared_ptr InferArrowTypeFromVector(SEXP x) { } else if (Rf_inherits(x, "POSIXct")) { auto tzone_sexp = Rf_getAttrib(x, symbols::tzone); if (Rf_isNull(tzone_sexp)) { - return timestamp(TimeUnit::MICRO); + auto systzone_sexp = symbols::systzone; + return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp, 0))); } else { return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(tzone_sexp, 0))); } From 05ebbb9488e3e55a1a8f0d8f9503347ca2622204 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 1 Feb 2022 11:02:03 +0000 Subject: [PATCH 22/55] update the type infer from `REALSXP` too --- r/src/type_infer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/r/src/type_infer.cpp b/r/src/type_infer.cpp index ca1e154e8f5..34b1ef1f448 100644 --- a/r/src/type_infer.cpp +++ b/r/src/type_infer.cpp @@ -89,7 +89,8 @@ std::shared_ptr InferArrowTypeFromVector(SEXP x) { if (Rf_inherits(x, "POSIXct")) { auto tzone_sexp = Rf_getAttrib(x, symbols::tzone); if (Rf_isNull(tzone_sexp)) { - return timestamp(TimeUnit::MICRO); + auto systzone_sexp = symbols::systzone; + return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp, 0))); } else { return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(tzone_sexp, 0))); } From 463bce3acfa31bdbc397a9fb8a40a802d817a6bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 1 Feb 2022 16:29:43 +0000 Subject: [PATCH 23/55] found what looks to be the right incantation for calling R functionality --- r/src/type_infer.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/r/src/type_infer.cpp b/r/src/type_infer.cpp index 34b1ef1f448..b1a0bd7bffb 100644 --- a/r/src/type_infer.cpp +++ b/r/src/type_infer.cpp @@ -72,7 +72,8 @@ std::shared_ptr InferArrowTypeFromVector(SEXP x) { } else if (Rf_inherits(x, "POSIXct")) { auto tzone_sexp = Rf_getAttrib(x, symbols::tzone); if (Rf_isNull(tzone_sexp)) { - auto systzone_sexp = symbols::systzone; + SEXP call = Rf_lang1(arrow::r::symbols::systzone); + SEXP systzone_sexp = Rf_eval(call, arrow::r::ns::arrow); return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp, 0))); } else { return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(tzone_sexp, 0))); @@ -89,7 +90,8 @@ std::shared_ptr InferArrowTypeFromVector(SEXP x) { if (Rf_inherits(x, "POSIXct")) { auto tzone_sexp = Rf_getAttrib(x, symbols::tzone); if (Rf_isNull(tzone_sexp)) { - auto systzone_sexp = symbols::systzone; + SEXP call = Rf_lang1(arrow::r::symbols::systzone); + SEXP systzone_sexp = Rf_eval(call, arrow::r::ns::arrow); return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp, 0))); } else { return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(tzone_sexp, 0))); From 388ff7e0ca0177cc8b99f41f59ed8098ce3b4ae0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 1 Feb 2022 18:24:01 +0000 Subject: [PATCH 24/55] stop going through `symbols::` for `Sys.timezone()` --- r/src/arrow_cpp11.h | 1 - r/src/symbols.cpp | 1 - 2 files changed, 2 deletions(-) diff --git a/r/src/arrow_cpp11.h b/r/src/arrow_cpp11.h index 43be1e3c8cd..f1338c02ca6 100644 --- a/r/src/arrow_cpp11.h +++ b/r/src/arrow_cpp11.h @@ -176,7 +176,6 @@ struct symbols { static SEXP new_; static SEXP create; static SEXP arrow; - static SEXP systzone; }; struct data { diff --git a/r/src/symbols.cpp b/r/src/symbols.cpp index eb46647736d..0cb32c462d3 100644 --- a/r/src/symbols.cpp +++ b/r/src/symbols.cpp @@ -34,7 +34,6 @@ SEXP symbols::arrow_attributes = Rf_install("arrow_attributes"); SEXP symbols::new_ = Rf_install("new"); SEXP symbols::create = Rf_install("create"); SEXP symbols::arrow = Rf_install("arrow"); -SEXP symbols::systzone= Rf_install("Sys.timezone"); // persistently protect `x` and return it SEXP precious(SEXP x) { From 79460d130e21ee94d46a99929985d6df0e035a62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 1 Feb 2022 18:24:21 +0000 Subject: [PATCH 25/55] call `Sys.timezone()` directly --- r/src/type_infer.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/r/src/type_infer.cpp b/r/src/type_infer.cpp index b1a0bd7bffb..75b1e85c426 100644 --- a/r/src/type_infer.cpp +++ b/r/src/type_infer.cpp @@ -72,9 +72,8 @@ std::shared_ptr InferArrowTypeFromVector(SEXP x) { } else if (Rf_inherits(x, "POSIXct")) { auto tzone_sexp = Rf_getAttrib(x, symbols::tzone); if (Rf_isNull(tzone_sexp)) { - SEXP call = Rf_lang1(arrow::r::symbols::systzone); - SEXP systzone_sexp = Rf_eval(call, arrow::r::ns::arrow); - return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp, 0))); + auto systzone_sexp = cpp11::package("base")["Sys.timezone"]; + return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp(), 0))); } else { return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(tzone_sexp, 0))); } @@ -90,9 +89,8 @@ std::shared_ptr InferArrowTypeFromVector(SEXP x) { if (Rf_inherits(x, "POSIXct")) { auto tzone_sexp = Rf_getAttrib(x, symbols::tzone); if (Rf_isNull(tzone_sexp)) { - SEXP call = Rf_lang1(arrow::r::symbols::systzone); - SEXP systzone_sexp = Rf_eval(call, arrow::r::ns::arrow); - return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp, 0))); + auto systzone_sexp = cpp11::package("base")["Sys.timezone"]; + return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(systzone_sexp(), 0))); } else { return timestamp(TimeUnit::MICRO, CHAR(STRING_ELT(tzone_sexp, 0))); } From 6b2705982d122c27c33e95d7df56d1fa05035b1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 2 Feb 2022 11:51:58 +0000 Subject: [PATCH 26/55] testing `TZ = NA` and `TZ = NULL` both result in a `NULL` `"tzone"` attribute --- r/tests/testthat/test-Array.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index b11d7e0d385..bbb9917f331 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -263,6 +263,7 @@ test_that("array supports POSIXct (ARROW-3340)", { test_that("array uses local timezone for POSIXct without timezone", { withr::with_envvar(c(TZ = ""), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 + expect_equal(attr(times, "tzone"), NULL) expect_array_roundtrip(times, timestamp("us", Sys.timezone())) # Also test the INTSXP code path @@ -273,10 +274,12 @@ test_that("array uses local timezone for POSIXct without timezone", { }) withr::with_envvar(c(TZ = "Pacific/Marquesas"), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 + expect_equal(attr(times, "tzone"), "Pacific/Marquesas") expect_array_roundtrip(times, timestamp("us", "Pacific/Marquesas")) }) withr::with_envvar(c(TZ = NA), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 + expect_equal(attr(times, "tzone"), NULL) expect_array_roundtrip(times, timestamp("us", Sys.timezone())) }) }) From 6a633b91508c4ad884f9d472a07e4c56f6d2d865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 00:57:49 -0930 Subject: [PATCH 27/55] skip 1 test on windows --- r/tests/testthat/test-dplyr-funcs-datetime.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index a7a705678c1..5a54902584e 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -298,6 +298,9 @@ test_that("is.* functions from lubridate", { test_df ) + # fails with "Cannot locate timezone 'UTC': Timezone database not found at "\tzdata"." + # This indicates dependency on ARROW-13168 + skip_on_os("windows") compare_dplyr_binding( .input %>% mutate( From 09f338253b7447c82b6cefeba733d987df64073f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 02:10:45 -0930 Subject: [PATCH 28/55] skip test 2 on windows --- r/tests/testthat/test-dplyr-funcs-datetime.R | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 5a54902584e..3c175a3490e 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -298,9 +298,7 @@ test_that("is.* functions from lubridate", { test_df ) - # fails with "Cannot locate timezone 'UTC': Timezone database not found at "\tzdata"." - # This indicates dependency on ARROW-13168 - skip_on_os("windows") + skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate( @@ -658,13 +656,6 @@ test_that("extract yday from date", { test_that("leap_year mirror lubridate", { - compare_dplyr_binding( - .input %>% - mutate(x = leap_year(date)) %>% - collect(), - test_df - ) - compare_dplyr_binding( .input %>% mutate(x = leap_year(datetime)) %>% @@ -686,6 +677,13 @@ test_that("leap_year mirror lubridate", { ) ) + skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 + compare_dplyr_binding( + .input %>% + mutate(x = leap_year(date)) %>% + collect(), + test_df + ) }) test_that("am/pm mirror lubridate", { From 5d455a9a8ff5a84d0fd32c3a582985f7bd78a63e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 02:17:47 -0930 Subject: [PATCH 29/55] skip test 3 on windows --- r/tests/testthat/test-dplyr-funcs-datetime.R | 1 + 1 file changed, 1 insertion(+) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 3c175a3490e..44fe6a23b59 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -473,6 +473,7 @@ test_that("extract hour from timestamp", { }) test_that("extract minute from timestamp", { + skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = minute(datetime)) %>% From 18168617205bdda870164d28875fdb8b6844c8d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 13:28:15 +0000 Subject: [PATCH 30/55] skip test 4 & 5 on windows --- r/tests/testthat/test-dplyr-funcs-datetime.R | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 44fe6a23b59..936aad41c42 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -647,6 +647,7 @@ test_that("extract mday from date", { }) test_that("extract yday from date", { + skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = yday(date)) %>% @@ -657,13 +658,6 @@ test_that("extract yday from date", { test_that("leap_year mirror lubridate", { - compare_dplyr_binding( - .input %>% - mutate(x = leap_year(datetime)) %>% - collect(), - test_df - ) - compare_dplyr_binding( .input %>% mutate(x = leap_year(test_year)) %>% @@ -685,6 +679,13 @@ test_that("leap_year mirror lubridate", { collect(), test_df ) + + compare_dplyr_binding( + .input %>% + mutate(x = leap_year(datetime)) %>% + collect(), + test_df + ) }) test_that("am/pm mirror lubridate", { From bf5b3746f5798bb1c0f255e0ec925a86b09724a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 14:13:59 +0000 Subject: [PATCH 31/55] skip tests 6, 7, 8, 9 & 10 on windows --- r/tests/testthat/test-dplyr-funcs-datetime.R | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 936aad41c42..43ad5cf7a2d 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -407,27 +407,26 @@ test_that("extract day from timestamp", { test_that("extract wday from timestamp", { compare_dplyr_binding( .input %>% - mutate(x = wday(datetime)) %>% + mutate(x = wday(date, week_start = 3)) %>% collect(), test_df ) compare_dplyr_binding( .input %>% - mutate(x = wday(date, week_start = 3)) %>% + mutate(x = wday(date, week_start = 1)) %>% collect(), test_df ) + skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% - mutate(x = wday(date, week_start = 1)) %>% + mutate(x = wday(datetime)) %>% collect(), test_df ) - skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 - compare_dplyr_binding( .input %>% mutate(x = wday(date, label = TRUE)) %>% @@ -446,6 +445,7 @@ test_that("extract wday from timestamp", { }) test_that("extract mday from timestamp", { + skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = mday(datetime)) %>% @@ -455,6 +455,7 @@ test_that("extract mday from timestamp", { }) test_that("extract yday from timestamp", { + skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = yday(datetime)) %>% @@ -464,6 +465,7 @@ test_that("extract yday from timestamp", { }) test_that("extract hour from timestamp", { + skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = hour(datetime)) %>% @@ -483,6 +485,7 @@ test_that("extract minute from timestamp", { }) test_that("extract second from timestamp", { + skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = second(datetime)) %>% From f9552f45121422ee0bc7010d5beac69d084fae19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 14:57:46 +0000 Subject: [PATCH 32/55] skip tests 11, 12, 13, and 14 on windows --- r/tests/testthat/test-dplyr-funcs-datetime.R | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 43ad5cf7a2d..bc769b673c9 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -341,13 +341,6 @@ test_that("extract quarter from timestamp", { }) test_that("extract month from timestamp", { - compare_dplyr_binding( - .input %>% - mutate(x = month(datetime)) %>% - collect(), - test_df - ) - skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( @@ -366,9 +359,17 @@ test_that("extract month from timestamp", { test_df, ignore_attr = TRUE ) + + compare_dplyr_binding( + .input %>% + mutate(x = month(datetime)) %>% + collect(), + test_df + ) }) test_that("extract isoweek from timestamp", { + skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = isoweek(datetime)) %>% @@ -378,6 +379,7 @@ test_that("extract isoweek from timestamp", { }) test_that("extract epiweek from timestamp", { + skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = epiweek(datetime)) %>% @@ -387,6 +389,7 @@ test_that("extract epiweek from timestamp", { }) test_that("extract week from timestamp", { + skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = week(datetime)) %>% From b0ffa4f86727d34b438686d408dc7eabffcef104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 15:53:59 +0000 Subject: [PATCH 33/55] checking to see if `ignore_attr` helps when comparing objects --- r/tests/testthat/test-dplyr-funcs-datetime.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index bc769b673c9..7d6e2fc1eef 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -648,7 +648,8 @@ test_that("extract mday from date", { .input %>% mutate(x = mday(date)) %>% collect(), - test_df + test_df, + ignore_attr = TRUE # can be removed after https://issues.apache.org/jira/browse/ARROW-13168 ) }) From 68ccb24b85ef8e0865a95df71af6d4f7776269cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 18:38:18 +0000 Subject: [PATCH 34/55] ignore attributes only on windows --- r/tests/testthat/test-dplyr-funcs-datetime.R | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 7d6e2fc1eef..e9fcaad00ad 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -621,7 +621,9 @@ test_that("extract wday from date", { .input %>% mutate(x = wday(date, week_start = 1)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = ifelse(tolower(Sys.info()[["sysname"]]) == "windows", TRUE, FALSE) ) skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 @@ -649,7 +651,8 @@ test_that("extract mday from date", { mutate(x = mday(date)) %>% collect(), test_df, - ignore_attr = TRUE # can be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = ifelse(tolower(Sys.info()[["sysname"]]) == "windows", TRUE, FALSE) ) }) From 8bac0476310d7886dce48090687178f283670159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 19:49:27 +0000 Subject: [PATCH 35/55] one more ignore attributes on windows --- r/tests/testthat/test-dplyr-funcs-datetime.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index e9fcaad00ad..b6c56fdd579 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -614,7 +614,9 @@ test_that("extract wday from date", { .input %>% mutate(x = wday(date, week_start = 3)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = ifelse(tolower(Sys.info()[["sysname"]]) == "windows", TRUE, FALSE) ) compare_dplyr_binding( From e3f6a5b6505acbcac6328e96bd8d08ec6840d3f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 20:04:09 +0000 Subject: [PATCH 36/55] defined and used `on_windows()` to selectively ignore attributes in expectations --- r/tests/testthat/helper-skip.R | 4 ++++ r/tests/testthat/test-dplyr-funcs-datetime.R | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/r/tests/testthat/helper-skip.R b/r/tests/testthat/helper-skip.R index df0de777258..f0761d1ce7f 100644 --- a/r/tests/testthat/helper-skip.R +++ b/r/tests/testthat/helper-skip.R @@ -78,3 +78,7 @@ process_is_running <- function(x) { cmd <- sprintf("ps aux | grep '%s' | grep -v grep", x) tryCatch(system(cmd, ignore.stdout = TRUE) == 0, error = function(e) FALSE) } + +on_windows <- function() { + ifelse(tolower(Sys.info()[["sysname"]]) == "windows", TRUE, FALSE) +} diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index b6c56fdd579..273064a697d 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -616,7 +616,7 @@ test_that("extract wday from date", { collect(), test_df, # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = ifelse(tolower(Sys.info()[["sysname"]]) == "windows", TRUE, FALSE) + ignore_attr = on_windows() ) compare_dplyr_binding( @@ -625,7 +625,7 @@ test_that("extract wday from date", { collect(), test_df, # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = ifelse(tolower(Sys.info()[["sysname"]]) == "windows", TRUE, FALSE) + ignore_attr = on_windows() ) skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 @@ -654,7 +654,7 @@ test_that("extract mday from date", { collect(), test_df, # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = ifelse(tolower(Sys.info()[["sysname"]]) == "windows", TRUE, FALSE) + ignore_attr = on_windows() ) }) From 28d193f8d685ef85acb1b855c9d0aedec3c23c3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 20:31:29 +0000 Subject: [PATCH 37/55] ignoring attributes for some more of the failing tests --- r/tests/testthat/test-dplyr-funcs-datetime.R | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 273064a697d..8c55dbd155e 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -598,7 +598,9 @@ test_that("extract day from date", { .input %>% mutate(x = day(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) @@ -607,7 +609,9 @@ test_that("extract wday from date", { .input %>% mutate(x = wday(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) compare_dplyr_binding( From 528647aeaf495cc1cb746b729b619caef6c5a4a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 21:14:47 +0000 Subject: [PATCH 38/55] some more attributes ignored --- r/tests/testthat/test-dplyr-funcs-datetime.R | 32 +++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 8c55dbd155e..6ecc7625c6f 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -506,7 +506,9 @@ test_that("extract year from date", { .input %>% mutate(x = year(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) @@ -515,7 +517,9 @@ test_that("extract isoyear from date", { .input %>% mutate(x = isoyear(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) @@ -524,7 +528,9 @@ test_that("extract quarter from date", { .input %>% mutate(x = quarter(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) @@ -533,7 +539,9 @@ test_that("extract month from date", { .input %>% mutate(x = month(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) @@ -542,7 +550,9 @@ test_that("extract isoweek from date", { .input %>% mutate(x = isoweek(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) @@ -551,7 +561,9 @@ test_that("extract epiweek from date", { .input %>% mutate(x = epiweek(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) @@ -560,7 +572,9 @@ test_that("extract week from date", { .input %>% mutate(x = week(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) @@ -569,7 +583,9 @@ test_that("extract month from date", { .input %>% mutate(x = month(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 From 5638c6bcde664dec9a29e7d594e4677fbb2c6865 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 4 Feb 2022 21:55:31 +0000 Subject: [PATCH 39/55] ignoring more attribute checks on windows --- r/tests/testthat/test-dplyr-funcs-datetime.R | 24 +++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 6ecc7625c6f..6b0fbdfe420 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -277,14 +277,18 @@ test_that("is.* functions from lubridate", { .input %>% mutate(x = is.POSIXct(datetime), y = is.POSIXct(integer)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) compare_dplyr_binding( .input %>% mutate(x = is.Date(date), y = is.Date(integer)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) compare_dplyr_binding( @@ -295,7 +299,9 @@ test_that("is.* functions from lubridate", { z = is.instant(integer) ) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 @@ -403,7 +409,9 @@ test_that("extract day from timestamp", { .input %>% mutate(x = day(datetime)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) @@ -412,14 +420,18 @@ test_that("extract wday from timestamp", { .input %>% mutate(x = wday(date, week_start = 3)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) compare_dplyr_binding( .input %>% mutate(x = wday(date, week_start = 1)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 From 166c7086cfb55a4c2870a7d5b963b8c42ead3f9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Mon, 7 Feb 2022 09:07:59 +0000 Subject: [PATCH 40/55] un-skip tests on win --- r/tests/testthat/test-dplyr-funcs-datetime.R | 34 ++++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 6b0fbdfe420..5a0e401cf2e 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -347,6 +347,13 @@ test_that("extract quarter from timestamp", { }) test_that("extract month from timestamp", { + compare_dplyr_binding( + .input %>% + mutate(x = month(datetime)) %>% + collect(), + test_df + ) + skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( @@ -365,17 +372,10 @@ test_that("extract month from timestamp", { test_df, ignore_attr = TRUE ) - - compare_dplyr_binding( - .input %>% - mutate(x = month(datetime)) %>% - collect(), - test_df - ) }) test_that("extract isoweek from timestamp", { - skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = isoweek(datetime)) %>% @@ -385,7 +385,7 @@ test_that("extract isoweek from timestamp", { }) test_that("extract epiweek from timestamp", { - skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = epiweek(datetime)) %>% @@ -395,7 +395,7 @@ test_that("extract epiweek from timestamp", { }) test_that("extract week from timestamp", { - skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = week(datetime)) %>% @@ -434,7 +434,7 @@ test_that("extract wday from timestamp", { ignore_attr = on_windows() ) - skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = wday(datetime)) %>% @@ -460,7 +460,7 @@ test_that("extract wday from timestamp", { }) test_that("extract mday from timestamp", { - skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = mday(datetime)) %>% @@ -470,7 +470,7 @@ test_that("extract mday from timestamp", { }) test_that("extract yday from timestamp", { - skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = yday(datetime)) %>% @@ -480,7 +480,7 @@ test_that("extract yday from timestamp", { }) test_that("extract hour from timestamp", { - skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = hour(datetime)) %>% @@ -490,7 +490,7 @@ test_that("extract hour from timestamp", { }) test_that("extract minute from timestamp", { - skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = minute(datetime)) %>% @@ -500,7 +500,7 @@ test_that("extract minute from timestamp", { }) test_that("extract second from timestamp", { - skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = second(datetime)) %>% @@ -691,7 +691,7 @@ test_that("extract mday from date", { }) test_that("extract yday from date", { - skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = yday(date)) %>% From 6ca5a4ea8474f2362ae69dabf5528a1b214ce646 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 8 Feb 2022 11:57:43 +0000 Subject: [PATCH 41/55] ignore attributes on windows for yday extractions --- r/tests/testthat/test-dplyr-funcs-datetime.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 5a0e401cf2e..7478e9cb845 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -696,7 +696,9 @@ test_that("extract yday from date", { .input %>% mutate(x = yday(date)) %>% collect(), - test_df + test_df, + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + ignore_attr = on_windows() ) }) From 41b580f9db5fd9a09073fad553bdccba62f72a78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 22 Feb 2022 09:39:22 +0000 Subject: [PATCH 42/55] added a bunch of TODOs related to ARROW-13168 --- r/tests/testthat/test-dplyr-funcs-datetime.R | 40 ++++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 7478e9cb845..ba102d48c67 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -278,7 +278,7 @@ test_that("is.* functions from lubridate", { mutate(x = is.POSIXct(datetime), y = is.POSIXct(integer)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -287,7 +287,7 @@ test_that("is.* functions from lubridate", { mutate(x = is.Date(date), y = is.Date(integer)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -300,7 +300,7 @@ test_that("is.* functions from lubridate", { ) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -410,7 +410,7 @@ test_that("extract day from timestamp", { mutate(x = day(datetime)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -421,7 +421,7 @@ test_that("extract wday from timestamp", { mutate(x = wday(date, week_start = 3)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -430,7 +430,7 @@ test_that("extract wday from timestamp", { mutate(x = wday(date, week_start = 1)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -519,7 +519,7 @@ test_that("extract year from date", { mutate(x = year(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -530,7 +530,7 @@ test_that("extract isoyear from date", { mutate(x = isoyear(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -541,7 +541,7 @@ test_that("extract quarter from date", { mutate(x = quarter(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -552,7 +552,7 @@ test_that("extract month from date", { mutate(x = month(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -563,7 +563,7 @@ test_that("extract isoweek from date", { mutate(x = isoweek(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -574,7 +574,7 @@ test_that("extract epiweek from date", { mutate(x = epiweek(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -585,7 +585,7 @@ test_that("extract week from date", { mutate(x = week(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -596,7 +596,7 @@ test_that("extract month from date", { mutate(x = month(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -627,7 +627,7 @@ test_that("extract day from date", { mutate(x = day(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -638,7 +638,7 @@ test_that("extract wday from date", { mutate(x = wday(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -647,7 +647,7 @@ test_that("extract wday from date", { mutate(x = wday(date, week_start = 3)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -656,7 +656,7 @@ test_that("extract wday from date", { mutate(x = wday(date, week_start = 1)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -685,7 +685,7 @@ test_that("extract mday from date", { mutate(x = mday(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -697,7 +697,7 @@ test_that("extract yday from date", { mutate(x = yday(date)) %>% collect(), test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 + # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) From daac293b74ed70d6d138d5eefb0fd256cb3bcbd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 22 Feb 2022 10:26:47 +0000 Subject: [PATCH 43/55] trying something --- r/tests/testthat/test-dplyr-funcs-datetime.R | 50 ++++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index ba102d48c67..257dd1b76bd 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -27,11 +27,11 @@ library(dplyr, warn.conflicts = FALSE) withr::local_timezone("UTC") # TODO: We should test on windows once ARROW-13168 is resolved. -if (tolower(Sys.info()[["sysname"]]) == "windows") { - test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "") -} else { +# if (tolower(Sys.info()[["sysname"]]) == "windows") { +# test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "") +# } else { test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "Pacific/Marquesas") -} +# } test_df <- tibble::tibble( @@ -278,7 +278,7 @@ test_that("is.* functions from lubridate", { mutate(x = is.POSIXct(datetime), y = is.POSIXct(integer)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -287,7 +287,7 @@ test_that("is.* functions from lubridate", { mutate(x = is.Date(date), y = is.Date(integer)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -300,7 +300,7 @@ test_that("is.* functions from lubridate", { ) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -410,7 +410,7 @@ test_that("extract day from timestamp", { mutate(x = day(datetime)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -421,7 +421,7 @@ test_that("extract wday from timestamp", { mutate(x = wday(date, week_start = 3)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -430,7 +430,7 @@ test_that("extract wday from timestamp", { mutate(x = wday(date, week_start = 1)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -519,7 +519,7 @@ test_that("extract year from date", { mutate(x = year(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -530,7 +530,7 @@ test_that("extract isoyear from date", { mutate(x = isoyear(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -541,7 +541,7 @@ test_that("extract quarter from date", { mutate(x = quarter(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -552,7 +552,7 @@ test_that("extract month from date", { mutate(x = month(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -563,7 +563,7 @@ test_that("extract isoweek from date", { mutate(x = isoweek(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -574,7 +574,7 @@ test_that("extract epiweek from date", { mutate(x = epiweek(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -585,7 +585,7 @@ test_that("extract week from date", { mutate(x = week(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -596,7 +596,7 @@ test_that("extract month from date", { mutate(x = month(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -627,7 +627,7 @@ test_that("extract day from date", { mutate(x = day(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -638,7 +638,7 @@ test_that("extract wday from date", { mutate(x = wday(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -647,7 +647,7 @@ test_that("extract wday from date", { mutate(x = wday(date, week_start = 3)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -656,7 +656,7 @@ test_that("extract wday from date", { mutate(x = wday(date, week_start = 1)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) @@ -685,7 +685,7 @@ test_that("extract mday from date", { mutate(x = mday(date)) %>% collect(), test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 + # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 ignore_attr = on_windows() ) }) @@ -696,9 +696,7 @@ test_that("extract yday from date", { .input %>% mutate(x = yday(date)) %>% collect(), - test_df, - # TODO remove the ignore step after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) From 04150489184f7c5477d509d3a1d884542b2ed7cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 22 Feb 2022 14:08:09 +0000 Subject: [PATCH 44/55] skip 1 test + datetime column OS aware --- r/tests/testthat/test-dplyr-funcs-datetime.R | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 257dd1b76bd..634ba32879e 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -27,11 +27,11 @@ library(dplyr, warn.conflicts = FALSE) withr::local_timezone("UTC") # TODO: We should test on windows once ARROW-13168 is resolved. -# if (tolower(Sys.info()[["sysname"]]) == "windows") { -# test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "") -# } else { +if (tolower(Sys.info()[["sysname"]]) == "windows") { + test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "") +} else { test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "Pacific/Marquesas") -# } +} test_df <- tibble::tibble( @@ -500,7 +500,7 @@ test_that("extract minute from timestamp", { }) test_that("extract second from timestamp", { - # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 + skip_on_os("windows") # TODO remove after https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = second(datetime)) %>% From d543415d54fc23c359ec584d43e4de712a35bb0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Tue, 22 Feb 2022 14:12:18 +0000 Subject: [PATCH 45/55] try Jon's suggestion --- r/tests/testthat/test-dplyr-funcs-datetime.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 634ba32879e..53652f14e23 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -28,7 +28,7 @@ withr::local_timezone("UTC") # TODO: We should test on windows once ARROW-13168 is resolved. if (tolower(Sys.info()[["sysname"]]) == "windows") { - test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "") + test_date <- as.POSIXct("2017-01-01 00:00:11.3456789")#, tz = "") } else { test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "Pacific/Marquesas") } @@ -500,7 +500,7 @@ test_that("extract minute from timestamp", { }) test_that("extract second from timestamp", { - skip_on_os("windows") # TODO remove after https://issues.apache.org/jira/browse/ARROW-13168 + # skip_on_os("windows") # TODO remove after https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = second(datetime)) %>% From 3343026771a07c030fe07c607316da04227b842c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 30 Mar 2022 11:00:02 +0100 Subject: [PATCH 46/55] some cleanup --- r/tests/testthat/test-dplyr-funcs-datetime.R | 53 +++++--------------- 1 file changed, 13 insertions(+), 40 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 53652f14e23..40e8e9dc05c 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -518,9 +518,7 @@ test_that("extract year from date", { .input %>% mutate(x = year(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) @@ -529,9 +527,7 @@ test_that("extract isoyear from date", { .input %>% mutate(x = isoyear(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) @@ -540,9 +536,7 @@ test_that("extract quarter from date", { .input %>% mutate(x = quarter(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) @@ -551,9 +545,7 @@ test_that("extract month from date", { .input %>% mutate(x = month(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) @@ -562,9 +554,7 @@ test_that("extract isoweek from date", { .input %>% mutate(x = isoweek(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) @@ -573,9 +563,7 @@ test_that("extract epiweek from date", { .input %>% mutate(x = epiweek(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) @@ -584,9 +572,7 @@ test_that("extract week from date", { .input %>% mutate(x = week(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) @@ -595,9 +581,7 @@ test_that("extract month from date", { .input %>% mutate(x = month(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 @@ -626,9 +610,7 @@ test_that("extract day from date", { .input %>% mutate(x = day(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) @@ -637,27 +619,21 @@ test_that("extract wday from date", { .input %>% mutate(x = wday(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) compare_dplyr_binding( .input %>% mutate(x = wday(date, week_start = 3)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) compare_dplyr_binding( .input %>% mutate(x = wday(date, week_start = 1)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 @@ -684,14 +660,11 @@ test_that("extract mday from date", { .input %>% mutate(x = mday(date)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) test_that("extract yday from date", { - # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = yday(date)) %>% From fa510ed13ff29444e4ab0018f2909f83ad19fb8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 30 Mar 2022 11:09:17 +0100 Subject: [PATCH 47/55] more cleanup --- r/tests/testthat/test-dplyr-funcs-datetime.R | 35 ++++---------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 40e8e9dc05c..9202499e351 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -277,18 +277,14 @@ test_that("is.* functions from lubridate", { .input %>% mutate(x = is.POSIXct(datetime), y = is.POSIXct(integer)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) compare_dplyr_binding( .input %>% mutate(x = is.Date(date), y = is.Date(integer)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) compare_dplyr_binding( @@ -299,12 +295,9 @@ test_that("is.* functions from lubridate", { z = is.instant(integer) ) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) - skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate( @@ -375,7 +368,6 @@ test_that("extract month from timestamp", { }) test_that("extract isoweek from timestamp", { - # skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = isoweek(datetime)) %>% @@ -385,7 +377,6 @@ test_that("extract isoweek from timestamp", { }) test_that("extract epiweek from timestamp", { - # skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = epiweek(datetime)) %>% @@ -395,7 +386,6 @@ test_that("extract epiweek from timestamp", { }) test_that("extract week from timestamp", { - # skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = week(datetime)) %>% @@ -409,9 +399,7 @@ test_that("extract day from timestamp", { .input %>% mutate(x = day(datetime)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) }) @@ -420,21 +408,16 @@ test_that("extract wday from timestamp", { .input %>% mutate(x = wday(date, week_start = 3)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) compare_dplyr_binding( .input %>% mutate(x = wday(date, week_start = 1)) %>% collect(), - test_df, - # the ignore step could be removed after https://issues.apache.org/jira/browse/ARROW-13168 - ignore_attr = on_windows() + test_df ) - # skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = wday(datetime)) %>% @@ -460,7 +443,6 @@ test_that("extract wday from timestamp", { }) test_that("extract mday from timestamp", { - # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = mday(datetime)) %>% @@ -470,7 +452,6 @@ test_that("extract mday from timestamp", { }) test_that("extract yday from timestamp", { - # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = yday(datetime)) %>% @@ -480,7 +461,6 @@ test_that("extract yday from timestamp", { }) test_that("extract hour from timestamp", { - # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = hour(datetime)) %>% @@ -490,7 +470,6 @@ test_that("extract hour from timestamp", { }) test_that("extract minute from timestamp", { - # skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = minute(datetime)) %>% @@ -500,7 +479,6 @@ test_that("extract minute from timestamp", { }) test_that("extract second from timestamp", { - # skip_on_os("windows") # TODO remove after https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = second(datetime)) %>% @@ -689,7 +667,6 @@ test_that("leap_year mirror lubridate", { ) ) - skip_on_os("windows") #https://issues.apache.org/jira/browse/ARROW-13168 compare_dplyr_binding( .input %>% mutate(x = leap_year(date)) %>% From d9d1d159930601af8c3f8d386d870750a60b02fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 30 Mar 2022 11:13:57 +0100 Subject: [PATCH 48/55] one more --- r/tests/testthat/test-dplyr-funcs-datetime.R | 34 +++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index 9202499e351..c99a852f731 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -28,7 +28,7 @@ withr::local_timezone("UTC") # TODO: We should test on windows once ARROW-13168 is resolved. if (tolower(Sys.info()[["sysname"]]) == "windows") { - test_date <- as.POSIXct("2017-01-01 00:00:11.3456789")#, tz = "") + test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "") } else { test_date <- as.POSIXct("2017-01-01 00:00:11.3456789", tz = "Pacific/Marquesas") } @@ -406,25 +406,27 @@ test_that("extract day from timestamp", { test_that("extract wday from timestamp", { compare_dplyr_binding( .input %>% - mutate(x = wday(date, week_start = 3)) %>% + mutate(x = wday(datetime)) %>% collect(), test_df ) compare_dplyr_binding( .input %>% - mutate(x = wday(date, week_start = 1)) %>% + mutate(x = wday(date, week_start = 3)) %>% collect(), test_df ) compare_dplyr_binding( .input %>% - mutate(x = wday(datetime)) %>% + mutate(x = wday(date, week_start = 1)) %>% collect(), test_df ) + skip_on_os("windows") # https://issues.apache.org/jira/browse/ARROW-13168 + compare_dplyr_binding( .input %>% mutate(x = wday(date, label = TRUE)) %>% @@ -655,30 +657,30 @@ test_that("leap_year mirror lubridate", { compare_dplyr_binding( .input %>% - mutate(x = leap_year(test_year)) %>% + mutate(x = leap_year(date)) %>% collect(), - data.frame( - test_year = as.Date(c( - "1998-01-01", # not leap year - "1996-01-01", # leap year (divide by 4 rule) - "1900-01-01", # not leap year (divide by 100 rule) - "2000-01-01" # leap year (divide by 400 rule) - )) - ) + test_df ) compare_dplyr_binding( .input %>% - mutate(x = leap_year(date)) %>% + mutate(x = leap_year(datetime)) %>% collect(), test_df ) compare_dplyr_binding( .input %>% - mutate(x = leap_year(datetime)) %>% + mutate(x = leap_year(test_year)) %>% collect(), - test_df + data.frame( + test_year = as.Date(c( + "1998-01-01", # not leap year + "1996-01-01", # leap year (divide by 4 rule) + "1900-01-01", # not leap year (divide by 100 rule) + "2000-01-01" # leap year (divide by 400 rule) + )) + ) ) }) From f4721032202dc95019c1156dbcc6e48f5f4645fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Wed, 30 Mar 2022 11:15:52 +0100 Subject: [PATCH 49/55] another one --- r/tests/testthat/test-dplyr-funcs-datetime.R | 4 ---- 1 file changed, 4 deletions(-) diff --git a/r/tests/testthat/test-dplyr-funcs-datetime.R b/r/tests/testthat/test-dplyr-funcs-datetime.R index c99a852f731..1618ef01e11 100644 --- a/r/tests/testthat/test-dplyr-funcs-datetime.R +++ b/r/tests/testthat/test-dplyr-funcs-datetime.R @@ -654,7 +654,6 @@ test_that("extract yday from date", { }) test_that("leap_year mirror lubridate", { - compare_dplyr_binding( .input %>% mutate(x = leap_year(date)) %>% @@ -685,7 +684,6 @@ test_that("leap_year mirror lubridate", { }) test_that("am/pm mirror lubridate", { - # https://issues.apache.org/jira/browse/ARROW-13168 skip_on_os("windows") @@ -705,8 +703,6 @@ test_that("am/pm mirror lubridate", { ), format = "%Y-%m-%d %H:%M:%S" ) - ) ) - }) From 876c483b5e8465c374ec0ca1cbb87b42be341dad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Thu, 31 Mar 2022 09:53:48 +0100 Subject: [PATCH 50/55] removed the `on_windows()` skip helper --- r/tests/testthat/helper-skip.R | 4 ---- 1 file changed, 4 deletions(-) diff --git a/r/tests/testthat/helper-skip.R b/r/tests/testthat/helper-skip.R index f0761d1ce7f..df0de777258 100644 --- a/r/tests/testthat/helper-skip.R +++ b/r/tests/testthat/helper-skip.R @@ -78,7 +78,3 @@ process_is_running <- function(x) { cmd <- sprintf("ps aux | grep '%s' | grep -v grep", x) tryCatch(system(cmd, ignore.stdout = TRUE) == 0, error = function(e) FALSE) } - -on_windows <- function() { - ifelse(tolower(Sys.info()[["sysname"]]) == "windows", TRUE, FALSE) -} From 5903fb229ca1d1ef9bdd46399b57de35e75727b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Thu, 31 Mar 2022 10:08:16 +0100 Subject: [PATCH 51/55] use `withr::with_timezone()` --- r/tests/testthat/test-Array.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index bbb9917f331..098bccea272 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -272,12 +272,14 @@ test_that("array uses local timezone for POSIXct without timezone", { attributes(times_int) <- attributes(times) expect_array_roundtrip(times_int, timestamp("us", "")) }) - withr::with_envvar(c(TZ = "Pacific/Marquesas"), { + withr::with_timezone("Pacific/Marquesas", { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 expect_equal(attr(times, "tzone"), "Pacific/Marquesas") expect_array_roundtrip(times, timestamp("us", "Pacific/Marquesas")) }) - withr::with_envvar(c(TZ = NA), { + + # and although the TZ is NULL in R, we set it to the Sys.timezone() + withr::with_timezone(NA, { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 expect_equal(attr(times, "tzone"), NULL) expect_array_roundtrip(times, timestamp("us", Sys.timezone())) From de58c8dc2ef6d9355e66ef27e8f2254e357a94e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 1 Apr 2022 13:00:35 +0100 Subject: [PATCH 52/55] erroring with `with_timezone("" ...)` instead of `with_envvar(c(TZ = "") ...)` --- r/tests/testthat/test-Array.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 098bccea272..5cafeaf2e79 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -261,7 +261,7 @@ test_that("array supports POSIXct (ARROW-3340)", { }) test_that("array uses local timezone for POSIXct without timezone", { - withr::with_envvar(c(TZ = ""), { + withr::with_timezone("", { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 expect_equal(attr(times, "tzone"), NULL) expect_array_roundtrip(times, timestamp("us", Sys.timezone())) @@ -272,6 +272,8 @@ test_that("array uses local timezone for POSIXct without timezone", { attributes(times_int) <- attributes(times) expect_array_roundtrip(times_int, timestamp("us", "")) }) + + # If there is a timezone set, we record that withr::with_timezone("Pacific/Marquesas", { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 expect_equal(attr(times, "tzone"), "Pacific/Marquesas") From e31bf0a116977b7efa4f1af00a14abb2ba5bed11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Fri, 1 Apr 2022 13:48:47 +0100 Subject: [PATCH 53/55] add test with a different timezone --- r/tests/testthat/test-Array.R | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index 5cafeaf2e79..db2ccefa89a 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -278,6 +278,13 @@ test_that("array uses local timezone for POSIXct without timezone", { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 expect_equal(attr(times, "tzone"), "Pacific/Marquesas") expect_array_roundtrip(times, timestamp("us", "Pacific/Marquesas")) + + times_with_tz <- strptime( + "2019-02-03 12:34:56", + format = "%Y-%m-%d %H:%M:%S", + tz = "Asia/Katmandu") + 1:10 + expect_equal(attr(times, "tzone"), "Asia/Katmandu") + expect_array_roundtrip(times, timestamp("us", "Asia/Katmandu")) }) # and although the TZ is NULL in R, we set it to the Sys.timezone() From e5039a8478b1a3422ff769a5aa2536ab88bcb943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Mon, 4 Apr 2022 21:24:52 +0300 Subject: [PATCH 54/55] removed `rlang::inform()` import --- r/R/arrow-package.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/R/arrow-package.R b/r/R/arrow-package.R index ea390ee1d77..509382e5dad 100644 --- a/r/R/arrow-package.R +++ b/r/R/arrow-package.R @@ -23,7 +23,7 @@ #' @importFrom rlang eval_tidy new_data_mask syms env new_environment env_bind set_names exec #' @importFrom rlang is_bare_character quo_get_expr quo_get_env quo_set_expr .data seq2 is_interactive #' @importFrom rlang expr caller_env is_character quo_name is_quosure enexpr enexprs as_quosure -#' @importFrom rlang is_list call2 is_empty as_function inform +#' @importFrom rlang is_list call2 is_empty as_function #' @importFrom tidyselect vars_pull vars_rename vars_select eval_select #' @useDynLib arrow, .registration = TRUE #' @keywords internal From fb19ce8d74c0fc6cf241cb7ffe6fe39bfec4a1ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Drago=C8=99=20Moldovan-Gr=C3=BCnfeld?= Date: Mon, 4 Apr 2022 21:25:50 +0300 Subject: [PATCH 55/55] switched back to `withr::with_envvar(c(TZ = ""))` instead of `withr::with_timezone("")` --- r/tests/testthat/test-Array.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index db2ccefa89a..2f75efb3d67 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -261,7 +261,7 @@ test_that("array supports POSIXct (ARROW-3340)", { }) test_that("array uses local timezone for POSIXct without timezone", { - withr::with_timezone("", { + withr::with_envvar(c(TZ = ""), { times <- strptime("2019-02-03 12:34:56", format = "%Y-%m-%d %H:%M:%S") + 1:10 expect_equal(attr(times, "tzone"), NULL) expect_array_roundtrip(times, timestamp("us", Sys.timezone()))