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

Dev #45

Closed
wants to merge 2,368 commits into from
Closed

Dev #45

wants to merge 2,368 commits into from

Conversation

jkamins7
Copy link
Contributor

No description provided.

Copy link
Contributor Author

@jkamins7 jkamins7 left a comment

Choose a reason for hiding this comment

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

General things to do :

  • make sure formatR is run on each file
  • go through and remove functions which are unused
  • document config options
  • write some global documentation on how to use the pipeline with docker
    I think if we do these, plus address the individual points, then we should be ok to master

@@ -119,17 +119,7 @@ make_initial_values_filename <- function(cholera_directory,
covariate_name_part,
config,
config_dict) {

if ("file_names" %in% names(config)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why did this get removed? was it broken, or did we have a merge issue?

@@ -139,18 +129,19 @@ make_initial_values_filename <- function(cholera_directory,
return(paste(filename, config[["file_names"]][["stan_output_filename"]], sep = "/"))
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be fixed by formatr, so seems like we need someone to install

@@ -0,0 +1,92 @@
testthat::skip_on_ci()
source("~/cholera-mapping-pipeline/packages/taxdat/R/database_api_key.R")
testthat::skip_on_ci()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You should only skip on ci once

time_overlap <- function(tl, tr, res_time, tl_vec, tr_vec){
# TODO
warning("This function is not written")
time_overlap <- function(tl, tr, res_time, tl_vec, tr_vec) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we delete this?

@@ -1,64 +1,201 @@
#' @export
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should make sure that all of these functions are still used somewhere before merging.

@jkamins7 jkamins7 requested review from eclee25 and javierps and removed request for eclee25 October 26, 2021 12:33
QLLZ and others added 26 commits October 16, 2023 11:30
Revert setting Mainland and ZNZ as ADM0 level
QLLZ and others added 28 commits June 21, 2024 12:10
Updated natural scale for manuscript figure 5c
…lera-mapping-pipeline into dev_update_postprocessing
Added stats for targeting strategy
…lera-mapping-pipeline into dev_update_postprocessing
new updates about manuscript figures and functions
…lera-mapping-pipeline into dev_update_postprocessing
removed some unnecessary code
…lera-mapping-pipeline into dev_update_postprocessing
@eclee25
Copy link
Collaborator

eclee25 commented Sep 4, 2024

master is no longer the default branch on this repo

@eclee25 eclee25 closed this Sep 4, 2024
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.

5 participants