feat(coverage): n_units_collected separates sampling from real zeros (#36)
provenance$coverage's n_units_reporting is category-conditional: it counts governments with rows for the SPECIFIC requested category, which conflates two different things -- a government never collected that year (sampling), and one collected but genuinely spending nothing in that category (a real zero). FY2012 Georgia Police is the motivating case from the issue: a complete census year reads as a 69% "response rate" because most of the gap is cities that contract policing to the county sheriff, not non-response. Adds a second counter, n_units_collected: how many of the caller's expected cohort appear in the corpus that year for ANY category. n_units_collected / n_units_expected is the true collection rate; n_units_reporting / n_units_collected is category participation among collected units. cog_geographic_rollup() and cog_peer_compare() both carry it; cog_explain() prints it alongside n_units_reporting. Two real bugs caught and fixed while finishing this (both against the already-written, previously-uncommitted draft): - .coverage_table()'s candidate list for the collection query was derived from the category-filtered result rows, not the caller's full expected cohort. A government with zero rows in the requested category across every requested year never appears in that result, so it was silently excluded from n_units_collected too -- collapsing the new counter back to the old, broken one for exactly the governments it exists to count. Fixed by threading an explicit `expected_ids` (all_govids / peer_govids) through instead. - The collection query hardcoded long_view = "spending_long_harmonized", which does not exist on a corpus with schema_version < 5 (R/basis.R resolves basis = "raw" there; R/views.R only registers the harmonized views on v5+). cog_geographic_rollup()/cog_peer_compare() would hard-error on a corpus vintage the package otherwise explicitly supports. Fixed by deriving long_view from the basis cog_spending() actually resolved (prov$basis) via the existing .select_long_view() helper, matching how every other basis-aware query in the package already does this. Also: cog_explain()'s general "complete census only in years ending in 2 or 7" footnote was gated on the OLD counter's absence, making it permanently unreachable now that both callers always supply the new one -- ungated it, since the explanation is orthogonal to which counter set is present. Dropped a dead conditional branch, fixed two stale roxygen blocks in R/peers.R/R/rollup.R still describing the old two-counter model, fixed the same staleness in README.md, and switched two `uscogdata:::` self-references to the package's own convention of calling internal helpers unqualified. 1101 tests pass (2 skipped live-corpus), including new direct regression tests for both bugs above (one exercising a government collected-but-absent from a category result, one running the full rollup/peer-compare path against a doctored schema_version 4 corpus). Reviewed by an independent code-reviewer pass (1 HIGH, 1 MEDIUM, 3 LOW -- all addressed above). Closes #36. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -48,6 +48,13 @@ test_that("multi-government aggregates disclose reporting coverage on every resu
|
||||
expect_equal(cov$n_units_reporting, c(152L, 597L, 112L, 114L))
|
||||
expect_equal(cov$is_census_year, c(FALSE, TRUE, FALSE, FALSE))
|
||||
|
||||
# uscogdata#36: with category = NULL (no category scope), "reported at
|
||||
# all" and "collected" are the same question, so n_units_collected must
|
||||
# equal n_units_reporting exactly here. This case alone cannot catch a
|
||||
# regression in HOW n_units_collected is computed, though: see the
|
||||
# category-scoped test below for that.
|
||||
expect_equal(cov$n_units_collected, cov$n_units_reporting)
|
||||
|
||||
# Cross-check against the raw partitions, scoped to the SAME universe the
|
||||
# rollup was given -- the 608 govids above. Scoping instead on the long
|
||||
# table's own `type`/`fips_state` asks a different question and answers 595:
|
||||
@@ -80,6 +87,8 @@ test_that("multi-government aggregates disclose reporting coverage on every resu
|
||||
expect_equal(cov_peers$n_units_expected, rep(15L, 3L))
|
||||
expect_equal(cov_peers$n_units_reporting, c(15L, 3L, 3L))
|
||||
expect_equal(cov_peers$is_census_year, c(TRUE, FALSE, FALSE))
|
||||
# uscogdata#36: same identity as the rollup case above, category = NULL.
|
||||
expect_equal(cov_peers$n_units_collected, cov_peers$n_units_reporting)
|
||||
|
||||
# -- the three coverage modes --------------------------------------------
|
||||
expect_equal(attr(cog_peer_compare(target_govid = chilton, peers = peers,
|
||||
@@ -101,3 +110,102 @@ test_that("multi-government aggregates disclose reporting coverage on every resu
|
||||
coverage = "census")
|
||||
expect_equal(sort(unique(census_only$year)), 2012)
|
||||
})
|
||||
|
||||
test_that("n_units_collected separates sampling from real zeros, category-scoped (uscogdata#36)", {
|
||||
# The motivating case from the issue: Wisconsin cities, category = "Police".
|
||||
# FY2012 is a complete census year -- collection is not partial -- yet a
|
||||
# category-conditional n_units_reporting alone reads like a sampling gap.
|
||||
# n_units_collected must diverge from n_units_reporting here, unlike the
|
||||
# category = NULL cases above, because most of the FY2012 gap is cities
|
||||
# that contract policing to the county sheriff (collected, real zero), not
|
||||
# cities Census never surveyed.
|
||||
wi <- cog_gov_search(name = NULL, state = "WI", type = "city")
|
||||
roll <- suppressMessages(cog_geographic_rollup(
|
||||
govids = list(city = wi$canonical_govid), category = "Police",
|
||||
years = c(2011L, 2012L, 2019L, 2020L)))
|
||||
cov <- wt_coverage(roll)
|
||||
|
||||
expect_equal(cov$n_units_expected, rep(608L, 4L))
|
||||
expect_equal(cov$n_units_collected, c(152L, 597L, 112L, 114L))
|
||||
expect_equal(cov$n_units_reporting, c(152L, 485L, 109L, 111L))
|
||||
|
||||
# The pair the issue actually wants: collected/expected is the true
|
||||
# collection rate (98% in the FY2012 census year, matching the raw
|
||||
# cross-check above); reporting/collected is category participation among
|
||||
# collected units (81% -- most of the gap is real, not sampling).
|
||||
expect_equal(round(cov$n_units_collected[cov$year == 2012] /
|
||||
cov$n_units_expected[cov$year == 2012], 2), 0.98)
|
||||
expect_equal(round(cov$n_units_reporting[cov$year == 2012] /
|
||||
cov$n_units_collected[cov$year == 2012], 2), 0.81)
|
||||
|
||||
# Every year: collected is bounded between reporting and expected.
|
||||
expect_true(all(cov$n_units_collected >= cov$n_units_reporting))
|
||||
expect_true(all(cov$n_units_collected <= cov$n_units_expected))
|
||||
})
|
||||
|
||||
test_that(".coverage_table() candidates a government collected-but-absent from the category result (uscogdata#36)", {
|
||||
# Direct regression test for the mechanism itself: n_units_collected's
|
||||
# candidate list must be the caller's full expected cohort (expected_ids),
|
||||
# never derived from `result`/`rows`. A government with zero rows in the
|
||||
# requested category across every requested year never appears in
|
||||
# `result` at all, so deriving candidates from `result` would silently
|
||||
# drop exactly the "collected but real zero" governments this counter
|
||||
# exists to count -- collapsing it back to n_units_reporting.
|
||||
con <- uscogdata:::.ensure_session()
|
||||
|
||||
# A real fixture govid, present in spending_long_harmonized for 2019 (in
|
||||
# SOME category), but absent from this fake category-specific `result`.
|
||||
govid <- "011021100004"
|
||||
fake_result <- data.frame(canonical_govid = character(0), year = integer(0))
|
||||
|
||||
cov <- uscogdata:::.coverage_table(
|
||||
fake_result, years = 2019L, n_expected = 1L,
|
||||
con = con, long_view = "spending_long_harmonized",
|
||||
expected_ids = govid
|
||||
)
|
||||
expect_equal(cov$n_units_collected, 1L)
|
||||
expect_equal(cov$n_units_reporting, 0L)
|
||||
|
||||
# Without a connection, long_view, or expected_ids, the lookup is skipped
|
||||
# rather than silently wrong.
|
||||
no_con <- uscogdata:::.coverage_table(fake_result, years = 2019L, n_expected = 1L)
|
||||
expect_true(is.na(no_con$n_units_collected))
|
||||
|
||||
no_ids <- uscogdata:::.coverage_table(
|
||||
fake_result, years = 2019L, n_expected = 1L,
|
||||
con = con, long_view = "spending_long_harmonized"
|
||||
)
|
||||
expect_true(is.na(no_ids$n_units_collected))
|
||||
})
|
||||
|
||||
test_that("n_units_collected uses the resolved basis's long view, not a hardcoded harmonized one (uscogdata#36)", {
|
||||
# spending_long_harmonized only exists when schema_version >= 5 (R/views.R
|
||||
# gates the harmonization views on it); on an older corpus cog_spending()
|
||||
# resolves basis = "raw" and queries spending_long instead. The coverage
|
||||
# lookup must follow the SAME resolved basis, not a literal
|
||||
# "spending_long_harmonized", or it hard-errors with a DuckDB catalog
|
||||
# error on every schema_version < 5 corpus -- a vintage the package
|
||||
# otherwise explicitly still supports (see test-manifest.R's dual-accept
|
||||
# tests).
|
||||
skip_if_no_corpus()
|
||||
with_doctored_schema_version(4L, {
|
||||
con <- cog_open()
|
||||
ids <- DBI::dbGetQuery(con,
|
||||
"SELECT DISTINCT canonical_govid FROM spending_long WHERE year = 2011 LIMIT 3"
|
||||
)$canonical_govid
|
||||
expect_gte(length(ids), 3L)
|
||||
|
||||
roll <- suppressMessages(cog_geographic_rollup(
|
||||
list(city = ids), category = NULL, years = 2011L))
|
||||
expect_equal(attr(roll, "provenance")$basis, "raw")
|
||||
cov <- attr(roll, "provenance")$coverage
|
||||
expect_false(is.na(cov$n_units_collected))
|
||||
expect_equal(cov$n_units_collected, length(ids))
|
||||
|
||||
cmp <- suppressMessages(cog_peer_compare(
|
||||
target_govid = ids[1], peers = ids[-1], category = NULL, years = 2011L))
|
||||
expect_equal(attr(cmp, "provenance")$basis, "raw")
|
||||
cov_peers <- attr(cmp, "provenance")$coverage
|
||||
expect_false(is.na(cov_peers$n_units_collected))
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user