tidyverse / lubridate

Make working with dates in R just that little bit easier
https://lubridate.tidyverse.org
GNU General Public License v3.0
724 stars 207 forks source link

`week_start` parameter documentation #1144

Open asadow opened 10 months ago

asadow commented 10 months ago

I'd like to re-open a discussion surrounding the parameter week_start's description. Currently it is,

day on which week starts following ISO conventions: 1 means Monday and 7 means Sunday (default). When label = FALSE and week_start = 7, the number returned for Sunday is 1, for Monday is 2, etc. When label = TRUE, the returned value is a factor with the first level being the week start (e.g. Sunday if week_start = 7). You can set lubridate.week.start option to control this parameter globally.

Perhaps going straight into an example and removing unnecessary detail is the simplest way to explain this parameter. Something like,

Day on which week starts. For example: if week_start is set to 2 or "Tuesday" and parameter label is FALSE, then wday() of a Monday's date will return 7. If week_start is set to 2 or "Tuesday" and parameter label is TRUE, then wday() of a Monday's date will return Mon, an ordered factor value where the first level is Tue.

Three other suggestions:

  1. I capitalized Day above. We can capitalize all first letters of parameter descriptions (e.g. see ?str_detect): Day is then less likely to be confused with the function day. This capitalization style is from the tidyverse style guide on the subject.
  2. Functions like wday are not referred to as wday() but as wday (e.g. in the description for parameter label). May we add parentheses after every function mentioned in the descriptions? This suggestion is also from the style guide. Likewise with backticking functions and values.
  3. In the current description (first above), the reader has to juggle two sets of meaning for the values 1-7, and two sets of return values depending on parameter label. The suggested description (second above) is more clear, but still suffers from the two sets of return values. From #1100 and #866 I gather there are historical reasons. Which leads me to the question: would an additional function like weekday() be appropriate to help circumvent historical reasons for this wday()'s confusion?
MichaelChirico commented 4 months ago

Adding to this issue since I found round_date() documentation a bit lacking today.

I was trying to ascertain what units get affected by the week_start parameter, with the suspicion that it's only unit="week", but was unable to confirm this until I looked at the source code, which pointed me to timechange::time_round() which documentation does confirm this:

https://github.com/vspinu/timechange/blob/77384ae0fab1ac67c7459bb790735ee01e639d6b/man/time_round.Rd#L54-L55

Maybe the documentation should just point directly to [timechange::time_round()] instead?