Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

week_start parameter documentation #1144

Open
asadow opened this issue Oct 16, 2023 · 1 comment
Open

week_start parameter documentation #1144

asadow opened this issue Oct 16, 2023 · 1 comment

Comments

@asadow
Copy link

asadow commented Oct 16, 2023

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 Confusing documentation vs default value for week_start in wday() #1100 and wday week_start documentation is incorrect or unituitive #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
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants