Refactor contact_matrix() to use assign_age_groups()#286
Conversation
Co-authored-by: sbfnk-bot <242615673+sbfnk-bot@users.noreply.github.com>
📝 WalkthroughWalkthroughAdds and exports agegroups_to_limits(); refactors contact_matrix() to delegate participant/contact age handling to assign_age_groups() and derive numeric age limits from assigned age-group labels; adjusts survey_pop_from_countries aggregation and removes a global variable; includes docs and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant CM as contact_matrix()
participant AG as assign_age_groups()
participant P as survey$participants
participant C as survey$contacts
participant AL as agegroups_to_limits()
rect rgba(200,220,255,0.5)
CM->>AG: pass participants + contacts + age group settings
end
AG-->>P: return participants with assigned age.group / lower.age.limit
AG-->>C: return contacts with assigned age.group / lower.age.limit
rect rgba(200,255,200,0.5)
CM->>AL: extract numeric limits from P$age.group
AL-->>CM: numeric age_limits
end
rect rgba(255,220,200,0.5)
CM->>CM: apply filters, compute contact matrix using assigned groups and age_limits
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Refactors contact_matrix() to delegate participant/contact age processing and age-group assignment to assign_age_groups(), and introduces a new helper to recover lower age limits from age-group labels.
Changes:
- Replaced duplicated age-processing logic in
contact_matrix()with a single call toassign_age_groups(). - Added exported
agegroups_to_limits()(documented + tests) to extract lower age limits from age-group labels. - Simplified representative-population handling in
survey_pop_from_countries()and updated exports/changelog accordingly.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
R/contact_matrix.R |
Delegates age processing to assign_age_groups() and derives age_limits from assigned age groups. |
R/contact-matrix-utils.R |
Simplifies representative survey population calculation using lower.age.limit. |
R/limits_to_agegroups.R |
Adds new exported agegroups_to_limits() helper and roxygen docs. |
man/agegroups_to_limits.Rd |
Generated documentation for the new exported function. |
tests/testthat/test-agegroups.r |
Adds unit tests for agegroups_to_limits(). |
NAMESPACE |
Exports agegroups_to_limits. |
NEWS.md |
Notes refactor and new helper function in the dev changelog. |
R/globals.R |
Removes now-unneeded global variable entry related to the prior representative-population logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: sbfnk-bot <242615673+sbfnk-bot@users.noreply.github.com>
This PR closes #227.
Summary
contact_matrix()now delegates all age processing (imputation, missing-age handling, age group assignment) toassign_age_groups(), replacing ~70 lines of duplicated inline logicagegroups_to_limits()function (inverse oflimits_to_agegroups()) recovers lower age limits from age group labelssurvey_pop_from_countries()sincelower.age.limitis already set byassign_age_groups()agegroups_to_limits()(round-trip with brackets/dashes, character input, single group)Notes
apply_data_filter()now runs afterassign_age_groups()rather than between age processing steps. This means the top age group break is based on unfiltered max age, but empty groups don't affect results.age_limits = NULL, limits are now inferred after contact factor-to-integer conversion (matchingassign_age_groups()ordering) rather than before. This is more correct but could produce minor differences with factor contact ages — unlikely to affect real-world use.Summary by CodeRabbit