AlenkaF commented on a change in pull request #12097:
URL: https://github.com/apache/arrow/pull/12097#discussion_r781103385
##########
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](https://lubridate.tidyverse.org/reference/week.html). And[ see Arrow C++
week
description](https://arrow.apache.org/docs/cpp/compute.html#temporal-component-extraction)
for comparison.
As for the PyArrow side, the logic behind
[`pc.week()`](https://arrow.apache.org/docs/python/generated/pyarrow.compute.week.html#pyarrow.compute.week)
is in line with the week compute function and Pandas
[`dt.week`](https://pandas.pydata.org/docs/reference/api/pandas.Series.dt.week.html).
What I will try to do is to use a divide C++ function instead of %%.
--
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]
