Skip to content

Conversation

@kal26
Copy link
Contributor

@kal26 kal26 commented Oct 30, 2025

Summary

Sets defaults for qc_method, pip_cutoff_to_skip_ind and pip_cutoff_to_skip_sumstat when multiple contexts are used, and regions when only region_name_col is passed or nothing is passed.

Changes

  • Setting a real default for qc_method via match.arg("rss_qc"). (This was mentioned in the docs but not implemented)
  • Allowing default PIP skip cutoffs to be used with multiple contexts/sumstats. Accept scalar or per-context vector; otherwise error with clear message. If nothing passed, default to 0 for all contexts.
  • Assigning phenotype names from region_name_col even without extract_region_name, with a fallback to region_1, region_2, ....

Rationale

Removes the need to always pass verbose arguments for common cases, sets defaults to match was is stated in the docs.
Region names are needed for colocboost_analysis_pipeline to run without errors if multiple regions per phenotype file are analyzed. Previously, names were only assigned if BOTH region_name_col and extract_region_name were passed. This meant in practice you could not pass phenotype files with multiple phenotypes within the region without also passing both these parameters.

Testing

  • Checked xQTL-only and xQTL+GWAS run with omitted qc_method
  • Checked multiple-context and multi-GWAS cases run with omitted pip_cutoff. Checked that error is throw if pip_cutoff does not match length of phenotypes/susmstats.
  • Checked region names are set as expected if only region_name_col is passed, or if no region_name_col is passes and that colocboost_analysis_pipeline runs without the naming error.

@gaow
Copy link
Contributor

gaow commented Nov 7, 2025

Thanks @kal26 from me eyeballing on the codes I don't see obvious concerns of backwards compatibility. The only issue is that we don't have unit tests so it's hard to tell exactly if it breaks anything but @danielnachun i think its fine to merge. Please take a look and do it on your end.

There is one merge conflict possibly due to formatting of white space --- please address that. To avoid white space reformatting, we recommend unifying the R code format before PR,

https://github.com/StatFunGen/pecotmr?tab=readme-ov-file#developers-notes

see the line format_r_code.sh

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.

2 participants