jorisvandenbossche commented on pull request #10598:
URL: https://github.com/apache/arrow/pull/10598#issuecomment-870455780


   @rok it's still unclear to me what the `week_start` option in the PR now 
means / does (eg how should the "Index of the first day" be expressed. A number 
relative to what?). Please try to make the docstring more explicit.
   
   There are two aspects that could be controlled:
   - On which day do we start counting (eg typically Sunday vs Monday) -> 
"start day"
   - At which number do we start counting (eg typically 0 vs 1) -> "start index"
   
   ISO starts counting at 1 on Monday, C++ starts counting at 0 on Sunday (but 
`date.h` has a `c_encoding()` and `iso_encoding()` to get the other), lubridate 
starts counting at 1 on Sunday, Python starts counting at 0 on Monday,  (to 
cover all the variations .. ;-))
   
   lubridate's `week_start` actually covers the start day (first point, on 
which day do we start counting, for the rest it always uses numbers 1-7)). 
While I *think* what you implemented here covers the start index (the second 
point, at which number to start counting)? 
   
   Taking a step back: for the "start index", would users ever want something 
else as 0 or 1 ? (I don't think that counting from 2 to 8 would ever make 
sense). In addition, going from 0-indexed to 1-indexed is a simple operation 
(addition +1). On the other hand, the "start day" is more difficult to change 
afterwards (since you need to wrap around, not a simple +1,  but eg 7 needs to 
become 0). So if we want to add some option, "start day" seems the more useful 
one to add?


-- 
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]


Reply via email to