From a778d790d8404e9b2ed0a7a3e151c203aced202d Mon Sep 17 00:00:00 2001 From: Jared Knowles Date: Fri, 24 Apr 2026 17:49:08 -0400 Subject: [PATCH] fix: govid input ergonomics + clearer missing-govid message Two UX fixes surfaced by first real-user use: 1. cog_spending / cog_revenue / cog_geographic_rollup now accept either a character vector OR a data.frame with a canonical_govid column (e.g. output of cog_gov_search() or cog_find_peers()). Shared .coerce_govid_input() helper in session.R. This lets the natural pipe work: cog_gov_search('MIAMI', state='FL', type='city') |> cog_spending(years=2022, category='Police') cog_peer_compare already accepted a data.frame for the peer arg; behavior there is unchanged. 2. .check_govids_in_scope() message reworded. The old text led with 'v0.1 covers gov_types 0-3' which falsely implied the missing govids were scope-excluded types when the more common real cause is a typo or a guessed value. New message leads with typo + pre-2017 PID, mentions scope exclusion as one possibility, and points at cog_gov_search() as the recovery path. Tests: 165 pass / 0 fail. check 0E/0W/0N. --- R/rollup.R | 13 ++++++++----- R/session.R | 28 +++++++++++++++++++++++++++- R/spending.R | 2 +- tests/testthat/test-revenue.R | 2 +- tests/testthat/test-rollup.R | 12 ++++++++++++ tests/testthat/test-spending.R | 23 ++++++++++++++++++++++- 6 files changed, 71 insertions(+), 9 deletions(-) diff --git a/R/rollup.R b/R/rollup.R index d5b58ab..7f8e495 100644 --- a/R/rollup.R +++ b/R/rollup.R @@ -26,8 +26,14 @@ cog_geographic_rollup <- function(govids, category, years, per_capita = FALSE, adjust_to_year = NULL) { call <- match.call() - .validate_rollup_govids(govids) + .validate_rollup_layers(govids) + # Accept character vector OR a data.frame with canonical_govid per layer, + # so cog_gov_search() output can be piped into one of the layer slots. + govids <- lapply(govids, .coerce_govid_input, arg = "govids[[layer]]") + if (any(lengths(govids) == 0L)) { + cli::cli_abort("Each layer in `govids` must be non-empty after coercion.") + } layer_names <- names(govids) all_govids <- unlist(govids, use.names = FALSE) layer_map <- tibble::tibble( @@ -51,7 +57,7 @@ cog_geographic_rollup <- function(govids, category, years, } #' @noRd -.validate_rollup_govids <- function(govids) { +.validate_rollup_layers <- function(govids) { if (!is.list(govids) || is.data.frame(govids)) { cli::cli_abort("`govids` must be a named list.") } @@ -68,9 +74,6 @@ cog_geographic_rollup <- function(govids, category, years, "`govids` names must be one of 'state', 'county', 'city'. Got: {bad}." ) } - if (any(lengths(govids) == 0L)) { - cli::cli_abort("Each layer in `govids` must be non-empty.") - } invisible(TRUE) } diff --git a/R/session.R b/R/session.R index a85365b..b385097 100644 --- a/R/session.R +++ b/R/session.R @@ -33,6 +33,31 @@ cog_open <- function(url = .resolve_url(), .uscogdata_env$con } +#' @noRd +#' Coerce an input to a character vector of canonical_govid values. +#' Accepts either a character vector (returned as-is after `as.character`) +#' or a data.frame / tibble with a `canonical_govid` column (such as the +#' output of [cog_gov_search()] or [cog_find_peers()]) — in that case the +#' column is extracted so results from discovery verbs can pipe directly +#' into the query verbs. +.coerce_govid_input <- function(x, arg = "govid") { + if (is.data.frame(x)) { + if (!"canonical_govid" %in% names(x)) { + cli::cli_abort(c( + "`{arg}` data frame must have a `canonical_govid` column.", + i = "Use the result of cog_gov_search() or cog_find_peers() directly, or pass a character vector of canonical_govids." + )) + } + return(as.character(x$canonical_govid)) + } + if (!is.character(x) && !is.numeric(x)) { + cli::cli_abort( + "`{arg}` must be a character vector or a data frame with a `canonical_govid` column." + ) + } + as.character(x) +} + #' @noRd #' Check which of the supplied govids exist in canonical_fips_xwalk. #' Emits a cli message listing any missing ones alongside a pointer to the @@ -55,7 +80,8 @@ cog_open <- function(url = .resolve_url(), cli::cli_inform(c( i = sprintf("%d govid%s not found in v0.1 corpus: %s%s", n, if (n == 1L) "" else "s", shown, more), - i = "v0.1 covers gov_types 0-3 (state/county/city/township). Types 4/5 excluded; see vignette('coverage-scope')." + i = "Common causes: typo, pre-2017 PID that isn't bridged, or a scope-excluded type (4=special district, 5=school district).", + i = "Resolve canonical names with cog_gov_search() first." )) } list(found = found, missing = missing) diff --git a/R/spending.R b/R/spending.R index 680c6f6..0c452ca 100644 --- a/R/spending.R +++ b/R/spending.R @@ -42,9 +42,9 @@ cog_spending <- function(govid, years, category = NULL, .verb_spendrev <- function(verb, view, subtype_col, call, govid, years, category, per_capita, adjust_to_year) { + govid <- .coerce_govid_input(govid, arg = "govid") .validate_verb_inputs(govid, years, category, per_capita, adjust_to_year) - govid <- as.character(govid) years <- as.integer(years) if (!is.null(adjust_to_year)) adjust_to_year <- as.integer(adjust_to_year) diff --git a/tests/testthat/test-revenue.R b/tests/testthat/test-revenue.R index 7c0cdaf..7c28290 100644 --- a/tests/testthat/test-revenue.R +++ b/tests/testthat/test-revenue.R @@ -34,5 +34,5 @@ test_that("cog_revenue result has provenance attribute", { }) test_that("cog_revenue rejects invalid inputs", { - expect_error(cog_revenue(123, 2020L), "character") + expect_error(cog_revenue(list(), 2020L), "character|data frame") }) diff --git a/tests/testthat/test-rollup.R b/tests/testthat/test-rollup.R index 06dbd55..b7e4b22 100644 --- a/tests/testthat/test-rollup.R +++ b/tests/testthat/test-rollup.R @@ -78,6 +78,18 @@ test_that("cog_geographic_rollup provenance reports the outer verb", { expect_true(grepl("cog_geographic_rollup", prov$call)) }) +test_that("cog_geographic_rollup accepts data.frames per layer", { + skip_if_no_corpus() + fl_state <- cog_gov_search("^FLORIDA STATE GOVT$", type = "state") + broward <- cog_gov_search("^BROWARD COUNTY$", state = "FL", type = "county") + r <- cog_geographic_rollup( + govids = list(state = fl_state, county = broward), + category = "Police", years = 2020L + ) + expect_setequal(unique(r$layer), c("state", "county")) + expect_gt(nrow(r), 0L) +}) + test_that("cog_geographic_rollup rejects invalid inputs", { expect_error(cog_geographic_rollup(list(), "Police", 2020L), "length") expect_error(cog_geographic_rollup(c("101006006"), "Police", 2020L), "list") diff --git a/tests/testthat/test-spending.R b/tests/testthat/test-spending.R index a83b182..cf5b3f7 100644 --- a/tests/testthat/test-spending.R +++ b/tests/testthat/test-spending.R @@ -92,6 +92,27 @@ test_that("cog_spending result has provenance attribute matching schema", { }) test_that("cog_spending rejects invalid inputs", { - expect_error(cog_spending(123, 2020L), "character") + expect_error(cog_spending(list(), 2020L), "character|data frame") expect_error(cog_spending("101006006", "2020"), "years") }) + +test_that("cog_spending accepts a cog_gov_search result directly", { + skip_if_no_corpus() + picks <- cog_gov_search("^BROWARD COUNTY$", state = "FL", type = "county") + expect_gt(nrow(picks), 0L) + r <- cog_spending(picks, 2020L, "Corrections") + expect_equal(unique(r$canonical_govid), "101006006") +}) + +test_that("cog_spending accepts a cog_find_peers result directly", { + skip_if_no_corpus() + peers <- cog_find_peers("101006006", max_peers = 3L) + r <- cog_spending(peers, 2020L, "Police") + expect_setequal(unique(r$canonical_govid), + sort(peers$canonical_govid)) +}) + +test_that("cog_spending rejects data.frame without canonical_govid column", { + bad <- tibble::tibble(foo = "bar") + expect_error(cog_spending(bad, 2020L), "canonical_govid") +})