jorisvandenbossche commented on code in PR #12657:
URL: https://github.com/apache/arrow/pull/12657#discussion_r866056212
##########
cpp/src/arrow/compute/api_scalar.h:
##########
@@ -117,6 +118,13 @@ class ARROW_EXPORT RoundTemporalOptions : public
FunctionOptions {
CalendarUnit unit;
/// What day does the week start with (Monday=true, Sunday=false)
bool week_starts_monday;
+ /// Times exactly on unit multiple boundary will be rounded one unit
multiple up.
+ /// This applies for ceiling only.
+ bool strict_ceil;
+ /// By default origin is 1970-01-01T00:00:00. By setting this to true,
rounding origin
+ /// will be beginning of one less precise calendar unit. E.g. rounding to
hours will use
+ /// beginning of day as origin.
+ bool calendar_based_origin;
Review Comment:
By looking at the code and the inline comments, it became clear. One of the
comments is
```
// Round to a multiple of units since the last greater unit.
// For example: round to multiple of days since the beginning of the
month or
// to hours since the beginning of the day.
```
I wanted to suggest using that. Although reading the above again, that's
actually already quite similar. But the additional example might help to
clarify.
One question I have is how it is exactly defined what the "one less precise
calendar unit" / "last greater unit" is? Eg we have "week" between "day" and
"month", but we are still using "month" as the greater unit for "day"? (which
probably makes sense in practice, but we should maybe document this somewhere
more explicitly)
--
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]
