rok commented on a change in pull request #10507: URL: https://github.com/apache/arrow/pull/10507#discussion_r649895508
########## File path: r/R/dplyr-functions.R ########## @@ -442,3 +442,37 @@ nse_funcs$strptime <- function(x, format = "%Y-%m-%d %H:%M:%S", tz = NULL, unit Expression$create("strptime", x, options = list(format = format, unit = unit)) } + +nse_funcs$wday <- function(x, label = FALSE, abbr = TRUE, week_start = getOption("lubridate.week.start", 7)) { + if (label) { + arrow_not_supported("Label argument") + } + offset <- get_date_offset(week_start) + Expression$create("add", Expression$create("day_of_week", x), Expression$scalar(offset)) +} + +#' Get date offset +#' +#' Arrow's `day_of_week` kernel counts from 0 (Monday) to 6 (Sunday), whereas +#' `lubridate::wday` counts from 1 to 7, and allows users to specify which day +#' of the week is first (Sunday by default). This function converts the returned Review comment: We could add options to the C++ kernel to enable different behaviors there. ########## File path: r/R/dplyr-functions.R ########## @@ -442,3 +442,37 @@ nse_funcs$strptime <- function(x, format = "%Y-%m-%d %H:%M:%S", tz = NULL, unit Expression$create("strptime", x, options = list(format = format, unit = unit)) } + +nse_funcs$wday <- function(x, label = FALSE, abbr = TRUE, week_start = getOption("lubridate.week.start", 7)) { + if (label) { + arrow_not_supported("Label argument") + } + offset <- get_date_offset(week_start) + Expression$create("add", Expression$create("day_of_week", x), Expression$scalar(offset)) +} + +#' Get date offset +#' +#' Arrow's `day_of_week` kernel counts from 0 (Monday) to 6 (Sunday), whereas +#' `lubridate::wday` counts from 1 to 7, and allows users to specify which day +#' of the week is first (Sunday by default). This function converts the returned Review comment: If we go that way it would probably be best to have another Jira for "TemporalOptions". It's probably best you proceed with the workaround and we loop back to this later. ########## File path: r/R/expression.R ########## @@ -28,8 +28,17 @@ # stringr spellings of those "str_length" = "utf8_length", "str_to_lower" = "utf8_lower", - "str_to_upper" = "utf8_upper" + "str_to_upper" = "utf8_upper", # str_trim is defined in dplyr.R + "year" = "year", + "isoyear" = "iso_year", + "quarter" = "quarter", + "month" = "month", + "day" = "day", + "yday" = "day_of_year", + "isoweek" = "iso_week", + "minute" = "minute", + "second" = "second" Review comment: What about nanoseconds? ```R > second(ymd_hms("2011-06-04 12:00:01.123456789")) [1] 1.123457 ``` Arrow would probably return `1.123456789`. ########## File path: r/R/expression.R ########## @@ -28,8 +28,17 @@ # stringr spellings of those "str_length" = "utf8_length", "str_to_lower" = "utf8_lower", - "str_to_upper" = "utf8_upper" + "str_to_upper" = "utf8_upper", # str_trim is defined in dplyr.R + "year" = "year", + "isoyear" = "iso_year", + "quarter" = "quarter", + "month" = "month", + "day" = "day", + "yday" = "day_of_year", + "isoweek" = "iso_week", + "minute" = "minute", + "second" = "second" Review comment: I think so too. So probably it should be "`second = second + round(subsecond, 6)`" to match that behaviour? ########## File path: r/R/expression.R ########## @@ -28,8 +28,17 @@ # stringr spellings of those "str_length" = "utf8_length", "str_to_lower" = "utf8_lower", - "str_to_upper" = "utf8_upper" + "str_to_upper" = "utf8_upper", # str_trim is defined in dplyr.R + "year" = "year", + "isoyear" = "iso_year", + "quarter" = "quarter", + "month" = "month", + "day" = "day", + "yday" = "day_of_year", + "isoweek" = "iso_week", + "minute" = "minute", + "second" = "second" Review comment: I think so too. So probably it should be "`second = second + round(subsecond, 6)`" to match that behavior? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org