OncoKB MAF bug and contributing guidelines by sigven · Pull Request #301 · sigven/pcgr

sigven · 2026-07-20T09:11:28Z

No description provided.

qodo-code-review · 2026-07-20T09:16:46Z

PR Summary by Qodo

Fix OncoKB MAF bind_rows type mismatch; add CONTRIBUTING; bump to 2.3.1.9001

🐞 Bug fix 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Prevent OncoKB MAF merge failures by forcing consistent character column types.
• Add contributing guidelines and surface them in the README and Code of Conduct.
• Bump dev version to 2.3.1.9001 and refresh conda recipes/lockfiles.

Diagram

graph TD
  maf_hgvsp[("HGVSp MAF")] --> read_hgvsp["read_tsv(.default=col_character)"] --> bind["dplyr::bind_rows()"] --> ann["all_variant_annotations"] --> downstream["Downstream interpretation"]
  maf_hgvsg[("HGVSg MAF")] --> read_hgvsg["read_tsv(.default=col_character)"] --> bind

High-Level Assessment

The following are alternative approaches to this PR:

1. Explicit per-column schema for OncoKB MAFs

➕ Preserves intended numeric/logical types for downstream computations
➕ Documents the expected MAF schema and can catch upstream format drift early
➖ Higher maintenance as OncoKB adds/changes columns
➖ More verbose and easier to get wrong than a blanket default

2. Normalize types after read (coerce selected columns before bind_rows)

➕ Keeps type inference for most columns while fixing known offenders
➕ Limits type coercion to the minimal surface area
➖ Requires knowing which columns can diverge across runs/files
➖ Still brittle if a different sparse column triggers type guessing later

Recommendation: The chosen approach (force all columns to character on ingest) is pragmatic and robust for MAF row-binding, especially given the two files are read independently and can sparsely populate different pass-through columns. Unless downstream logic relies on numeric types from these MAFs (which can be re-cast later in a single place), this is the lowest-risk, lowest-maintenance fix.

Files changed (19) +184 / -30

Bug fix (1) +14 / -2

oncokb.RFix OncoKB MAF bind_rows failures by forcing character columns +14/-2

Fix OncoKB MAF bind_rows failures by forcing character columns

• For both HGVSp and HGVSg OncoKB MAF inputs, readr::read_tsv now uses col_types with .default=col_character() to prevent inconsistent per-file type guessing. This avoids dplyr::bind_rows() errors when combining the two independently-read tables.

pcgrr/R/oncokb.R

Documentation (4) +144 / -2

CODE_OF_CONDUCT.mdExpand Code of Conduct reporting contact information +1/-1

Expand Code of Conduct reporting contact information
• Adds an additional maintainer email address for reporting incidents.
CODE_OF_CONDUCT.md

CONTRIBUTING.mdAdd project contributing guidelines +133/-0

Add project contributing guidelines

• Introduces a contribution guide covering code of conduct, bug reports, workflow targeting the dev branch, conventions, and validation expectations.

CONTRIBUTING.md

README.mdAdd Contributing section with links to guidelines and CoC +9/-0

Add Contributing section with links to guidelines and CoC
• Adds a README section pointing contributors to CONTRIBUTING.md and the Code of Conduct, and clarifies that PRs should target dev.
README.md

installation.RmdUpdate installation vignette to dev version 2.3.1.9001 +1/-1

Update installation vignette to dev version 2.3.1.9001
• Updates the PCGR_VERSION environment variable used in the vignette examples.
pcgrr/vignettes/installation.Rmd

Other (14) +26 / -26

.bumpversion.tomlBump tracked version to 2.3.1.9001 +1/-1

Bump tracked version to 2.3.1.9001
• Updates bumpversion configuration to the new dev version string.
.bumpversion.toml

build_conda_recipes.yamlUpdate conda build workflow VERSION to 2.3.1.9001 +1/-1

Update conda build workflow VERSION to 2.3.1.9001
• Ensures the conda recipe build workflow uses the bumped dev version.
.github/workflows/build_conda_recipes.yaml

pcgr-linux-64.lockRegenerate linux-64 conda lock for dev version +4/-4

Regenerate linux-64 conda lock for dev version
• Updates the lockfile input hash and refreshes locked artifacts, including the pcgr package reference under the dev label and minor dependency bumps.
conda/env/lock/pcgr-linux-64.lock

pcgr-osx-64.lockRegenerate osx-64 conda lock for dev version +4/-4

Regenerate osx-64 conda lock for dev version
• Updates the lockfile input hash and refreshes locked artifacts, including the pcgr package reference under the dev label and minor dependency bumps.
conda/env/lock/pcgr-osx-64.lock

pcgrr-linux-64.lockRegenerate pcgrr linux-64 conda lock for dev label packages +4/-4

Regenerate pcgrr linux-64 conda lock for dev label packages
• Updates the lockfile input hash and switches r-pcgrr and r-cpsr to dev-label builds; also refreshes some locked system libraries.
conda/env/lock/pcgrr-linux-64.lock

