epiforecasts / covidregionaldata

An interface to subnational and national level COVID-19 data. For all countries supported, this includes a daily time-series of cases. Wherever available we also provide data on deaths, hospitalisations, and tests. National level data is also supported using a range of data sources as well as linelist data and links to intervention data sets.
https://epiforecasts.io/covidregionaldata/
Other
37 stars 18 forks source link

We've also made a fix to `complete()` to ensure that it always works as expected with grouped data frames. One of the results of this fix is that you can no longer supply group variables to `complete()` (if you have a grouped data frame, `complete()` will work "within" each group so you shouldn't have access to them). See https://github.com/tidyverse/tidyr/pull/1300 for more details. #449

Closed seabbs closed 2 years ago

seabbs commented 2 years ago

We've also made a fix to complete() to ensure that it always works as expected with grouped data frames. One of the results of this fix is that you can no longer supply group variables to complete() (if you have a grouped data frame, complete() will work "within" each group so you shouldn't have access to them). See https://github.com/tidyverse/tidyr/pull/1300 for more details.

This seems to have also broken a few tests. In particular it seems related to https://github.com/epiforecasts/covidregionaldata/blob/b93e19dcb1e39b67828c3e4b6038cbf2b158ef1d/R/processing.R#L60 At some point in your tests you pass this a grouped data frame with level_1_region as a group column, and then you try and complete on that column too, which is not allowed / well-defined. It is possible that all you really wanted was to group by the level_ columns and then call complete(date = full_seq(data$date, period = 1)) (i.e. within each level group, complete the full range of dates from the original data set), but I am not entirely sure.

If you could take a look at that one too, I'd appreciate it. I spent a bit of time trying to figure out the best place to ungroup, but it was a little tough for me to know where the best place would be.

Originally posted by @DavisVaughan in https://github.com/epiforecasts/covidregionaldata/issues/445#issuecomment-1010101937

RichardMN commented 2 years ago

I recommend we prioritize moving the fix for #447 into main ASAP, or just folding it into the various PRs. It breaks the R-CMD-Check so badly that these other PRs will fail even though the fixes themselves are ok.

seabbs commented 2 years ago

Sounds like a good idea. I have merged the fix for this issue and made the default to not test region-level data (i.e the default is to just test within package functionality).