initial work setting week_start by string

[MichaelChirico]

Closes #1017

Initial work; filing here to get high-level agreement before finishing tests & all. Also wasn't having any luck fidgeting with LC_TIME to try and get non-English week names to show up so we can test that too, any ideas?

[vspinu]

Thanks for this. The abbreviated names should also be supported.

In terms of implementation maybe a better option would be to construct a long vector possible matches and then take a modulo 7 of the match. That is concatenate English names (abbr and full) with current locale names (abbr and full) and then do the match.

Regarding locale tests, we currently don't do that but would be good to have, but that would require adding extra languages to the CI. The plan is to move all locale suport directly into the package (like readr does it), but no time so far.

[MichaelChirico]

The abbreviated names should also be supported.

this is done with pmatch():

for (abbr in lubridate:::.get_locale_regs()$wday_names$abr)
  writeLines(format(round_date(ct, "week", week_start = abbr), "%a/%A"))
Sun/Sunday
Mon/Monday
Tue/Tuesday
Wed/Wednesday
Thu/Thursday
Fri/Friday
Sat/Saturday

My thinking was it's nice that it also works for less-canonical but also-accepted abbreviations:

for (abbr in c("Tues", "Thur", "Thurs"))
  writeLines(format(round_date(ct, "week", week_start = abbr), "%a/%A"))
Tue/Tuesday
Thu/Thursday
Thu/Thursday

maybe a better option would be to construct a long vector possible matches and then take a modulo 7 of the match.

Nice idea! I wonder if maybe it's too clever though. e.g. it assumes the week always has 7 days (a quick search shows that there a very small number of places not using a 7-day week... I'm not sure if they have any support in LC_TIME, however).

Happy to switch to that if you'd like & are OK losing the extended matches from pmatch().

Regarding locale tests, we currently don't do that but would be good to have, but that would require adding extra languages to the CI. The plan is to move all locale suport directly into the package (like readr does it), but no time so far.

OK, noted, I'll not bother trying to add tests for this quite yet (though please do test manually if you're using a non-English locale)

[vspinu]

Completed! Thanks for starting the work!