pcgrr-osx-64.lockRegenerate pcgrr osx-64 conda lock for dev label packages +4/-4

Regenerate pcgrr osx-64 conda lock for dev label packages
• Updates the lockfile input hash and switches r-pcgrr and r-cpsr to dev-label builds; also refreshes some locked system libraries.
conda/env/lock/pcgrr-osx-64.lock

pcgr.yamlPin pcgr env YAML to 2.3.1.9001 +1/-1

Pin pcgr env YAML to 2.3.1.9001
• Bumps the pcgr version pin used by the conda environment definition.
conda/env/yaml/pcgr.yaml

pcgrr.yamlPin r-pcgrr env YAML to 2.3.1.9001 +1/-1

Pin r-pcgrr env YAML to 2.3.1.9001
• Bumps the r-pcgrr version pin used by the conda environment definition.
conda/env/yaml/pcgrr.yaml

pkgdown.yamlPin pkgdown env YAML to r-pcgrr 2.3.1.9001 +1/-1

Pin pkgdown env YAML to r-pcgrr 2.3.1.9001
• Bumps the r-pcgrr pin used for generating pkgdown documentation.
conda/env/yaml/pkgdown.yaml

recipe.yamlBump pcgr conda recipe version to 2.3.1.9001 +1/-1

Bump pcgr conda recipe version to 2.3.1.9001
• Updates the conda recipe context version for pcgr.
conda/recipe/pcgr/recipe.yaml

recipe.yamlBump r-pcgrr conda recipe version to 2.3.1.9001 +1/-1

Bump r-pcgrr conda recipe version to 2.3.1.9001
• Updates the conda recipe context version for r-pcgrr.
conda/recipe/pcgrr/recipe.yaml

_version.pyBump Python package version constant +1/-1

Bump Python package version constant
• Updates pcgr __version__ to 2.3.1.9001.
pcgr/_version.py

DESCRIPTIONBump R package version to 2.3.1.9001 +1/-1

Bump R package version to 2.3.1.9001
• Updates the pcgrr DESCRIPTION Version field for the dev cycle.
pcgrr/DESCRIPTION

pyproject.tomlBump pyproject version to 2.3.1.9001 +1/-1

Bump pyproject version to 2.3.1.9001
• Updates the project version metadata for the Python package build.
pyproject.toml

Copilot

Pull request overview

This PR updates PCGR’s development metadata/versioning and adds contributor documentation, alongside a fix intended to make OncoKB MAF ingestion more robust when combining annotations from independently parsed files.

Changes:

Add formal contribution guidelines and link them from the README; update Code of Conduct contact details.
Bump the project’s development version to 2.3.1.9001 across Python, R, conda recipes/envs, CI, and bumpversion config.
Adjust OncoKB MAF reading to avoid dplyr::bind_rows() failures due to inconsistent per-file type inference.

Reviewed changes

Copilot reviewed 15 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
README.md	Adds a “Contributing” section pointing to CONTRIBUTING.md and Code of Conduct.
pyproject.toml	Bumps Python package version to `2.3.1.9001`.
pcgrr/vignettes/installation.Rmd	Updates vignette version variable to match dev version.
pcgrr/R/oncokb.R	Changes MAF TSV parsing to enforce consistent column typing across files.
pcgrr/DESCRIPTION	Bumps R package version to `2.3.1.9001`.
pcgr/_version.py	Bumps Python runtime version constant.
CONTRIBUTING.md	Adds contribution, reporting, workflow, and validation guidance.
conda/recipe/pcgrr/recipe.yaml	Bumps conda recipe version for r-pcgrr.
conda/recipe/pcgr/recipe.yaml	Bumps conda recipe version for pcgr.
conda/env/yaml/pkgdown.yaml	Updates pinned r-pcgrr version for docs env.
conda/env/yaml/pcgrr.yaml	Updates pinned r-pcgrr version for pcgrr env.
conda/env/yaml/pcgr.yaml	Updates pinned pcgr version for pcgr env.
conda/env/lock/pcgrr-osx-64.lock	Updates lockfile hash and resolved packages for dev label artifacts.
conda/env/lock/pcgrr-linux-64.lock	Updates lockfile hash and resolved packages for dev label artifacts.
conda/env/lock/pcgr-osx-64.lock	Updates lockfile hash and resolved packages for dev label artifacts.
conda/env/lock/pcgr-linux-64.lock	Updates lockfile hash and resolved packages for dev label artifacts.
CODE_OF_CONDUCT.md	Adds an additional contact email for reporting incidents.
.github/workflows/build_conda_recipes.yaml	Updates workflow VERSION env var to the dev version.
.bumpversion.toml	Updates bumpversion current_version to the dev version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The previous fix forced all OncoKB MAF columns to character to avoid bind_rows() type mismatches, but that also broke the `== TRUE` filters on ANNOTATED/GENE_IN_ONCOKB/VARIANT_IN_ONCOKB: OncoKB's MafAnnotator writes Python-style "True"/"False", and "True" == TRUE is FALSE in R, so every row would have been silently dropped. Keep those three columns logical explicitly, force everything else to character.

