pitrou commented on a change in pull request #10598:
URL: https://github.com/apache/arrow/pull/10598#discussion_r662207127
##########
File path: cpp/src/arrow/compute/kernels/scalar_temporal.cc
##########
@@ -136,19 +136,25 @@ struct Day {
// ----------------------------------------------------------------------
// Extract day of week from timestamp
+//
+// By default week starts on Monday represented by 0 and ends on Sunday
represented
+// by 6. Start day of the week (Monday=1, Sunday=7) and numbering start (0 or
1) can be
+// set using DayOfWeekOptions
template <typename Duration>
struct DayOfWeek {
- explicit DayOfWeek(TemporalComponentExtractionOptions options) :
options(options) {}
+ explicit DayOfWeek(DayOfWeekOptions options) : options(options) {}
template <typename T, typename Arg0>
T Call(KernelContext*, Arg0 arg, Status*) const {
- return static_cast<T>(
- weekday(year_month_day(floor<days>(sys_time<Duration>(Duration{arg}))))
- .iso_encoding() -
- 1 + options.week_start);
+ auto wd = arrow_vendored::date::year_month_weekday(
+ floor<days>(sys_time<Duration>(Duration{arg})))
+ .weekday()
+ .iso_encoding();
+ return (wd + 7 - options.week_start) % 7 + options.one_based_numbering;
Review comment:
If we are worried about the modulo cost, we could instead use a simple
condition:
```c++
const auto shifted_wd = wd + 7 - options.week_start;
return options.one_based_numbering + ((shifted_wd > 7) ? shifted_wd - 7 :
shifted_wd);
```
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
