Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set-up temperature-R0 code and vignette #3

Merged
merged 12 commits into from
Aug 28, 2024
Merged

Set-up temperature-R0 code and vignette #3

merged 12 commits into from
Aug 28, 2024

Conversation

EmilieFinch
Copy link
Collaborator

This PR adds preliminary package functionality including:

  • Adding posterior estimates of relative R0 from Mordecai et al 2019 as package data
  • Initial code for atemperature_r0 function which takes a time-series of temperature data and a specified vector-pathogen combination as inputs and returns median posterior estimates of relative R0
  • A vignette introducing this using data from the 2013/14 DENV3 outbreak in Fiji as a case study

Copy link
Member

@joshwlambert joshwlambert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @EmilieFinch! I've left a few inline comments in the files. Overall, it's looking good and is a solid foundation to develop from.

Let me know if you're unable to fix any of the failing checks and I'm happy to help out.

#' CxpiWNVxl; CxtaWEEV; CxtaWNVx; CxunWNVx
#'@return numeric vector of relative R0 values corresponding to input temperature values
#'@export
temperature_r0 <- function(data, vector_pathogen){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will need an example (using @examples) at some point as it's an exported function, but this can be added later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fab, have added example here.


#' @description This function takes an input series of mean temperature values in °C
#' and returns estimated relative R0 values according to temperature-R0 curves in
#' from Mordecai et al 2019 https://doi.org/10.1111/ele.13335
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also the @references tag from Roxygen to add full references to function documentation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have moved reference to the @references tag, thanks!

Comment on lines 16 to 21
if(!vector_pathogen %in% vector_reference$vector_pathogen){
message("Vector-pathogen code is not recognised. This should be one of:
AeaeDENV; AeaeZIKV; AealDENV; AetaRVFV;
AetaSINV; AetrEEEV; AngaPfal; CxanMVEx;
CxanRRVx; CxpiSINV; CxpiWNVxl; CxtaWEEV;
CxtaWNVx; CxunWNVx.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably be replaced by match.arg() (which is a function from base R).

It comes with a couple of features that you may not want or differ from the current code.

  1. It defaults to the first element in the options vector.
  2. It errors if the user gives an string not in the options vector, whereas currently it's a message.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've switched this to match.arg() but added a nicer error message with a tryCatch()

Comment on lines 29 to 30
r0_posteriors <- eval(parse(text = paste0(vector_pathogen, "_R0")))
constant_temps <- eval(parse(text = paste0(vector_pathogen, "_T")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to rewrite this without the use of eval(parse()) as this can be a code vulnerability, especially if what is being evaluated is a user-defined input. We can discuss what these lines do and what the best alternative is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes makes sense - I've moved the data from data to inst/extdata which has solved this issue as it's now called with system.file()


# Wrangle posteriors

tr0 <- r0_posteriors |>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be better to rewrite this without the pipe (|>) as chained operations with the pipe can make functions harder to debug and by using |> you impose a minimum version of R on the package users of 4.1.0. As you're using {dplyr} then you can switch to %>% without taking on any other dependencies.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have amended, thanks!

@@ -0,0 +1,14 @@
#' Fiji 2014.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other data sets will also need to be documented to remove the warnings from R CMD check.

@@ -7,6 +7,3 @@
# * https://testthat.r-lib.org/articles/special-files.html

library(testthat)
library(packagetemplate)

test_check("packagetemplate", stop_on_warning = FALSE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you removed this to stop the tests failing? I think it would be better to not remove it and replace packagetemplate with climateR0 so that any unit tests you write in the future will be automatically run.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes makes sense, have updated accordingly.

)
```

Temperature is an important driver of vector-borne disease transmission, affecting vector reproduction, development, and survival, as well as the probability of pathogen transmission. Previous work by [Mordecai and colleagues](https://onlinelibrary.wiley.com/doi/10.1111/ele.13335) empirically estimated the effect of temperature on different vector traits, and used these to develop models of temperature dependent R~0~.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reference can be added using a bibliography file and automatically render the reference list (similar to LaTeX). You don't need to do this in this PR, but could be a nice enhancement in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave this for now but would be nice to add in the future.

@EmilieFinch EmilieFinch merged commit 34a65ef into main Aug 28, 2024
7 of 9 checks passed
@EmilieFinch EmilieFinch deleted the temperature-r0 branch August 28, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants