Prepare SelectSim 0.1.6 for CRAN submission#10
Conversation
|
Warning Review limit reached
Next review available in: 55 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis release-preparation update adds structured dataset and function help, smaller examples, corrected wording, citation formatting, CRAN check notes, changelog edits, spelling support, metadata updates, and minor R style changes. ChangesDocumentation and release preparation
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@man/filter_maf_column.Rd`:
- Around line 16-18: Update the implementation of the fixed argument in the
relevant filtering function so fixed = FALSE passes through to grep as fixed =
FALSE, enabling regex matching, while preserving literal matching when fixed =
TRUE; keep the fixed documentation consistent with this behavior.
In `@man/filter_maf_truncating.Rd`:
- Line 12: Update the `schema` parameter description in `filter_maf_truncating`
documentation to identify it as a nested list, and retain the reference to
`TCGA_maf_schema` as the example of the expected structure.
In `@R/gam_utils.r`:
- Line 329: Update the roxygen documentation for the column parameter of
stat_maf_column() to describe it as a column name, matching its use in maf[,
column] and the string example "Variant_Classification"; remove the incorrect
data-frame schema and TCGA_maf_schema reference.
- Line 231: Update the `@param schema` documentation for all four filtering
helpers in `R/gam_utils.r` to describe `schema` as a list containing column
mappings and mutation-type definitions, rather than as a data-frame schema. Keep
the references to `TCGA_maf_schema` and equivalent schema examples as
appropriate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4de6ad12-6346-4b77-98d0-5e2d0321ba0f
📒 Files selected for processing (28)
.RbuildignoreNEWS.mdR/data.RR/gam_utils.rR/selectX_run.RREADME.RmdREADME.mdcran-comments.mdinst/WORDLISTman/GENIE_maf_schema.Rdman/TCGA_maf_schema.Rdman/filter_maf_column.Rdman/filter_maf_ignore.Rdman/filter_maf_missense.Rdman/filter_maf_schema.Rdman/filter_maf_truncating.Rdman/luad_maf.Rdman/luad_result.Rdman/luad_run_data.Rdman/maf2gam.Rdman/mutation_type.Rdman/oncokb_genes.Rdman/oncokb_truncating_genes.Rdman/selectX.Rdman/stat_maf_column.Rdman/variant_catalogue.Rdvignettes/data_processing.Rmdvignettes/introduction.Rmd
| \item{inclusive}{a boolean to include or exclude the data frame with values in list provided} | ||
|
|
||
| \item{fixed}{a grep argument to specify if grep use the argumnet as string or not} | ||
| \item{fixed}{a grep argument to specify if grep use the argument as string or not} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== relevant files ==\n'
git ls-files | rg '(^|/)(filter_maf_column|filter_maf_column\\.Rd|R/|man/)' || true
printf '\n== locate symbols/usages ==\n'
rg -n "filter_maf_column|inclusive|fixed" R man -S || true
printf '\n== read Rd file ==\n'
sed -n '1,120p' man/filter_maf_column.Rd
printf '\n== candidate implementation files ==\n'
fd -a 'filter_maf_column' R man || trueRepository: CSOgroup/SelectSim
Length of output: 6196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== function body around filter_maf_column ==\n'
sed -n '88,125p' R/gam_utils.r
printf '\n== roxygen docs around the parameter text ==\n'
sed -n '74,86p' R/gam_utils.r
printf '\n== other helpers that delegate to filter_maf_column ==\n'
sed -n '145,205p' R/gam_utils.rRepository: CSOgroup/SelectSim
Length of output: 4767
Clarify fixed or change the implementation. fixed = FALSE still calls grep(..., fixed = TRUE), so regex matching never happens; either update the code or document that only literal matching is supported.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@man/filter_maf_column.Rd` around lines 16 - 18, Update the implementation of
the fixed argument in the relevant filtering function so fixed = FALSE passes
through to grep as fixed = FALSE, enabling regex matching, while preserving
literal matching when fixed = TRUE; keep the fixed documentation consistent with
this behavior.
| \item{maf}{a maf as dataframe} | ||
|
|
||
| \item{schema}{a schema of datafrane check Select::TCGA_maf_schema for example} | ||
| \item{schema}{a data-frame schema; see \code{TCGA_maf_schema} for an example} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Describe schema as a list, not a data-frame schema.
TCGA_maf_schema is defined in R/gam_utils.r:31-46 as a nested list, and filter_maf_truncating() accesses its nested entries. Calling this a “data-frame schema” may lead users to pass a data frame instead of the expected list.
-\item{schema}{a data-frame schema; see \code{TCGA_maf_schema} for an example}
+\item{schema}{a list describing the MAF schema; see \code{TCGA_maf_schema} for an example}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| \item{schema}{a data-frame schema; see \code{TCGA_maf_schema} for an example} | |
| \item{schema}{a list describing the MAF schema; see \code{TCGA_maf_schema} for an example} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@man/filter_maf_truncating.Rd` at line 12, Update the `schema` parameter
description in `filter_maf_truncating` documentation to identify it as a nested
list, and retain the reference to `TCGA_maf_schema` as the example of the
expected structure.
| #' | ||
| #' @param maf a maf as dataframe | ||
| #' @param schema a schema of datafrane check Select::TCGA_maf_schema for example | ||
| #' @param schema a data-frame schema; see `TCGA_maf_schema` for an example |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe schema as a list, not a data frame.
TCGA_maf_schema and GENIE_maf_schema are lists containing column mappings and mutation-type definitions. Calling them a “data-frame schema” is misleading for all four filtering helpers.
Proposed wording
-#' `@param` schema a data-frame schema; see `TCGA_maf_schema` for an example
+#' `@param` schema a schema list defining MAF column mappings and mutation classes;
+#' see `TCGA_maf_schema` for an exampleAlso applies to: 258-258, 281-281, 304-304
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@R/gam_utils.r` at line 231, Update the `@param schema` documentation for all
four filtering helpers in `R/gam_utils.r` to describe `schema` as a list
containing column mappings and mutation-type definitions, rather than as a
data-frame schema. Keep the references to `TCGA_maf_schema` and equivalent
schema examples as appropriate.
| #' | ||
| #' @param maf a maf as dataframe | ||
| #' @param column a schema of datafrane check Select::TCGA_maf_schema for example | ||
| #' @param column a data-frame schema; see `TCGA_maf_schema` for an example |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document column as a column name.
stat_maf_column() uses maf[, column] and the example passes "Variant_Classification", so this parameter is not a data-frame schema.
Proposed wording
-#' `@param` column a data-frame schema; see `TCGA_maf_schema` for an example
+#' `@param` column column name in the MAF data frame to summarize📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #' @param column a data-frame schema; see `TCGA_maf_schema` for an example | |
| #' `@param` column column name in the MAF data frame to summarize |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@R/gam_utils.r` at line 329, Update the roxygen documentation for the column
parameter of stat_maf_column() to describe it as a column name, matching its use
in maf[, column] and the string example "Variant_Classification"; remove the
incorrect data-frame schema and TCGA_maf_schema reference.
1 participant
Summary
Prepare SelectSim 0.1.6 for its initial CRAN submission.
Changes
\valueand\examplessections to dataset documentationcran-comments.md.RbuildignoreR CMD check --as-cranValidation
R CMD check --as-cran: 0 errors, 0 warnings, 1 expected NOTE (New submission)Summary by CodeRabbit