Skip to content

filter alignments if using --vcf-samples#288

Merged
pdimens merged 6 commits into
mainfrom
typer
Jul 13, 2026
Merged

filter alignments if using --vcf-samples#288
pdimens merged 6 commits into
mainfrom
typer

Conversation

@pdimens

@pdimens pdimens commented Jul 13, 2026

Copy link
Copy Markdown
Owner

fixes #285
fixes #287

Summary by CodeRabbit

  • Bug Fixes
    • Phasing and imputation workflows now consistently restrict alignment inputs to the samples selected from the VCF when sample filtering is enabled.
    • Improved sample matching helps prevent unrelated alignment data from being included in processing.
  • New Features
    • Added support for retrieving sample names directly from VCF files, improving sample selection and validation.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The --vcf-samples option now filters alignment inputs for impute and both phase subcommands. VCF.samplelist() exposes header sample names, and XAM.filter() retains matching BAM/SAM files. An unused local variable was removed.

Changes

VCF sample filtering

Layer / File(s) Summary
Sample discovery and alignment filtering
harpy/validation/vcf.py, harpy/validation/xam.py
Adds VCF header sample extraction and in-place filtering of alignment files by sample basename; removes an unused local variable.
Command workflow filtering
harpy/commands/impute.py, harpy/commands/phase.py
Impute and the bam and snp phase subcommands filter alignments to the matched VCF samples when vcf_samples is enabled.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ImputeOrPhaseCommand
  participant VCF
  participant XAM
  participant Workflow
  ImputeOrPhaseCommand->>VCF: Match configured samples
  VCF-->>ImputeOrPhaseCommand: Return selected sample names
  ImputeOrPhaseCommand->>XAM: Filter alignment files
  XAM-->>Workflow: Provide filtered alignments
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR also changes phase.py to filter alignments in phasing commands, which is outside the linked impute-only issue. Move the phase.py update to a separate PR or link an issue for it, and keep this change focused on impute plus its helpers.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement the requested --vcf-samples restriction for the STITCH run in impute, matching the linked issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: filtering alignments when --vcf-samples is used.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch typer

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@harpy/validation/xam.py`:
- Around line 157-164: Update the alignment filtering flow in filter() so
self.lr_type and self.count are recomputed from the retained self.files after
keeplist filtering. Ensure both phase workflows use this refreshed metadata when
deciding linked-read processing, including when all linked-read BAMs are
excluded.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 781be5fc-108b-4a51-92fe-2d18a202f5aa

📥 Commits

Reviewing files that changed from the base of the PR and between 620f46f and a7c312e.

📒 Files selected for processing (4)
  • harpy/commands/impute.py
  • harpy/commands/phase.py
  • harpy/validation/vcf.py
  • harpy/validation/xam.py

Comment thread harpy/validation/xam.py
@pdimens pdimens merged commit c8fee93 into main Jul 13, 2026
15 checks passed
@pdimens pdimens deleted the typer branch July 13, 2026 19:31
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.

--vcf-samples in harpy impute doesn't restrict the STITCH sample panel rm second align pie chart

1 participant