Step 1.2: make maf optional (derived from af) in univariate and multivariate pipelines#515
Merged
Merged
Conversation
…erived from af) Step 1.2 follow-up to rss-top-loci-af. In univariate_analysis_pipeline(): - maf becomes optional (still 3rd positional arg; existing callers unaffected). - af is the single source of truth: when af is supplied, the maf_cutoff filtering MAF is derived from it (min(af, 1-af)); a supplied maf is a fallback only and, if it disagrees with the af-derived value, af wins (with a warning). - neither af nor maf supplied + maf_cutoff set -> clear error; maf is never exported. - roxygen regenerated; man page updated. Tests (test_univariate_pipeline.R): af-only -> derived; maf-only -> af NA; both-disagree -> warning; neither+maf_cutoff -> error; neither+no-cutoff -> OK; plus a behavioral test that af-derived MAF filters identically to the equivalent maf (sub-cutoff variant removed; af>0.5 ensures the minor-allele derivation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…finemapping_fits The maf->af rename in StatFunGen#513 missed multivariate_analysis_pipeline, which broke test_multivariate_pipeline.R. The multivariate maf is directionless and only used for filtering, so leave af unset (top_loci$af = NA) instead of relabeling it. Added a test. Pending follow-up: give multivariate_analysis_pipeline its own optional directional af input so it can export a real af, like univariate does.
gaow
reviewed
Jun 11, 2026
| #' only for \code{maf_cutoff} filtering; never exported. | ||
| #' @param maf Optional vector of minor allele frequencies for each variant in X, | ||
| #' used ONLY for \code{maf_cutoff} filtering and never exported. \code{af} is | ||
| #' the single source of truth: when \code{af} is supplied the filtering MAF is |
Same af handling as univariate_analysis_pipeline: maf optional, optional directional af exported as top_loci$af, filtering MAF derived from af when supplied (maf is a fallback, never exported), error when neither is given with maf_cutoff. Updates the old maf-required validation tests and adds an af-supplied test. test_multivariate_pipeline.R: 0 fail / 53 pass.
Yining97
changed the title
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.
mafis now optional in bothunivariate_analysis_pipelineandmultivariate_analysis_pipeline, withafas the directional input. Whenafis given, the MAF used bymaf_cutoffis computed from it (min(af, 1-af)); a passed-inmafis a fallback only and is never written totop_loci. Existing calls that passmafbehave the same.This also closes a CI break from #513, which renamed
postprocess_finemapping_fits'smafargument toafbut missed the multivariate caller (it still passedmafand failedtest_multivariate_pipeline.R).