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

Revised Colombia to change data source - partial fix for #430 #432

Closed RichardMN closed 2 years ago

RichardMN commented 2 years ago

This switch to using Socrata api to download case list from Colombian open data source.

This will provide level 2 data in addition to level 1. Aggregation to level 1 not yet working; it is currently a quick copy-paste from Lithuania code.

This makes the data download a lot slower. There is a possibility in the Socrata API to request aggregation of the data before download but I have not yet got this to work. Switching to using Socrata means a new dependency on RSocrata but this appears a light load. (RSocrata is in CRAN, doesn't have any other additional dependencies, and seems regularly-maintained.)

RichardMN commented 2 years ago

With these changes I think it’s running correctly.

One Note appearing in the cmd check is that we are now importing 20+ packages, and that some of these should be suggested.

I think that this could be possible for RSocrata and possibly some other packages which are only used for one or two country classes. If you don’t need/want to download the regional data for Colombia, you should be able to download and build and operate the package without this package.

I will try to disentangle this import and may see if there are others which can be moved to suggests.

kathsherratt commented 2 years ago

Hi @RichardMN - thank you so much for this and sorry it's been a while to get back to you. I've checked locally and this looks great to me. I'll approve the PR for now, and tagging @seabbs too in case he has anything to add.

I agree moving any country-specific packages to suggests makes a lot of sense - please could you open a separate issue for this good idea? :)

RichardMN commented 2 years ago

There's a new PR on #433 which is essentially this PR but with the require / suggested approach implemented. I had put it onto this branch but it broke things so I took it elsewhere to get it running.

If we like #433 and that approach then we could merge that in and close this ... but there's something odd with the documentation linking to .data right now so it'll need to be looked at.

I will make a separate issue to propose pruning our dependency tree.

seabbs commented 2 years ago

Thanks for this Richard and sorry it took so long for me to get on it. I think #433 is the way to go for this so shall we close this out and focus on merging that (its very close!)