no warning (or error) with impossible dates

[scottyaz]

When passing a character as a date to date2week it seems like only the last two numbers (understandably) are read in the presumed day field. Seems like at a minimum a warning should be thrown to indicate that this conversion is assuming that the characters are in Y M D format. If more than two digits are picked up in the month or day position, perhaps an error or warning should also be thrown?

`
R> date2week("1910/12/11",week_start="saturday")

[1] "1910-W50-2"

R> date2week("1910/12/111",week_start="saturday")

[1] "1910-W50-2"

R> date2week("1910/12/1111",week_start="saturday")

[1] "1910-W50-2"
`

[zkamvar]

Hi @scottyaz,

Thank you for the issue. You've got a good point! This behavior is due to the fact that the first thing date2week() attempts to do is convert the input to a POSIXlt object:

aweek/R/date2week.R

Lines 97 to 100 in 51b36c9

	date2week <- function(x, week_start = 1, floor_day = FALSE, numeric = FALSE, factor = FALSE, ...) {
	x <- tryCatch(as.POSIXlt(x, ...), error = function(e) e)

I think it makes sense to throw an error here and assume that users will have already converted their dates to yyyy-mm-dd format (if a character).

[zkamvar]

Hi @scottyaz,

I've fixed this in #4, would you mind checking to see if it works for you?

You can install the proposed change with:

remotes::install_github("reconhub/aweek#4")

[scottyaz]

Works fine though I guess in someways it is a shame to not be borrowing from as.POSIXct()'s attempt to be smart (e.g., its nice that it works with slashes, with mm of only a single number etc).

[zkamvar]

Works fine though I guess in someways it is a shame to not be borrowing from as.POSIXct()'s attempt to be smart (e.g., its nice that it works with slashes, with mm of only a single number etc).

True, but as.POSIXlt() gives you those same impossible dates that caused the issue in the first place:

as.POSIXlt("1910/12/1111")
#> [1] "1910-12-11 GMT"

^{Created on 2019-03-07 by the reprex package (v0.2.1)}

The way I see it, parsing non-standard dates is a bit out of the scope for aweek since there are packages like lubridate and linelist that deal with this very issue.