search

epiverse-trace / episoap

[Not published - under active development] A Store of Outbreak Analytics Pipelines Provided as Rmarkdown Report Templates
https://epiverse-trace.github.io/episoap/
Other
4 stars 2 forks source link

Address {incidence2} grouped data not working with `EpiNow2::regional_epinow()` #134

Closed jamesmbaazam closed 2 months ago

jamesmbaazam commented 4 months ago

This PR attempts to address #118 but is not for merging into main as it's a branch of Tx_pipeline_update.

In this PR, I suggest improvements to some terminology and provide a fix #118, where a grouped dataset created with {incidence2} does not work smoothly with EpiNow2::regional_epinow(). It appears in the grouped dataset, the date column is no longer valid for use with {EpiNow2}. I will investigate further why that is the case.

@CarmenTamayo could you check out this branch and run it locally to see if it fixes the issue you raised? It works for me.

CarmenTamayo commented 2 months ago

hello @jamesmbaazam, I tried the modification to add dplyr::mutate(date = as.Date(date)) and it still didn't work properly Then I realised the problem was that the column "count_variable" from incidence2 was still in the dataset, which was generating an error, I deleted the column first and then the process worked. I'm not going to merge this PR as I'm trying to solve another issue by which I'm not able to pull the changes from GitHub to my local version, but I've added the changes you suggested in this PR- I hope this is okay Thank you James!

CarmenTamayo commented 2 months ago

Additionally, I was wondering why the change in terminology, i.e., from global to aggregated transmissibility? Is there a reason behind this suggestion other than personal preference? If so I think that global might be more intuitive as aggregated transmissibility might be misinterpreted or confused with other parts of the report where aggregated vs linelist data is used

jamesmbaazam commented 2 months ago

hello @jamesmbaazam, I tried the modification to add dplyr::mutate(date = as.Date(date)) and it still didn't work properly Then I realised the problem was that the column "count_variable" from incidence2 was still in the dataset, which was generating an error, I deleted the column first and then the process worked.

That's interesting. I wonder how that was causing an error. It's good that you caught it.

I'm not going to merge this PR as I'm trying to solve another issue by which I'm not able to pull the changes from GitHub to my local version, but I've added the changes you suggested in this PR- I hope this is okay Thank you James!

Yes, this wasn't necessarily for merging as stated in the PR description. You can simply close it with a comment.

jamesmbaazam commented 2 months ago

Additionally, I was wondering why the change in terminology, i.e., from global to aggregated transmissibility? Is there a reason behind this suggestion other than personal preference? If so I think that global might be more intuitive as aggregated transmissibility might be misinterpreted or confused with other parts of the report where aggregated vs linelist data is used

That's fine. I'll leave that to you as it was only a suggestion. The term "global" could be confused with the geographic term. You could use something like "unstratified", which is clear by definition.