diff --git a/.github/workflows/pkgdown.yaml b/.github/workflows/pkgdown.yaml index f7bf0f93..607c71b1 100644 --- a/.github/workflows/pkgdown.yaml +++ b/.github/workflows/pkgdown.yaml @@ -16,6 +16,7 @@ name: pkgdown jobs: pkgdown: + # only build docs on the main repository and not forks if: github.repository_owner == 'cmu-delphi' runs-on: ubuntu-latest # Only restrict concurrency for non-PR jobs @@ -39,19 +40,21 @@ jobs: needs: website - name: Build site + # - target_ref gets the ref from a different variable, depending on the event + # - override allows us to set the pkgdown mode and version_label + # - mode: release is the standard build mode, devel places the site in /dev + # - version_label: 'light' and 'success' are CSS labels for Bootswatch: Cosmo + # https://bootswatch.com/cosmo/ + # - we use pkgdown:::build_github_pages to build the site because of an issue in pkgdown + # https://github.com/r-lib/pkgdown/issues/2257 run: | - override <- if (startsWith("${{ github.event_name }}", "pull_request")) { - if ("${{ github.base_ref }}" == "main") { - list(development = list(mode = "release", version_label = "light")) - } else { - list(development = list(mode = "devel", version_label = "success")) - } + target_ref <- "${{ github.event_name == 'pull_request' && github.base_ref || github.ref }}" + override <- if (target_ref == "main" || target_ref == "refs/heads/main") { + list(development = list(mode = "release", version_label = "light")) + } else if (target_ref == "dev" || target_ref == "refs/heads/dev") { + list(development = list(mode = "devel", version_label = "success")) } else { - if ("${{ github.ref_name }}" == "main") { - list(development = list(mode = "release", version_label = "light")) - } else { - list(development = list(mode = "devel", version_label = "success")) - } + stop("Unexpected target_ref: ", target_ref) } pkg <- pkgdown::as_pkgdown(".", override = override) cli::cli_rule("Cleaning files from old site...") diff --git a/DESCRIPTION b/DESCRIPTION index b478a352..451c7a15 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,13 +1,14 @@ Package: epidatr Type: Package Title: Client for Delphi's 'Epidata' API -Version: 1.1.1 +Version: 1.2.0 Authors@R: c( person("Logan", "Brooks", email = "lcbrooks@andrew.cmu.edu", role = c("aut")), person("Dmitry", "Shemetov", email = "dshemeto@andrew.cmu.edu", role = c("aut")), person("Samuel", "Gratzl", email = "sam@sgratzl.com", role = c("aut")), person("David", "Weber", email = "davidweb@andrew.cmu.edu", role = c("ctb", "cre")), + person("Nat", "DeFries", role = c("ctb")), person("Alex", "Reinhart", role = c("ctb")), person("Daniel", "McDonald", role = c("ctb")), person("Kean Ming", "Tan", role = c("ctb")), diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index cd7d75a3..792da66c 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -48,8 +48,12 @@ Please follow the guidelines in the [PR template document](.github/pull_request_ Open a release issue and then copy and follow this checklist in the issue (modified from the checklist generated by `usethis::use_release_issue(version = "1.0.2")`): -- [ ] `git pull` -- [ ] Check [current CRAN check results](https://cran.rstudio.org/web/checks/check_results_epidatr.html) + +Open a release issue and then copy and follow this checklist in the issue (modified from the checklist generated by `usethis::use_release_issue(version = "1.0.2")`): + +- [ ] `git pull` on `dev` branch. +- [ ] Make sure all changes are committed and pushed. +- [ ] Check [current CRAN check results](https://cran.rstudio.org/web/checks/check_results_epidatr.html). - [ ] `devtools::check(".", manual = TRUE, env_vars = c(NOT_CRAN = "false"))`. - Aim for 10/10, no notes. - [ ] If check works well enough, merge to main. Otherwise open a PR to fix up. @@ -61,20 +65,20 @@ Open a release issue and then copy and follow this checklist in the issue (modif - This may choke on the MIT license url, and that's ok. - [ ] `devtools::build_readme()` - [ ] `devtools::check_win_devel()` -- [ ] Check email for problems +- [ ] Have maintainer ("cre" in description) check email for problems. - [ ] `revdepcheck::revdep_check(num_workers = 4)`. - This may choke, it is very sensitive to the binary versions of packages on a given system. Either bypass or ask someone else to run it if you're concerned. - [ ] Update `cran-comments.md` -- [ ] PR with any changes +- [ ] PR with any changes (and go through the list again) into `dev` and run through the list again. Submit to CRAN: -- [ ] `devtools::submit_cran()` -- [ ] Approve email +- [ ] `devtools::submit_cran()`. +- [ ] Maintainer approves email. Wait for CRAN... -- [ ] Accepted :tada: -- [ ] `dev` -- [ ] `usethis::use_github_release(publish = FALSE)` (publish off, otherwise it won't push). -- [ ] check the release notes and publish the branch on github +- [ ] If accepted :tada:, move to next steps. If rejected, fix and resubmit. +- [ ] Open and merge a PR containing any updates made to `main` back to `dev`. +- [ ] `usethis::use_github_release(publish = FALSE)` (publish off, otherwise it won't push) will create a draft release based on the commit hash in CRAN-SUBMISSION and push a tag to the GitHub repo. +- [ ] Go to the repo, verify the release notes, and publish when ready. diff --git a/NEWS.md b/NEWS.md index 25512ee5..c8e837c3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,14 @@ +# epidatr 1.2.0 + +## Changes + +- Improve handling of the `EPIDATR_USE_CACHE` environment variable, allowing it + to be any value convertable by `as.logical()` and handle the case when it + can't be converted. +- Support more date formats in function to convert dates to epiweeks. Use `parse_api_date` since it already supports both common formats. #276 +- `EPIDATR_USE_CACHE` only supported exactly "TRUE" before. Now it supports all logical values and includes a warning when any value that can't be converted to logical is provided. #273 +- `missing` doesn't count default values as non-missing. If a user doesn't pass `geo_values` or `time_values` (both of which default to `"*"` in `pub_covidcast`), or `dates` (in `pub_covid_hosp_state_timeseries`), the missing check fails. To avoid this, just don't check missingness of those two arguments. + # epidatr 1.1.1 ## Changes @@ -5,12 +16,18 @@ ## Features ## Patches -- Fixed failure when passing `as_of` values in `Date` format to - `pub_covidcast` while caching is enabled (#259) + +- Fix failure when passing `as_of` values in `Date` format to + `pub_covidcast` while caching is enabled (#259). +- For `pub_covidcast` data source `nchs-mortality`, parse dates as `epiweek` + and expect `epiweek` inputs from user (#260). +- Fix failure in `pub_covidcast` when user doesn't pass `geo_values` or + `time_values`, even though those arguments have defaults (#268). # epidatr 1.1.0 ## Changes + - `pub_covid_hosp_state_timeseries` now supports use of the `as_of` parameter (#209). - `release_date` and `latest_update` fields are now parsed as `Date`, rather than as text. This change impacts several endpoints. @@ -18,14 +35,18 @@ - `get_api_key` no longer reads from R options and only uses environment variables (#217). - `pvt_twitter` and `pub_wiki` now use `time_type` and `time_values` args instead of mutually exclusive `dates` and `epiweeks` (#236). This matches the interface of the `pub_covidcast` endpoint. - Updated the default `timeout_seconds` to 15 minutes to allow large queries by default. + ## Features + - Function reference now displays commonly-used functions first (#205). - Support `Date` objects passed to version arguments `as_of` and `issues` in endpoints (#192, #194). - `clear_cache` now handles positional arguments just like `set_cache` (#197). - `set_api_key` now available to help persist API key environment variables (#181, #217). - All endpoints now support the use of "\*" as a wildcard to fetch all dates or epiweeks (#234). + ## Patches + - Endpoints now fail when passed misspelled arguments (#187, #201). - `pub_fluview_meta` fixed to `fetch` the response automatically. - `pub_covid_hosp_state_timeseries` now correctly parses the `issue` field, diff --git a/R/endpoints.R b/R/endpoints.R index 0146ca64..d920b77c 100644 --- a/R/endpoints.R +++ b/R/endpoints.R @@ -576,9 +576,10 @@ pub_covid_hosp_state_timeseries <- function( # Check parameters rlang::check_dots_empty() - if (missing(states) || missing(dates)) { - stop( - "`states` and `dates` are both required" + if (missing(states)) { + cli::cli_abort( + "`states` is required", + class = "epidatr__pub_covid_hosp_state_timeseries__missing_required_args" ) } @@ -960,9 +961,9 @@ pub_covidcast_meta <- function(fetch_args = fetch_args_list()) { #' ). #' @param time_type string. The temporal resolution of the data (either "day" or #' "week", depending on signal). -#' @param geo_values character. The geographies to return. "*" fetches -#' all. (See: -#' .) +#' @param geo_values character. The geographies to return. Defaults to all +#' ("*") geographies within requested geographic resolution (see: +#' .). #' @param time_values [`timeset`]. Dates to fetch. Defaults to all ("*") dates. #' @param ... not used for values, forces later arguments to bind by name #' @param as_of Date. Optionally, the as of date for the issues to fetch. If not @@ -999,17 +1000,19 @@ pub_covidcast <- function( missing(source) || missing(signals) || missing(time_type) || - missing(geo_type) || - missing(time_values) || - missing(geo_values) + missing(geo_type) ) { - stop( - "`source`, `signals`, `time_type`, `geo_type`, `time_values`, and `geo_values` are all required" + cli::cli_abort( + "`source`, `signals`, `time_type`, and `geo_type` are all required", + class = "epidatr__pub_covidcast__missing_required_args" ) } if (sum(!is.null(issues), !is.null(lag), !is.null(as_of)) > 1) { - stop("`issues`, `lag`, and `as_of` are mutually exclusive") + cli::cli_abort( + "`issues`, `lag`, and `as_of` are mutually exclusive", + class = "epidatr__pub_covidcast__too_many_issue_params" + ) } assert_character_param("data_source", source, len = 1) @@ -1025,6 +1028,13 @@ pub_covidcast <- function( as_of <- parse_timeset_input(as_of) issues <- parse_timeset_input(issues) + if (source == "nchs-mortality" && time_type != "week") { + cli::cli_abort( + "{source} data is only available at the week level", + class = "epidatr__nchs_week_only" + ) + } + create_epidata_call( "covidcast/", list( @@ -1051,8 +1061,14 @@ pub_covidcast <- function( c("day", "week") ), create_epidata_field_info("geo_value", "text"), - create_epidata_field_info("time_value", "date"), - create_epidata_field_info("issue", "date"), + create_epidata_field_info("time_value", switch(time_type, + day = "date", + week = "epiweek" + )), + create_epidata_field_info("issue", switch(time_type, + day = "date", + week = "epiweek" + )), create_epidata_field_info("lag", "int"), create_epidata_field_info("value", "float"), create_epidata_field_info("stderr", "float"), diff --git a/R/epidatr-package.R b/R/epidatr-package.R index 1b22c9e4..495368d3 100644 --- a/R/epidatr-package.R +++ b/R/epidatr-package.R @@ -8,8 +8,14 @@ "_PACKAGE" .onLoad <- function(libname, pkgname) { - cache_environ$use_cache <- Sys.getenv("EPIDATR_USE_CACHE", unset = FALSE) - cache_environ$use_cache <- (cache_environ$use_cache == "TRUE") + cache_environ$use_cache <- as.logical(Sys.getenv("EPIDATR_USE_CACHE", unset = FALSE)) + if (is.na(cache_environ$use_cache)) { + cli::cli_warn( + "Failed to read EPIDATR_USE_CACHE environment variable. + Should be a logical. Defaulting to FALSE." + ) + cache_environ$use_cache <- FALSE + } if (cache_environ$use_cache) { set_cache(startup = TRUE) } diff --git a/R/model.R b/R/model.R index 90bddd51..383c6ab7 100644 --- a/R/model.R +++ b/R/model.R @@ -239,7 +239,7 @@ parse_data_frame <- function(epidata_call, df, disable_date_parsing = FALSE) { #' @importFrom MMWRweek MMWRweek #' @keywords internal date_to_epiweek <- function(value) { - date_components <- MMWRweek::MMWRweek(as.Date(as.character(value), "%Y%m%d")) + date_components <- MMWRweek::MMWRweek(parse_api_date(value)) as.numeric(paste0( date_components$MMWRyear, # Pad with zeroes up to 2 digits (x -> 0x) diff --git a/_pkgdown.yml b/_pkgdown.yml index 98124004..5bdc2d00 100644 --- a/_pkgdown.yml +++ b/_pkgdown.yml @@ -1,5 +1,6 @@ -# Colors should stay consistent across epipredict & epidatr, using Carnegie -# Red https://www.cmu.edu/brand/brand-guidelines/visual-identity/colors.html +# Colors should stay consistent across epipredict, epiprocess, and epidatr, +# using Carnegie Red +# https://www.cmu.edu/brand/brand-guidelines/visual-identity/colors.html # This is to give a default value to the `mode` parameter in the # `pkgdown::build_site` function. This is useful when building the site locally, @@ -15,11 +16,14 @@ template: bslib: font_scale: 1.0 primary: "#C41230" + success: "#B4D43C" link-color: "#C41230" navbar: bg: primary - type: dark + type: light + +url: https://cmu-delphi.github.io/epidatr/ home: links: diff --git a/man/epidatr-package.Rd b/man/epidatr-package.Rd index 9521ea06..1809cb9a 100644 --- a/man/epidatr-package.Rd +++ b/man/epidatr-package.Rd @@ -37,6 +37,7 @@ Authors: Other contributors: \itemize{ + \item Nat DeFries [contributor] \item Alex Reinhart [contributor] \item Daniel McDonald [contributor] \item Kean Ming Tan [contributor] diff --git a/man/pub_covidcast.Rd b/man/pub_covidcast.Rd index 65fb736f..2f6a2bab 100644 --- a/man/pub_covidcast.Rd +++ b/man/pub_covidcast.Rd @@ -31,9 +31,9 @@ pub_covidcast( \item{time_type}{string. The temporal resolution of the data (either "day" or "week", depending on signal).} -\item{geo_values}{character. The geographies to return. "*" fetches -all. (See: -\url{https://cmu-delphi.github.io/delphi-epidata/api/covidcast_geography.html}.)} +\item{geo_values}{character. The geographies to return. Defaults to all +("*") geographies within requested geographic resolution (see: +\url{https://cmu-delphi.github.io/delphi-epidata/api/covidcast_geography.html}.).} \item{time_values}{\code{\link{timeset}}. Dates to fetch. Defaults to all ("*") dates.} diff --git a/tests/testthat/test-cache.R b/tests/testthat/test-cache.R index 5ac9d57e..8d75644b 100644 --- a/tests/testthat/test-cache.R +++ b/tests/testthat/test-cache.R @@ -18,7 +18,7 @@ test_set_cache <- function(cache_dir = new_temp_dir, } test_that("cache set as expected", { - test_set_cache() + expect_message(test_set_cache()) if (grepl("/", as.character(new_temp_dir))) { # this is what check produces expect_equal(cache_info()$dir, normalizePath(new_temp_dir)) @@ -38,7 +38,7 @@ test_that("cache set as expected", { # use an existing example to save, then load and compare the values test_that("cache saves & loads", { - test_set_cache() + expect_message(test_set_cache()) epidata_call <- pub_covidcast( source = "jhu-csse", signals = "confirmed_7dav_incidence_prop", @@ -128,7 +128,7 @@ test_that("check_is_cachable", { expect_false(check_is_cachable(epidata_call, fetch_args)) } } - test_set_cache() + expect_message(test_set_cache()) check_fun(expected_result = FALSE) # doesn't specify issues or as_of check_fun(as_of = "2020-01-01", expected_result = TRUE) # valid as_of check_fun(issues = "2020-01-01", expected_result = TRUE) # valid issues @@ -178,14 +178,14 @@ test_that("check_is_cachable", { # cases where the cache isn't active disable_cache() check_fun(as_of = "2020-01-01", expected_result = FALSE) - test_set_cache() + expect_message(test_set_cache()) cache_environ$epidatr_cache <- NULL check_fun(as_of = "2020-01-01", expected_result = FALSE) - test_set_cache() + expect_message(test_set_cache()) check_fun(as_of = "2020-01-01", expected_result = TRUE) }) -test_set_cache() +expect_message(test_set_cache()) cache_environ$epidatr_cache$prune() clear_cache(disable = TRUE) rm(new_temp_dir) diff --git a/tests/testthat/test-endpoints.R b/tests/testthat/test-endpoints.R index 39194535..e29acb12 100644 --- a/tests/testthat/test-endpoints.R +++ b/tests/testthat/test-endpoints.R @@ -517,3 +517,67 @@ test_that("pub_covid_hosp_state_timeseries supports versioned queries", { expect_identical(epidata_call$params$as_of, 20220101) expect_identical(epidata_call$params$lag, NULL) }) + +test_that("nchs-mortality call fails if time_type not week", { + expect_error(pub_covidcast( + source = "nchs-mortality", + signals = "signal", + time_type = "day", + geo_type = "state", + time_values = "*", + geo_values = "*" + ), class = "epidatr__nchs_week_only") +}) + +test_that("pub_covidcast catches missing args for args without defaults", { + expect_no_error(pub_covidcast( + source = "jhu-csse", + signals = "confirmed_7dav_incidence_prop", + time_type = "day", + geo_type = "state", + fetch_args = fetch_args_list(dry_run = TRUE) + )) + expect_error( + pub_covidcast( + signals = "confirmed_7dav_incidence_prop", + time_type = "day", + geo_type = "state" + ), + class = "epidatr__pub_covidcast__missing_required_args" + ) + expect_error( + pub_covidcast( + source = "jhu-csse", + time_type = "day", + geo_type = "state" + ), + class = "epidatr__pub_covidcast__missing_required_args" + ) + expect_error( + pub_covidcast( + source = "jhu-csse", + signals = "confirmed_7dav_incidence_prop", + geo_type = "state" + ), + class = "epidatr__pub_covidcast__missing_required_args" + ) + expect_error( + pub_covidcast( + source = "jhu-csse", + signals = "confirmed_7dav_incidence_prop", + time_type = "day" + ), + class = "epidatr__pub_covidcast__missing_required_args" + ) +}) + +test_that("pub_covid_hosp_state_timeseries catches missing args for args without defaults", { + expect_no_error(pub_covid_hosp_state_timeseries( + states = "fl", + fetch_args = fetch_args_list(dry_run = TRUE) + )) + expect_error( + pub_covid_hosp_state_timeseries(), + class = "epidatr__pub_covid_hosp_state_timeseries__missing_required_args" + ) +})