Skip to content

Comments

Alcohol#166

Open
caitlink12 wants to merge 17 commits intofeature/v3.0.0-validation-infrastructurefrom
alcohol
Open

Alcohol#166
caitlink12 wants to merge 17 commits intofeature/v3.0.0-validation-infrastructurefrom
alcohol

Conversation

@caitlink12
Copy link

Alcohol variables ALCDTTM, ALWDWKY, binge drinker and it's dependents (ALW_2A1, ALW_2A2, ALW_2A3, ALW_2A4, ALW_2A5, ALW_2A6, ALW_2A7) were updated for ICES survey cycles in variable and variable_details sheets.

included ICES survey cycles into existing ALCDTTM variable and updated suffix from _i to _m
included ICES survey cycles into existing ALWDWKY variable and updated suffixes from _i to _m
included ICES survey cycles into existing ALW_2A1 variable and updated suffixes from _i to _m
included ICES survey cycles to existing ALW_2A2 variable and updated suffixes from _i to _m
included ICES survey cycles to existing ALW_2A3 variable and updated suffixes from _i to _m
included ICES survey cycles to existing ALW_2A4 variable and updated suffixes from _i to _m
included ICES survey cycles to existing ALW_2A5 variable and updated suffixes from _i to _m
included ICES survey cycles to existing ALW_2A6 variable and updated suffixes from _i to _m
included ICES survey cycles to existing ALW_2A7 variable and updated suffixes from _i to _m
included ICES survey cycles into existing ALW_1 variable with suffix _m
included ICES survey cycles into existing binge_drinker derived variable
included ICES survey cycles into existing ALW_1 variable
missed line of ALW_2A1 to include ICES survey cycles
P0: ALW_2A1 row 1 had ALW_1's source variables (ALCA_5, ALCC_5, ALCE_5,
ALW_005) instead of ALW_2A1's (ALCA_5A1, ALCC_5A1, ALCE_5A1, ALW_010).
Copy-paste error that would produce silently wrong data at runtime.

P1: Converted cchs2009_s/cchs2010_s/cchs2012_s to cchs2009_m/cchs2010_m/
cchs2012_m in databaseStart for 11 alcohol variables (48 rows).

Verified against MCP metadata database and StatCan PDF codebooks.
CEP-011 review artifacts included.
@DougManuel
Copy link
Contributor

Review: PR #166 (Alcohol)

Reviewed the _i_m conversion for 11 alcohol variables. L6 integration testing passed for all 9 PUMF cycles — no step changes at era boundaries.

Full review details in CEP-011.

P0: ALW_2A1 row 1 — wrong source variables (fixed)

ALW_2A1 row 1 (recStart=[0,50]) had ALW_1's source variables instead of ALW_2A1's. Copy-paste error.

Was (wrong):

cchs2001_p::ALCA_5, cchs2003_p::ALCC_5, cchs2005_p::ALCE_5,
cchs2015_2016_p::ALW_005, cchs2017_2018_p::ALW_005, [ALW_1]

Now (corrected, matching rows 2-4):

cchs2001_p::ALCA_5A1, cchs2003_p::ALCC_5A1, cchs2005_p::ALCE_5A1,
cchs2015_2016_p::ALW_010, cchs2017_2018_p::ALW_010,
cchs2001_m::ALCA_5A1, cchs2003_m::ALCC_5A1, cchs2005_m::ALCE_5A1,
cchs2015_2016_m::ALW_010, cchs2017_2018_m::ALW_010, [ALW_2A1]

The _5 variables are "any alcohol past week" (yes/no); the _5A1 variables are "number of drinks on Sunday" (continuous). At runtime this would have silently produced wrong values.

P1: _s databases → _m (fixed)

Converted cchs2009_s/cchs2010_s/cchs2012_s to cchs2009_m/cchs2010_m/cchs2012_m in databaseStart for all 11 alcohol variables (48 rows). variables.csv already had _m.

Both fixes are in commit 30667cf.

Copy link
Collaborator

@rafdoodle rafdoodle left a comment

Choose a reason for hiding this comment

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

Alcohol variables — _m database update

Completed (11 variables)

Updated databaseStart and variableStart in both variables.csv and variable_details.csv:

  • Replaced legacy _s database entries with full _m equivalents
  • Added missing singular-year _m entries (cchs2009_m, cchs2010_m, cchs2011_2012_m, cchs2012_m) in correct chronological order
  • Added explicit _m source variable mappings in variableStart
  • Fixed miscellaneous formatting issues (trailing spaces, double spaces, incorrect copy-row variableStart)

Variables:

  • ALCDTTM — two-group structure preserved (pre-/post-2007)
  • ALW_1
  • ALW_2A1 — also fixed copy-row variableStart; removed trailing space
  • ALW_2A2ALW_2A7
  • ALWDWKY — fixed space after :: in variableStart
  • binge_drinker

Still needs _m updating

  • ALC_005 — no _m entries; only covers 2001–2007/2008 _p
  • ALC_1 — has only cchs2009_m, cchs2010_m, cchs2012_m; needs full _m set + explicit variableStart mappings
  • ALCDTYP — no _m entries; only covers 2001–2007/2008 _p
  • ALCDTYP_A — typo 2011_2012_m (missing cchs prefix); missing cchs2009_m, cchs2010_m, cchs2012_m singular years
  • ALWDDLY — has only cchs2009_m, cchs2010_m, cchs2012_m; needs full _m set + explicit variableStart mappings; double space in databaseStart; trailing spaces in variableStart
  • ALWDVLTR_der — has only cchs2009_m, cchs2010_m, cchs2012_m; needs full _m set; double space in databaseStart
  • ALWDVSTR_der — has only cchs2009_m, cchs2010_m, cchs2012_m; needs full _m set; double space in databaseStart
  • low_drink_score — has only cchs2009_m, cchs2010_m, cchs2012_m; needs full _m set + explicit variableStart mappings
  • low_drink_score1 — no _m entries; only covers select _p years

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.

3 participants