Closed scottyaz closed 5 years ago
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:
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).
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")
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).
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.
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")