Conversation
|
The following subjects now contain raw and derived variables extended past 2017-2018 PUMF and Master cycles in
Deferred / not yet extended:
Sources: 613 Data Dictionary and NotebookLLM @DougManuel Please let me know if you think this is good enough for v3 or if more extensions are warranted. Feel free to use the MCP to validate Rafidul |
…adl.R @note metadata Review findings: the failing CSV check was a workflow bug (devtools not installed on the runner), not worksheet violations - worksheets pass all checks locally on the same commit. Adds devtools to the workflow install step and rewords the catch-all failure message to surface the real cause. Restores the three @note version-metadata lines dropped from R/adl.R in the July refactor, returning the test suite to fully green (828 passing, 0 failures). CEP-018 contains the release review summary (L3-L5 sweeps, L6 cross- cycle prevalence matrix, ADL step-change diagnosis, fast-fix regression checks) with two open items for the team: issue #139 arms and the missing-data priority decision.
Code reviewReview of v3 -> dev: automated L3-L5 sweeps over all 481 changed variables (120 added, 41 removed, 320 modified), L6 cross-cycle integration sampled across the changed domains, and package health checks. Full findings, the L6 prevalence matrix, and diagnosis scripts: Fixes applied in 1406aff
Open items before merge (or as fast-follows)
Verified cleanEra-boundary source mappings (0 gaps), databaseStart tokens (CEP-017's malformed DEN_132 tokens fixed), no deprecated L6 limitation: no sample data exists for 2019-2023 PUMFs, so this release's headline coverage extensions to those cycles are validated by worksheet checks only. Recommendation: approve once the CI check re-runs green; items 1-2 above can land in this PR (S-effort each) or as fast-follow issues at the team's preference. CEP: |
…, cross-file keys Extends check_worksheet() with three declarative rules, setting the stage for progressively richer schema validation (full column schema deferred to the v4 refactor, where this merges into the recodeflow schema layer): - Database-token registry (inst/metadata/schemas/core/database_registry.yaml, 32 valid identifiers): every databaseStart token is validated against the registry, so typo identifiers fail at PR time instead of becoming silent dead rows. Adding a new cycle now requires a deliberate, reviewable registry entry. - Controlled vocabularies (column_enums in the worksheet schemas): typeStart (cat/cont/N/A) and typeEnd (cat/cont) in variable_details, variableType (Categorical/Continuous) in variables. - Cross-file key integrity (check_cross_file_keys()): every variable_details entry must have a variables.csv row. Registry naming follows the recodeflow database-metadata draft for a clean v4 merge. Fixes one pre-existing worksheet cell surfaced by the enum rule (HWTGCOR_der Func:: row had an empty typeStart; now N/A per convention) and updates test fixtures that used never-valid values (variableType 'cont', bare 'cchs' and invented 'cchs2013_p' tokens). Full checker green on the worksheets (all five rule groups); suite at 836 passing, 0 failures. Relates to CEP-017 Track 2 and CEP-018.
|
Follow-up in 638de98 — first increment of schema-driven content validation, per the validation discussion:
All rules run in the existing CI check and scoped mode. One pre-existing cell fixed (HWTGCOR_der's Registry naming follows the recodeflow database-metadata draft so this layer merges cleanly into recodeflow at the v4 refactor. Full column schema (types, cardinality, dummyVariable grammar) stays deferred to v4 per CEP-017 Track 2. |
…iority rules The CCHS missing-data pattern schema was shipped but dormant. It now has two thin consumers reading only its normative blocks: - clean_variables()'s fallback pattern (used when a variable has no worksheet metadata) is built from the schema's pattern families instead of hardcoded values. Net behavior gain: the early-cycle decimal missing codes (999.6-999.9), declared in the schema but absent from the old hardcoded fallback, are now handled. - load_priority_rules() resolves the not-applicable-over-missing priority from the schema's declared hierarchy, replacing the built-in fallback and its once-per-session 'YAML files not found' warning. The priority decision is now read from a versioned, reviewable file. The schema gains an applies_to: source_codes declaration (codes are in the raw StatCan domain, not the harmonized domain) and a consumer note marking variable_assignments as reference-only - variable_details.csv remains the per-variable source of truth. Also adds .DS_Store to .Rbuildignore so Finder droppings stay out of the built package. Suite: 842 passing, 0 failures. Relates to CEP-017 Track 3.
Removes the three sources of drift and confusion flagged in review:
- transformation_map: deleted. It embedded R expressions as YAML strings
(haven::tagged_na('a')) and duplicated priority_hierarchy, which is now
the single normative code-to-tag encoding with symbolic tag values.
- variable_assignments: deleted. Per-variable pattern assignment belongs
to variable_details.csv; the block's judgment content (the never-smoked
category-6 collision warning, canonical examples per family) is
preserved in a non-normative authoring_guidance section.
- implementation blocks: deleted. They pointed at a never-merged function
(preprocess_cchs_missing_codes) and a feature-branch-only constants
file. Replaced by an accurate consumers list naming the three functions
that actually read this schema.
- na_category_definitions renamed na_category_semantics, keeping the
epidemiological guidance (denominator treatment) and dropping its
duplicate code lists.
Consumers verified byte-identical in behaviour: fallback pattern codes,
decimal variants, and priority resolution unchanged; suite at 842
passing, 0 failures. File shrinks from 219 to ~130 lines with no loss of
normative content.
Review follow-through: validation and schema consumption (
|
Open items — all addressed1. Issue #139 — Already resolved in 2. Missing-data priority The code is correct and internally consistent — NA::a (not applicable) wins over NA::b (missing/not stated) across:
The CEP-018 review referenced a "never-merged config on 3. DEN_132 / cchs2001_p The 2001 PUMF mapping ( |
7 participants
Summary of changes to address #189
Hello @DougManuel and @yulric,
Here are a summary of changes I made so far over the past week to help finalize cchsflow v3:
Must do
Namespace and passing all tests for CRAN submission:
ca02a94— Restructured DESCRIPTION Depends/Imports for CRAN compliance8171e96— Added .claude/ and ceps/ to .Rbuildignore51c81a0— Replaced non-ASCII characters for CRAN portabilitye13cc20— Fixed broken examples, dead cross-references, Rd usage mismatches05421fe— Fixed test assertions, qualified sjlabelled in tests5301929— Resolved global variable binding NOTEs35db4c6— Updated NAMESPACE via roxygen, fixed RData object namingc1a16df— Upgraded version to 3.0.0devtools::check()passes with 0 errors, 0 warnings, 2 NOTEs (installed size + timestamps — both non-actionable)Review and update documentation and package build:
860167b— Standardized roxygen documentation across all 46 v3 3-step functions to follow adl/alcohol template: verb-first titles,@description,@detailswith domain context,@returnwith tagged_na info,@examples,@seealsoe13cc20— Fixed broken examples, synced@paramwith function signatures, fixed Rd line widthsAssess infrastructure — validation functions and branch consolidation:
967be79— Consolidated validation infrastructure from Doug/Yulric feature branches (feature/v3.0.0-validation-infrastructure,v3.0.0-validation-infrastructure, yulric fork,3.0.0-merge-strategy); confirmed all cherry-pick candidates already merged or superseded; trimmed worksheets to match YAML schemas; added extra column removal tofix_worksheet()4c22630— Updatedvariable_details.yamlschema to enforce alphabetical sorting viaid_column_nameReview and use variable and variable_details schemas:
967be79— Confirmed v3 YAML schemas are UTD with correct 10-column (variables) and 16-column (variable_details) structure; trimmed CSVs to match;check_worksheet()andfix_worksheet()return 0 errors on both worksheetsNice to have
DemPoRTv2 variables updated to CCHS 2022 (PUMF) and 2023 (Master):
d50d7aa— Age and sex variables extended to all 2001-2023 PUMF and Master cycles52d5c0b— SDC ethnicity/language/migration variables extended to 2019-2023 Master (SDCGLHM/SDCDFOLS: pre-2015 explicit source variable mappings and Master coverage gaps #178, Add successor ethnicity/language/migration variables (2015+ renames) #179)850039f— ADL variables (12 raw + 3 derived) extended to 2019-2023 Mastercb4d1a4— Extended ALCDTTM, ALWDWKY, FVCD* (diet), DEN_132, SLP, EHG2/EDUDR04, PAA/energy_exp, HWTD* (BMI), DHH_MS, GEN_07/10 to 2021-2023a666197— Extended chronic conditions (CCC_071, CCC_101, CCC_121, CCC_151, CCC_181, CCC_280) to 2021-2023e67ef32,2c99210— Standardized worksheet units and NA labels per csv-conventions.md across both reposDerived variables adopt 3-step approach (#173):
f078e9d— Incorporated 3-step architecture (clean_variables()→case_when→clean_variables()) for all DemPoRTv2-used derived variable functions; properly named functions (calculate_*,categorize_*,derive_*,assess_*)1ca5e6e— Standardized NULL handling withtagged_na("c")for all-NULL inputs, addedexpand_null_inputs()DRY helper, added legacy test files0860899— Fixed NA handling in education function (get_priority_missinginstead ofassign_missingfor all-missing inputs)4c22630— RefactoredCOPD_Emph_derintoCCC_091_derwith singlecategorize_CCC_091()function using consistentCCC_091input across all erasassign_missing(),get_priority_missing(),any_missing(), worksheet patterns) remains deferred as a separate sub-issue of Standardise NULL-handling pattern for standalone derived variable functions #173. The currenttagged_na("c")return for all-NULL inputs works for standalone function calls but is not yet a first-class missing type in the v3 pipeline.Tagged NA preservation through recodeflow (#190):
b6270d2— Addedprep_cat_output()helper as a workaround for a recodeflow architectural issue whereas.factor(unlist(...))inrec_with_table()strips tagged NA bit patterns during factorization, causing all tagged NAs to collapse to plain NA and get mislabeled as NA(c) by downstream processing. The helper converts tagged NAs to their string equivalents ("NA(a)","NA(b)","NA(c)") at the return boundary of categorical Func:: functions so they survive factor conversion. Applied to all 18 categorical derived variable functions. The proper fix belongs in recodeflow'srec_with_table()factorization step, which would make the per-function workaround unnecessary — see Tagged NAs lost during factor conversion in categorical Func:: functions #190 for details.DemPoRTv2 protocol branch alignment:
The v3 3-step architecture has been fully incorporated into DemPoRTv2's protocol branch.
R/cchsflow-helpers.Rprovides the same v3 infrastructure primitives (clean_variables(),any_missing(),get_priority_missing(),assign_missing(),normalize_input_lengths(),expand_null_inputs(),prep_cat_output()), andR/custom-functions.Rcontains identical copies of all DemPoRTv2-used derived variable functions using the same 3-step pattern. These are sourced by the targets pipeline (via_targets.R) so thatrecodeflow::rec_with_table()calls the v3 functions during harmonization. The DemPoRT worksheet (worksheets/cchsflow-variable-details.csv) references these functions viaFunc::entries and has been updated to match cchsflow's csv-conventions.md (dummyVariable naming, NA labels, unit consistency, block ordering, alphabetical sorting).Note that only DemPoRTv2-used variables and their derived functions have been fully updated to the v3 architecture and extended to 2022/2023 cycles; other variables within the same sections (e.g., remaining chronic conditions, physical activity, alcohol) will continue to be updated in coming days.
Test plan
devtools::check()— 0 errors, 0 warnings, 2 NOTEsdevtools::test()— 0 failures, 829+ passingcheck_worksheet()— 0 errors on both worksheetscheck_recode_blocks()— 0 collisionsexec/check-worksheets.Randexec/fix-worksheets.Rrun without errorsPlease let me know what you think.
Rafidul