Conversation
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
Contributor
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. By default, CodeRabbit skips reviewing PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. To trigger a single review, invoke the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds small helper functions, refactors age-column/key-resolution logic, introduces preprocessing and validation steps for contact age-group assignment, scopes nolint markers, reorders .lintr entries, and applies widespread documentation, test, and vignette reflowing. No public function signatures changed. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
tests/testthat/test-matrix.r (2)
1141-1153: Formatting and description update for symmetric norm test — also flagged byair.The test description is clearer now. Note that the
airformatter suggested slightly different formatting for thecontact_matrix()calls on lines 1144–1146 and 1149–1151.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/testthat/test-matrix.r` around lines 1141 - 1153, Update the formatting of the two contact_matrix(...) calls inside the test_that("symmetric matrices with large norm weights warn", ...) to match the air formatter suggestions: place each argument on its own line with consistent indentation and no trailing commas/spaces so the calls for expect_snapshot_warning(contact_matrix(...)) and expect_warning(contact_matrix(...)) mirror each other and are aligned; keep the same arguments (survey = polymod, age_limits = c(0, 90), symmetric = TRUE) and preserve the use of expect_snapshot_warning and expect_warning and the test_that description.
47-51: Indentation changes conflict withairformatter suggestions.The
airformatter (reviewdog) has already flagged different indentation preferences for these lines. You may want to reconcile with theairsuggestions or suppress them if intentionally diverging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/testthat/test-matrix.r` around lines 47 - 51, Indentation of the multi-line logical expression is inconsistent with the air formatter; fix the continuation indentation for the expression involving polymod11$participants$part_age and polymod11$participants$dayofweek so that logical operators and parentheses are aligned per air rules (put the binary operators at the end of the preceding lines and align subsequent lines with the opening parenthesis), e.g. reflow the two clauses so both "(polymod11$participants$part_age %in% ... & polymod11$participants$dayofweek %in% ...)" blocks have consistent indentation and operator placement; run the air formatter (or reviewdog) to verify the change.R/check.R (1)
95-97:⚠️ Potential issue | 🟠 Major
format-suggestblocker still present in wrapped conditions.These condition blocks still differ from the formatter/reviewdog suggestion and are likely the CI blocker reported in this PR.
🧩 Suggested formatting fix
- if ( - !(id.column %in% - colnames(x$participants) && - id.column %in% colnames(x$contacts)) - ) { + if ( + !( + id.column %in% colnames(x$participants) && + id.column %in% colnames(x$contacts) + ) + ) { @@ - if (!check_age_column( - x$participants, participant.age.column, "Participant" - )) { + if ( + !check_age_column( + x$participants, + participant.age.column, + "Participant" + ) + ) { @@ - if (!check_age_column(x$contacts, contact.age.column, "Contact")) { + if ( + !check_age_column( + x$contacts, + contact.age.column, + "Contact" + ) + ) {As per coding guidelines:
**/*.{R,r}“Runlintr::lint_package()to check for style issues and fix any in files you modified before committing”.Also applies to: 106-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@R/check.R` around lines 95 - 97, The boolean expression using id.column and colnames(x$participants)/colnames(x$contacts) is line-wrapped in a way that violates the formatter; reformat the condition so the entire negation and logical AND are split across lines consistently (for example place the opening parenthesis after ! and put each operand on its own indented line with the && at the end or start as your style guide requires), apply the same formatting fix to the similar block that references id.column, colnames(x$participants) and colnames(x$contacts) later, and run lintr::lint_package() to ensure the style passes CI.
🧹 Nitpick comments (2)
tests/testthat/test-agegroups.r (1)
112-112: Minor: test description lost the(brackets)qualifier.The
(brackets)suffix was removed (likely for line length), but the test at line 114 specifically testsnotation = "brackets". The companion test at line 119 still says(dashes). The asymmetry is slightly confusing but not a functional issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/testthat/test-agegroups.r` at line 112, Update the test description string for the test_that("agegroups_to_limits round-trips with limits_to_agegroups", ...) to include the qualifier "(brackets)" so it matches the fact that this test calls agegroups_to_limits/limits_to_agegroups with notation = "brackets"; keep the companion test that mentions "(dashes)" unchanged. This ensures the test name reflects the specific notation parameter used and removes the asymmetry between the two related tests.tests/testthat/test-plot.r (1)
26-35: Consider de-duplicating these two legend tests.Both blocks currently test the same call; either merge them or make one cover a distinct case to keep test intent sharper.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/testthat/test-plot.r` around lines 26 - 35, The two tests in tests/testthat/test-plot.r both call matrix_plot(dta$matrix, min.legend = 4, max.legend = 40) and are redundant; either merge them into a single test (keep one of the test_that blocks, remove the duplicate) or change one to exercise a different legend scenario (e.g., different min.legend/max.legend or ad-hoc vs default legend) so they validate distinct behavior—update the test names ("contact matrix can be plotted with ad-hoc legend" and "contact matrix plotted with ad-hoc legend values") accordingly and keep the pdf()/dev.off() wrappers around the single consolidated or modified test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@man/contact_matrix.Rd`:
- Around line 128-130: The documentation for the weights argument contains a
typo: change "column names(s)" to "column name(s)" (or "column names") in the
description for the weights parameter (the \item{weights}{...} entry) so the
user-facing Rd text reads correctly and clearly; update the text inside the
\item{weights} block in man/contact_matrix.Rd accordingly.
- Line 174: The rendered help contains a leaked lint directive text "# nolint
line_length_linter."—remove that token from the roxygen source comment that
documents contact_matrix (the block listing deprecated args like survey.pop,
age.limits, sample.participants, etc.), then regenerate the Rd file by running
devtools::document() so the cleaned text is emitted; ensure only the deprecation
badge/text remains in the roxygen comment and no inline "# nolint" tokens are
present in the source roxygen block for contact_matrix.
In `@R/matrix_plot.R`:
- Around line 30-33: The roxygen docs for parameters xlab and ylab state
defaults with “(years)” but the function implementation (parameters xlab and
ylab in matrix_plot) uses “(year)”; update the docblock to match the code by
changing the default strings in the `@param` descriptions to use “(year)” so the
documented defaults match the implementation (or alternatively change the
function defaults to “(years)” if you prefer that plural form) — ensure the
`@param` lines for xlab and ylab and the actual default values in the matrix_plot
function are identical.
In `@tests/testthat/_snaps/matrix.md`:
- Line 232: Update the snapshot heading text that currently reads "# warning if
day of week weigh requested but not present" to correct the typo: change "weigh"
to "weight" so it reads "# warning if day of week weight requested but not
present"; locate the exact header string in the snapshot file and make this
single-word correction to keep wording consistent.
---
Duplicate comments:
In `@R/check.R`:
- Around line 95-97: The boolean expression using id.column and
colnames(x$participants)/colnames(x$contacts) is line-wrapped in a way that
violates the formatter; reformat the condition so the entire negation and
logical AND are split across lines consistently (for example place the opening
parenthesis after ! and put each operand on its own indented line with the && at
the end or start as your style guide requires), apply the same formatting fix to
the similar block that references id.column, colnames(x$participants) and
colnames(x$contacts) later, and run lintr::lint_package() to ensure the style
passes CI.
In `@tests/testthat/test-matrix.r`:
- Around line 1141-1153: Update the formatting of the two contact_matrix(...)
calls inside the test_that("symmetric matrices with large norm weights warn",
...) to match the air formatter suggestions: place each argument on its own line
with consistent indentation and no trailing commas/spaces so the calls for
expect_snapshot_warning(contact_matrix(...)) and
expect_warning(contact_matrix(...)) mirror each other and are aligned; keep the
same arguments (survey = polymod, age_limits = c(0, 90), symmetric = TRUE) and
preserve the use of expect_snapshot_warning and expect_warning and the test_that
description.
- Around line 47-51: Indentation of the multi-line logical expression is
inconsistent with the air formatter; fix the continuation indentation for the
expression involving polymod11$participants$part_age and
polymod11$participants$dayofweek so that logical operators and parentheses are
aligned per air rules (put the binary operators at the end of the preceding
lines and align subsequent lines with the opening parenthesis), e.g. reflow the
two clauses so both "(polymod11$participants$part_age %in% ... &
polymod11$participants$dayofweek %in% ...)" blocks have consistent indentation
and operator placement; run the air formatter (or reviewdog) to verify the
change.
---
Nitpick comments:
In `@tests/testthat/test-agegroups.r`:
- Line 112: Update the test description string for the
test_that("agegroups_to_limits round-trips with limits_to_agegroups", ...) to
include the qualifier "(brackets)" so it matches the fact that this test calls
agegroups_to_limits/limits_to_agegroups with notation = "brackets"; keep the
companion test that mentions "(dashes)" unchanged. This ensures the test name
reflects the specific notation parameter used and removes the asymmetry between
the two related tests.
In `@tests/testthat/test-plot.r`:
- Around line 26-35: The two tests in tests/testthat/test-plot.r both call
matrix_plot(dta$matrix, min.legend = 4, max.legend = 40) and are redundant;
either merge them into a single test (keep one of the test_that blocks, remove
the duplicate) or change one to exercise a different legend scenario (e.g.,
different min.legend/max.legend or ad-hoc vs default legend) so they validate
distinct behavior—update the test names ("contact matrix can be plotted with
ad-hoc legend" and "contact matrix plotted with ad-hoc legend values")
accordingly and keep the pdf()/dev.off() wrappers around the single consolidated
or modified test.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
.lintrNEWS.mdR/as_contact_survey.RR/assign-age-groups.RR/check.RR/checkers.RR/clean.RR/contact-matrix-utils.RR/contact_matrix.RR/deprecated.RR/lists.RR/load-survey-utils.RR/matrix_plot.RR/new_contact_survey.RR/pop_age.RR/subset-contact-survey.RR/survey.RR/utils.RR/wpp_age.Rman/add_age.Rdman/as_contact_survey.Rdman/check.Rdman/clean.Rdman/contact_matrix.Rdman/get_survey.Rdman/matrix_plot.Rdman/new_contact_survey.Rdman/pop_age.Rdman/survey.Rdman/wpp_age.Rdtests/testthat/_snaps/matrix.mdtests/testthat/test-agegroups.rtests/testthat/test-convert-factor-to-integer.Rtests/testthat/test-matrix.rtests/testthat/test-plot.rtests/testthat/test-survey-country-population.Rtests/testthat/test-weigh.Rvignettes/intro.qmdvignettes/socialmixr.Rmd
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
R/check.R (1)
1-31:cli_warnmessage: missing\\continuation andcallparameterTwo minor inconsistencies in
check_age_column'scli_warn(lines 25–29):
Raw newlines in the format string. The string spans multiple source lines without
\\line-continuations. Unlike every other multi-linecli_warn/cli_abortin this file (e.g. lines 86–88, 100–103, 122–124) which use\\to produce a single paragraph, this string contains embedded\ncharacters and leading spaces on the continuation lines that will appear verbatim in the rendered warning.Missing
callparameter. All warning helpers incheckers.Raccept and forwardcall = rlang::caller_env()so that the warning points to the user-facing call site.check_age_columnomits this, so the warning will appear to originate fromcheck_age_columnitself rather than fromcheck.contact_survey. Since the function is@noRd, adding acallparameter keeps it consistent and forward-compatible if it is reused elsewhere.♻️ Proposed fix
+check_age_column <- function(df, age_column, label, + call = rlang::caller_env()) { if (age_column %in% colnames(df)) { return(TRUE) } exact_col <- paste(age_column, "exact", sep = "_") min_col <- paste(age_column, "est_min", sep = "_") max_col <- paste(age_column, "est_max", sep = "_") if ( (exact_col %in% colnames(df)) || (min_col %in% colnames(df) && max_col %in% colnames(df)) ) { return(TRUE) } cli::cli_warn( - "{label} age column {.arg {age_column}} or columns to - estimate {tolower(label)} age ({.arg {exact_col}} or {.arg {min_col}} - and {.arg {max_col}}) do not exist in the {tolower(label)} data frame." + "{label} age column {.arg {age_column}} or columns to estimate \\ + {tolower(label)} age ({.arg {exact_col}} or {.arg {min_col}} \\ + and {.arg {max_col}}) do not exist in the {tolower(label)} data frame.", + call = call ) FALSE }Then forward
callat the two call sites incheck.contact_survey:- if (!check_age_column(x$participants, participant.age.column, "Participant")) { + if (!check_age_column(x$participants, participant.age.column, "Participant", + call = rlang::caller_env())) { - if (!check_age_column(x$contacts, contact.age.column, "Contact")) { + if (!check_age_column(x$contacts, contact.age.column, "Contact", + call = rlang::caller_env())) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@R/check.R` around lines 1 - 31, The cli::cli_warn in check_age_column uses a multi-line string with raw newlines and missing the standard call forwarding; update the cli::cli_warn call in check_age_column to use backslash line-continuations (\"\\\") so the message is a single paragraph and add call = rlang::caller_env() to the cli::cli_warn invocation. Then, in check.contact_survey where check_age_column is invoked, forward the caller environment by passing call = rlang::caller_env() (or otherwise propagate the call argument) so warnings point to the user-facing call site.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@R/load-survey-utils.R`:
- Around line 273-279: The nolint markers are mispaired causing cyclocomp
suppression (started earlier at the first "# nolint start: cyclocomp_linter.")
to close too early; update the block around the construction of key_code (in the
try_merge_additional_files / load-survey-utils.R code) so that the "# nolint
start: object_usage_linter." paired with its corresponding "# nolint end" does
not close the cyclocomp suppression prematurely — either move the second "#
nolint end" to after the end of the try_merge_additional_files block (after the
code that follows key_code) so the two starts/end markers pair correctly, or
replace the block-level markers with per-line "# nolint" annotations on the
specific lines (e.g., the key_code lines) to avoid mispairing.
---
Nitpick comments:
In `@R/check.R`:
- Around line 1-31: The cli::cli_warn in check_age_column uses a multi-line
string with raw newlines and missing the standard call forwarding; update the
cli::cli_warn call in check_age_column to use backslash line-continuations
(\"\\\") so the message is a single paragraph and add call = rlang::caller_env()
to the cli::cli_warn invocation. Then, in check.contact_survey where
check_age_column is invoked, forward the caller environment by passing call =
rlang::caller_env() (or otherwise propagate the call argument) so warnings point
to the user-facing call site.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
R/check.RR/checkers.RR/clean.RR/contact_matrix.RR/load-survey-utils.RR/matrix_plot.RR/new_contact_survey.RR/survey.Rman/contact_matrix.Rdman/matrix_plot.Rdtests/testthat/test-matrix.rtests/testthat/test-survey-country-population.R
✅ Files skipped from review due to trivial changes (2)
- tests/testthat/test-survey-country-population.R
- man/contact_matrix.Rd
🚧 Files skipped from review as they are similar to previous changes (5)
- man/matrix_plot.Rd
- R/survey.R
- R/new_contact_survey.R
- R/contact_matrix.R
- tests/testthat/test-matrix.r
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
R/check.R (1)
9-10:⚠️ Potential issue | 🔴 CriticalFix
check_age_columnsignature formatting — pipeline is failing because of this.The
# nolint: indentation_linter.comment suppresses lintr, but theformat-suggest.yamlpipeline runs the air formatter, which has its own formatting rules and is not affected bynolint. The pipeline error at lines 9–15 will keep blocking until the signature is reformatted to the style air expects (one argument per line, closing paren on its own line).🔧 Proposed fix (matches the air formatter's suggestion from the previous review)
-check_age_column <- function(df, age_column, label, - call = rlang::caller_env()) { # nolint: indentation_linter. +check_age_column <- function( + df, + age_column, + label, + call = rlang::caller_env() +) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@R/check.R` around lines 9 - 10, The function signature for check_age_column is currently on a single line with a trailing "# nolint: indentation_linter." which the air formatter rejects; reformat the signature to one argument per line with the closing parenthesis on its own line (e.g. split df, age_column, label, and call = rlang::caller_env() each onto separate lines) and remove or relocate the nolint comment so the air formatter can accept the style; ensure the function name check_age_column and the default argument call = rlang::caller_env() remain unchanged.
🧹 Nitpick comments (1)
R/check.R (1)
99-100: Avoidnolintworkaround — reformat the compound condition instead.The two
# nolint: indentation_linter.suppressions exist only because the logical expression inside!(... && ...)isn't broken at a point that satisfies the indentation linter. Restructuring the condition eliminates the need for the suppression:♻️ Proposed refactor
- if ( - !(id.column %in% - colnames(x$participants) && # nolint: indentation_linter. - id.column %in% colnames(x$contacts)) # nolint: indentation_linter. - ) { + part_has_id <- id.column %in% colnames(x$participants) + cont_has_id <- id.column %in% colnames(x$contacts) + if (!(part_has_id && cont_has_id)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@R/check.R` around lines 99 - 100, The compound logical check inside the negation uses a single-line && split with nolint comments; refactor the condition in the function that performs the check (the expression referencing id.column, x$participants and x$contacts) so the logical operands are split onto separate lines (e.g., test each membership separately and combine with && on its own line or negate each part as appropriate) to satisfy the indentation linter and remove both "# nolint: indentation_linter." comments; ensure the logic of the overall !(...) expression is preserved after reformatting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@R/check.R`:
- Around line 9-10: The function signature for check_age_column is currently on
a single line with a trailing "# nolint: indentation_linter." which the air
formatter rejects; reformat the signature to one argument per line with the
closing parenthesis on its own line (e.g. split df, age_column, label, and call
= rlang::caller_env() each onto separate lines) and remove or relocate the
nolint comment so the air formatter can accept the style; ensure the function
name check_age_column and the default argument call = rlang::caller_env() remain
unchanged.
---
Nitpick comments:
In `@R/check.R`:
- Around line 99-100: The compound logical check inside the negation uses a
single-line && split with nolint comments; refactor the condition in the
function that performs the check (the expression referencing id.column,
x$participants and x$contacts) so the logical operands are split onto separate
lines (e.g., test each membership separately and combine with && on its own line
or negate each part as appropriate) to satisfy the indentation linter and remove
both "# nolint: indentation_linter." comments; ensure the logic of the overall
!(...) expression is preserved after reformatting.
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.lintr (1)
1-10: Consider pinning cyclocomp_linter and line_length_linter parameters explicitly for stability across lintr versions.The current
.lintrconfig relies onlinters_with_tags(tags = NULL)to implicitly enablecyclocomp_linterandline_length_linterwith their built-in defaults (complexity_limit = 15Landlength = 80L). While these defaults currently match your intended thresholds, explicitly configuring them guards against silent changes if a future lintr release alters its defaults:🔧 Proposed explicit configuration
linters: linters_with_tags( tags = NULL, # include all linters object_name_linter = NULL, undesirable_function_linter = NULL, implicit_integer_linter = NULL, todo_comment_linter = NULL, function_argument_linter = NULL, indentation_linter = NULL, # air handles indentation via style.yaml + cyclocomp_linter(complexity_limit = 15L), + line_length_linter(length = 80L), backport_linter("3.5.0") )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.lintr around lines 1 - 10, The lintr config currently relies on linters_with_tags(tags = NULL) which implicitly enables cyclocomp_linter and line_length_linter with default values; explicitly add cyclocomp_linter(complexity_limit = 15L) and line_length_linter(length = 80L) into the linters list (alongside other nulled linters) so their thresholds are pinned and won’t change if lintr defaults change, e.g. include references to cyclocomp_linter and line_length_linter inside the linters_with_tags(...) call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@R/check.R`:
- Around line 30-36: The cli::cli_warn call that builds the multi-line glue
string using variables label, age_column, exact_col, min_col, and max_col uses
single backslashes before physical newlines; replace each single "\" with "\\"
in that string literal so the R parser produces a single backslash in the
resulting string (which glue recognises as the continuation marker), and make
this change consistently wherever similar multi-line glue strings are used
(e.g., the other cli::cli_warn/cli::cli_abort occurrences).
- Around line 30-36: The cli::cli_warn call's glue string uses a single
backslash for physical-line continuation, which produces an unrecognized escape
in R string literals; update the string so each physical-newline continuation
uses a double backslash (i.e. replace each "\" before the newline with "\\"),
preserving the same message and the interpolated symbols (label, age_column,
exact_col, min_col, max_col) so glue continues to join the lines correctly.
---
Nitpick comments:
In @.lintr:
- Around line 1-10: The lintr config currently relies on linters_with_tags(tags
= NULL) which implicitly enables cyclocomp_linter and line_length_linter with
default values; explicitly add cyclocomp_linter(complexity_limit = 15L) and
line_length_linter(length = 80L) into the linters list (alongside other nulled
linters) so their thresholds are pinned and won’t change if lintr defaults
change, e.g. include references to cyclocomp_linter and line_length_linter
inside the linters_with_tags(...) call.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.lintrNEWS.mdR/check.RR/clean.Rtests/testthat/test-matrix.r
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/testthat/test-matrix.r
- NEWS.md
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
Collaborator
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@R/check.R`:
- Around line 54-56: The roxygen comment in R/check.R describing the
fallback-column requirement is missing the word "exist" (reads "... must (see
the `estimated.participant.age` option in [contact_matrix()])"); update the
documentation text in that roxygen block so both occurrences of the phrase read
"... must exist (see the `estimated.participant.age` option in
[contact_matrix()])" to produce grammatically correct help output. Ensure you
change both places in the same doc block where the fallback-column requirement
is described.
- Around line 107-108: The warning message in R/check.R incorrectly references
the parameter name `{.arg id.columns}`; update the string to use the actual
function argument `{.arg id.column}` so the warning reads "{.arg id.column}
{.val {id.column}} does not exist in both the participants and contacts data
frames." Locate the message construction that references `id.column` (the code
assembling the warning) and replace the plural token `id.columns` with
`id.column` to ensure the parameter name in the warning matches the function
signature.
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
Co-authored-by: sbfnk <sebastian.funk@lshtm.ac.uk>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR closes #293.
Removes the four "extra exclusion" linters from
.lintrthat were marked as"should ideally be removed at some point":
cyclocomp_linter(default threshold 15) — reduced complexity ofcheck.contact_survey()(21→11),[.contact_survey(17→9), andfind_unique_key()(17→8) by extracting helper functions. Excludedcontact_matrix(),download_survey(), andtry_merge_additional_files()via
nolint(too large/deprecated to tackle here).object_usage_linter— works correctly with the existingroxyglobalssetup (only 3 false positives from cli string interpolation, suppressed with
nolint). Removed one genuinely unused assignment in deprecated code.line_length_linter(default 80 chars) — wrapped long roxygen@paramdescriptions, code comments, cli messages, test descriptions, and vignette
code across ~23 files. One unavoidable long line (comma-separated deprecated
param names in
contact_matrix.R) excluded vianolint.indentation_linter— fixed 4 indentation issues across 3 files.Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
Chores