qodo-code-review · 2026-07-20T09:28:45Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~OncoKB MAF filter drops all~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

process_oncokb_maf() now forces all TSV columns to character, which can leave boolean flags like
GENE_IN_ONCOKB as strings (e.g., True), so the later filter .data$GENE_IN_ONCOKB == TRUE
becomes case/representation sensitive and can exclude all rows. This can silently skip OncoKB API
annotation for SNV/InDel MAF inputs (both HGVSp and HGVSg paths).

Code

pcgrr/R/oncokb.R[R1133-1145]

+      # Force character typing for all columns rather than relying on
+      # readr's per-file column type guessing: the HGVSp and HGVSg MAF
+      # files are read independently, so a sparsely populated pass-through
+      # column (e.g. MUTATION_EFFECT_CITATIONS) can be guessed as a
+      # different type (character/double/logical) in each file, which
+      # later breaks dplyr::bind_rows() when rows from both files are
+      # combined into all_variant_annotations.
      maf_hgvsp <- readr::read_tsv(
-        maf_file_hgvsp, show_col_types = FALSE)
+        maf_file_hgvsp, show_col_types = FALSE,
+        col_types = readr::cols(.default = readr::col_character()))

      # Filter for variants in OncoKB genes regardless of ANNOTATED flag:
      # the MAF annotator batch endpoint misses variants covered only by gene-level

Evidence

The PR changes read_tsv() to force all columns to character, but immediately after still filters
using == TRUE on GENE_IN_ONCOKB, which depends on the column being logical or being an exact
string match after coercion. Upstream, the pipeline filters GENE_IN_ONCOKB by lowercasing string
values and writes the file back with pandas, which commonly produces True/False representations
that will not match "TRUE" under the R filter once the column is forced to character.

pcgrr/R/oncokb.R[1131-1153]
pcgrr/R/oncokb.R[1286-1301]
pcgr/biomarker.py[499-514]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`process_oncokb_maf()` reads the OncoKB-annotated MAFs with `col_types = cols(.default = col_character())`, but then filters with logical comparisons like `.data$GENE_IN_ONCOKB == TRUE`. If upstream outputs contain `True`/`False` (common when written by pandas), forcing character preserves that exact casing/representation and the logical comparison may not match, dropping all rows and preventing downstream OncoKB API calls.

### Issue Context
Upstream code filters OncoKB outputs by comparing `GENE_IN_ONCOKB` as a lowercased string to `'true'` and writes back to disk (pandas `to_csv`), which commonly yields `True`/`False` values in the file. The R side should therefore either:
- explicitly parse these columns as logical during `read_tsv`, or
- normalize string booleans before filtering.

### Fix Focus Areas
- pcgrr/R/oncokb.R[1131-1153]
- pcgrr/R/oncokb.R[1286-1301]

### Suggested fix approach
Option A (preferred): keep `.default = col_character()` but override the boolean columns used in filters:
- `col_types = cols(GENE_IN_ONCOKB = col_logical(), VARIANT_IN_ONCOKB = col_logical(), ANNOTATED = col_logical(), .default = col_character())`

Option B: normalize after reading:
- `mutate(GENE_IN_ONCOKB = tolower(GENE_IN_ONCOKB) == 'true')` (and same for other boolean flags), then filter using that logical column.

Ensure both HGVSp and HGVSg readers apply the same logic so later `bind_rows()` remains safe.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

sigven and others added 5 commits July 12, 2026 23:55

force character type of OncoKB MAF columns

df0278e

Bump version: 2.3.1 → 2.3.1.9001

182a5e7

[bot] Updating conda-lock files (v2.3.1.9001)

383595c

updated contributing guidelines

621b35d

Merge branch 'dev' of github.com:sigven/pcgr into dev

77f5e6c

sigven requested a review from Copilot July 20, 2026 09:14

Copilot started reviewing on behalf of sigven July 20, 2026 09:14 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Comment thread pcgrr/R/oncokb.R Outdated

Comment thread pcgrr/R/oncokb.R Outdated

qodo-code-review Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread pcgrr/R/oncokb.R

qodo-code-review Bot reviewed Jul 20, 2026

View reviewed changes

Comment thread pcgrr/R/oncokb.R

sigven merged commit a2fae95 into main Jul 20, 2026
4 checks passed

sigven mentioned this pull request Aug 10, 2026

Biomarker matching fixes #303

Merged

Navigation Menu

OncoKB MAF bug and contributing guidelines - #301

OncoKB MAF bug and contributing guidelines#301
sigven merged 6 commits into
mainfrom
dev

sigven commented Jul 20, 2026

Uh oh!

qodo-code-review Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sigven commented Jul 20, 2026

Uh oh!

qodo-code-review Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

qodo-code-review Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

qodo-code-review Bot commented Jul 20, 2026 •

edited

Loading

qodo-code-review Bot commented Jul 20, 2026 •

edited

Loading