jonkeane commented on a change in pull request #12097:
URL: https://github.com/apache/arrow/pull/12097#discussion_r783470625
##########
File path: r/R/dplyr-funcs-datetime.R
##########
@@ -101,6 +101,10 @@ register_bindings_datetime <- function() {
Expression$create("day_of_week", x, options = list(count_from_zero =
FALSE, week_start = week_start))
})
+ register_binding("week", function(x) {
+ (call_binding("yday", x) - 1) %/% 7 + 1
+ })
+
Review comment:
> After talking to Rok I am confident it is better to match this PR with
"yday" then "week" as lubirdate::week is not connected with calander week at
all but is a calculation of "complete seven day periods", see here. And see
Arrow C++ week description for comparison.
I also had this concern, but indeed like Alenka says _this_ week is
literally 7-day periods from January 1. It's almost too deceptively simple
compared to ... every other week circumstance 😂 , but that's definitely what it
is! Here's the source too:
https://github.com/tidyverse/lubridate/blob/7f5323b295813812bf52ffafb1fe26ac8bec95dd/R/accessors-week.r#L27
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
