search

Public-Health-Scotland / phsmethods

An R package to standardise methods used in Public Health Scotland (https://public-health-scotland.github.io/phsmethods/)
https://public-health-scotland.github.io/phsmethods/
54 stars 13 forks source link

qtr only accepts lubridate date types #47

Closed Moohan closed 3 years ago

Moohan commented 3 years ago

Currently, the qtr_ functions will only accept a date variable if it is specifically the lubridate Date class this is more restrictive than the fin_year function, which also accepts dates of POSIXct class. This is a bit confusing when you can possibly use one function on a date column but not another function.

I don't see any reason why qtr_ can't take a POSIXct class date column so I suggest making that change, might be nice to create a util function is_date which can be used by all other functions.

Moohan commented 3 years ago

@Tina815 and @jackhannah95 wrote the original function(s).

Moohan commented 3 years ago

@davidc92 Just continuing the chat from #48 here as if I make the change you suggested last I'll start a new PR.

Yes, the issue is just that the error checking in the qtr_ functions is too strict, but I thought a new function was the best option as without it the check code is repeated quite a lot. The other advantage of a separate function is that it can be tested independently.