aligners - #294
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change consolidates alignment workflows, adds minimap2 and Arachne support, updates report metrics and validation, relocates notebook resources, registers a Harpy Jupyter kernel, and revises alignment documentation. ChangesAlignment, reporting, and validation update
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
harpy/notebooks/validate_fastq.ipynb (1)
96-116: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the FASTQ report schema and VX tag type.
The generated TSV also contains
noVXandbxNotLast, but the Metrics section omits both. The standard validation tag isVX:i, notVX:Z.🤖 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 `@harpy/notebooks/validate_fastq.ipynb` around lines 96 - 116, Update the Metrics section in the notebook report to list all generated TSV columns, including noVX and bxNotLast, and correct every standard-format reference from VX:Z to VX:i. Keep the existing validation descriptions and footnote structure consistent with the corrected schema.
🟡 Minor comments (14)
harpy/commands/align.py-152-152 (1)
152-152: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the epilog documentation URL.
The
minimapcommand points to thebwadocumentation page.📝 Proposed fix
-@click.command(no_args_is_help = True, context_settings={"allow_interspersed_args" : False}, epilog= "Documentation: https://pdimens.github.io/harpy/workflows/align/bwa/") +@click.command(no_args_is_help = True, context_settings={"allow_interspersed_args" : False}, epilog= "Documentation: https://pdimens.github.io/harpy/workflows/align/minimap/")🤖 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 `@harpy/commands/align.py` at line 152, Update the epilog URL on the align command declaration to point to the align command’s documentation page rather than the bwa page, while preserving the existing command configuration and documentation URL format.harpy/commands/align.py-180-182 (1)
180-182: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the minimap docstring.
The help text for
minimapdescribes BWA. This text is printed byharpy align minimap --help.📝 Proposed fix
- BWA is a fast, robust, and reliable aligner that does not use barcodes when mapping. + minimap2 is a fast, versatile aligner for short and long reads that does not use barcodes when mapping. + Use `--technology` to select the minimap2 preset that matches your sequence type.🤖 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 `@harpy/commands/align.py` around lines 180 - 182, Update the minimap help text in the relevant command definition or docstring in align.py so it describes minimap rather than BWA, while preserving the existing linked-read and molecule-distance behavior details.harpy/snakefiles/align_arachne.smk-293-298 (1)
293-298: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReport names and labels say
bwain the arachne workflow.Lines 293-294 write
reports/bwa.summary.ipynb, line 298 logs tologs/reports/bwa.report.log, line 330 passes the literalminibwaas the aligner placeholder, and line 360 requestsreports/bwa.summary.ipynb. Users of the arachne workflow receive reports labelled for a different aligner. Replace these witharachne.Also applies to: 330-330, 360-360
🤖 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 `@harpy/snakefiles/align_arachne.smk` around lines 293 - 298, Rename the arachne workflow’s report artifacts and labels from bwa to arachne: update the summary notebook paths, report log path, aligner placeholder literal, and downstream summary request associated with the visible rule. Preserve the existing workflow structure and parameters while replacing each specified bwa/minibwa reference consistently.harpy/utils/check_fastq.py-11-26 (1)
11-26: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAccept valid SAM tag names.
Line 11 only accepts two uppercase letters. Valid SAM tags can use a lowercase letter or digit as the second character. For example,
q1:i:3is valid but incrementsBAD_SAM_SPEC.Proposed fix
- SAMSPEC = re.compile(r'[A-Z][A-Z]:[AifZHB]:') + SAMSPEC = re.compile(r'[A-Za-z][A-Za-z0-9]:[AifZHB]:')🤖 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 `@harpy/utils/check_fastq.py` around lines 11 - 26, Update the SAMSPEC pattern used by check_samspec to accept valid SAM tag names with an uppercase first character and either an uppercase/lowercase letter or digit as the second character, while preserving the existing type validation and BX ordering checks.harpy/notebooks/validate_bam.ipynb-109-125 (1)
109-125: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the documented BAM TSV schema.
The workflow emits
file,records,nameMismatch,noMI,noBX,noVX,bxNotLast, andbadBX. The report instead listsalignmentsand nonexistentformat, and omitsrecordsandbxNotLast.Also describe
noMIas applying to BX-tagged alignment records.check_bamskips MI checks after a missing BX tag.Also applies to: 169-174
🤖 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 `@harpy/notebooks/validate_bam.ipynb` around lines 109 - 125, The validate_bam notebook’s documented BAM TSV schema is outdated. Update the column list and supporting validation table to use file, records, nameMismatch, noMI, noBX, noVX, bxNotLast, and badBX; remove alignments and format, and add records and bxNotLast. Revise the noMI description to state that it applies to BX-tagged alignment records, matching check_bam’s behavior when BX is missing.harpy/notebooks/validate_bam.ipynb-72-74 (1)
72-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCalculate
noVXtagfrom BX-tagged records.
check_bamincrementsnoVXonly for records that already haveBX. Therefore,noVX == recordsis false whenever a file contains unbarcoded records, even if every BX-tagged record lacks VX. TheNo VX Tagmetric then underreports affected files.Proposed fix
- "noVXtag = (data['noVX'] == data['records']).sum()\n", + "bx_records = data['records'] - data['noBX']\n", + "noVXtag = ((bx_records > 0) & (data['noVX'] == bx_records)).sum()\n",🤖 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 `@harpy/notebooks/validate_bam.ipynb` around lines 72 - 74, Update the noVXtag calculation in the check_bam results to compare noVX against the number of BX-tagged records, using the existing noBX-derived value or equivalent BX count rather than total records. Leave the noMItag and noBXtag calculations unchanged.harpy/notebooks/validate_fastq.ipynb-65-70 (1)
65-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude
noVXin the review calculation.The workflow now reports
noVX, and the metrics table defines it as a failed condition. This calculation excludes it, so files with BX tags but missing VX tags do not increaseNeeds Review.Proposed fix
- " .select(pl.sum_horizontal(['badBX', 'badSamSpec', 'allmissing']).alias('attention_sum'))\n", + " .select(pl.sum_horizontal(['badBX', 'badSamSpec', 'allmissing', 'noVX']).alias('attention_sum'))\n",🤖 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 `@harpy/notebooks/validate_fastq.ipynb` around lines 65 - 70, Update the attention calculation in the notebook’s `attention` expression to include the `noVX` column in the horizontal failure sum alongside `badBX`, `badSamSpec`, and `allmissing`, so missing VX tags contribute to `Needs Review`.harpy/notebooks/align_lrstats.ipynb-411-413 (1)
411-413: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unnecessary f-string prefix.
f'alignments.fragspermol'has no replacement field. Ruff reports F541 for this expression.Proposed fix
- usermeta={'embedOptions': {'downloadFileName': f'alignments.fragspermol'}}\n", + usermeta={'embedOptions': {'downloadFileName': 'alignments.fragspermol'}}\n",🤖 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 `@harpy/notebooks/align_lrstats.ipynb` around lines 411 - 413, Remove the unnecessary f-string prefix from the downloadFileName value in the chart properties configuration, keeping the literal filename unchanged.Source: Linters/SAST tools
CHANGELOG.md-4-15 (1)
4-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the release-note text before publication.
Use
compatibilityon Line 8,accommodateon Line 14,VS Codeon Line 27, andetc.on Line 34.Also applies to: 26-35
🤖 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 `@CHANGELOG.md` around lines 4 - 15, Correct the specified CHANGELOG.md release-note spelling and capitalization: use “compatibility” in the minimap2 entry, “accommodate” in the report entry, “VS Code” on the referenced later line, and “etc.” on the final referenced line.Source: Linters/SAST tools
docs/Commands/align/arachne.md-130-145 (1)
130-145: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the documented output path in the example tree.
The paragraph says the default output directory is
Align/arachne, but the tree starts withAlign/{aligner}. Replace the placeholder withAlign/arachne, or label the tree as a generic template.🤖 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 `@docs/Commands/align/arachne.md` around lines 130 - 145, Update the output directory example tree to use the documented concrete path Align/arachne instead of Align/{aligner}, keeping the surrounding folder structure unchanged.docs/Commands/align/align.md-12-16 (1)
12-16: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the
minibwapreprint URL in the publication column.The repository link is already in the
repositorycolumn; thepreprintlink should point to the minibwa preprint, for examplehttps://arxiv.org/abs/2606.15357, instead of reusing the GitHub URL.🤖 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 `@docs/Commands/align/align.md` around lines 12 - 16, Update the publication link in the bwa row of the alignment command table to use the minibwa preprint URL, such as https://arxiv.org/abs/2606.15357, while keeping the existing GitHub repository link unchanged.docs/Commands/align/standard.md-35-48 (1)
35-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake shared aligner options generic.
This page covers minibwa, strobealign, and minimap2, but the shared section lists only BWA and strobealign. It also describes
--extra-paramsas “Additional BWA arguments,” and the minibwa section repeats the stale BWA label. Use aligner-neutral wording and list all supported aligners.Also applies to: 193-193
🤖 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 `@docs/Commands/align/standard.md` around lines 35 - 48, Update the shared Running Options documentation to list minibwa, strobealign, and minimap2 as supported aligners, and replace BWA-specific wording for --extra-params with aligner-neutral wording. Also update the minibwa section’s repeated stale BWA label to use generic aligner terminology.docs/Commands/align/standard.md-28-30 (1)
28-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the
stobecommand typo.The usage example should use
strobe, as used elsewhere in this page.Proposed fix
-harpy align bwa|stobe|minimap OPTIONS... REFERENCE INPUTS... +harpy align bwa|strobe|minimap OPTIONS... REFERENCE INPUTS...🤖 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 `@docs/Commands/align/standard.md` around lines 28 - 30, Update the usage example for the align command to replace the misspelled “stobe” subcommand with “strobe,” matching the command name used elsewhere on the page.docs/Commands/align/standard.md-147-152 (1)
147-152: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the documented aligner report names with the generated workflow output.
The workflow generates
reports/{aligner}.summary.ipynb, but this page listsreports/{aligner}.stats.ipynb. The tree/table anddocs/Commands/align/arachne.mdshould both use the workflow’s{aligner}.summary.ipynbname.🤖 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 `@docs/Commands/align/standard.md` around lines 147 - 152, Update the documented aligner report filename in the tree/table and the corresponding references in arachne.md from reports/{aligner}.stats.ipynb to reports/{aligner}.summary.ipynb, matching the workflow output.
🧹 Nitpick comments (2)
harpy/snakefiles/align.smk (1)
184-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
IPYTHONDIRpaths are hardcoded tobwa.Line 184 sets
/tmp/ipython-bwa-statsand line 230 sets/tmp/ipython-bwa.lrfor every aligner. Two concurrent Harpy runs on the same machine also share these directories. Include the aligner and a run-unique component.♻️ Proposed change
- export IPYTHONDIR=/tmp/ipython-bwa-stats + export IPYTHONDIR=$(mktemp -d)Also applies to: 230-230
🤖 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 `@harpy/snakefiles/align.smk` at line 184, Update the IPYTHONDIR assignments in the aligner statistics and LR execution blocks to derive paths from the current aligner and a run-unique component, rather than hardcoded BWA names. Ensure both assignments use the same run-specific naming scheme so concurrent Harpy runs do not share temporary directories.docs/Commands/align/arachne.md (1)
220-324: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unrelated standard-aligner sections from the Arachne page.
Lines 220-324 document strobealign and minimap2, although
CHANGELOG.mdLine 37 says their documentation was consolidated intodocs/Commands/align/standard.md. Keep those sections instandard.mdonly so the command documentation has one source of truth.🤖 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 `@docs/Commands/align/arachne.md` around lines 220 - 324, Remove the strobealign and minimap2 documentation sections, including their parameter tables, from the Arachne page. Keep the consolidated sections only in the standard alignment documentation and leave the surrounding Arachne-specific content unchanged.
🤖 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 @.github/filters.yml:
- Line 5: Update the filters in .github/filters.yml: replace notebook paths
under harpy/report/notebooks with harpy/notebooks, add align.smk,
align_arachne.smk, and align_minimap.smk to the align filter, and include
harpy/notebooks/fastp_qc.ipynb and harpy/notebooks/qc_bx_stats.ipynb in each
relevant filter.
In `@docs/Commands/align/arachne.md`:
- Around line 41-42: Update the Arachne documentation’s running-options header
and help text to describe the actual `arachne align` workflow and its options
instead of `align bwa`/`align strobe` and BWA-only behavior. Clearly label the
`minibwa map` configuration as the fallback for invalid-barcoded reads, while
preserving the output path as `Align/arachne`.
In `@harpy/commands/align.py`:
- Line 154: Update the --extra-params option in the alignment command to use a
MinimapParams validator instead of BwaParams, and add or reuse MinimapParams in
the CLI parameter definitions so minimap2-specific flags are accepted and
validation messages reference minimap2 rather than bwa-mem2.
In `@harpy/notebooks/samtools_stats.ipynb`:
- Around line 323-331: Update the sample loop that reads each
`{samplename}.regions.bed.gz` file so coverage depth is averaged by interval
length: calculate each row’s width from `Position End - Position`, use those
widths as weights for `Read Depth`, and append the resulting global weighted
mean rounded to two decimals in `depths`.
In `@harpy/report/static.py`:
- Around line 89-133: Replace the fixed tmp_nb_path construction in the
conversion flow with a unique temporary notebook path created under
nb_path.parent, preserving the original notebook’s directory for relative
resource resolution. Ensure the path is reserved before writing and remains
compatible with the existing finally cleanup via tmp_nb_path.unlink.
In `@harpy/snakefiles/align_arachne.smk`:
- Line 191: Add the linked-read declarations for lr_type and ignore_bx near the
existing top-level configuration declarations in align_arachne.smk, matching the
definitions and defaults used by align.smk. Ensure these symbols are available
before the rule expressions assign bx_mode and evaluate lr_type, without
changing those existing expressions.
- Around line 209-218: Update the combine_alignments workflow to use
coordinate-sorted inputs: route arachne/align/{sample}.arachne.bam through the
existing sort rule, then replace samtools cat in combine_alignments with
samtools merge using the sorted arachne BAM and markdup BAM. Preserve the
existing output and log targets.
- Around line 114-118: Update the arachne_align shell block to use the declared
R1/R2 inputs instead of undefined input.fastq, and correct the samtools view
invocation to specify BAM as the -O value and consume stdin explicitly. Verify
the arachne align parameter expansion uses supported -s and -d options for
RG_tag, dist, and extra.
In `@harpy/snakefiles/align_bwa.smk`:
- Around line 37-39: Update process_reference in harpy/snakefiles/align_bwa.smk
lines 37-39 by removing ".fai" and ".gzi" from bwa_idx, leaving only ".l2b" and
".mbw"; retain the conditional gzi output. Apply the corresponding change in
harpy/snakefiles/align_arachne.smk lines 42-44, removing both FASTA suffixes so
multiext contains only the arachne and minibwa index suffixes.
- Line 66: Update the minibwa command usage in the relevant alignment workflow
after checking the installed minibwa --help output: make mapping flags such as
-y, -R, and -q match the implementation’s supported interface, and document or
remove unsupported options. If the command remains only a placeholder migration
rather than a verified invocation, remove sync:True.
In `@harpy/snakefiles/align_minimap.smk`:
- Line 13: Align minimap technology naming across the workflow: in
harpy/snakefiles/align_minimap.smk lines 13 and 78, read the
“aligner-technology” parameter into tech and use tech for the -ax preset instead
of the undefined technology variable. In harpy/commands/align.py line 198,
preserve workflow.param(technology.lower(), "aligner-technology") and ensure it
matches the workflow key.
In `@harpy/snakefiles/align.smk`:
- Around line 31-57: The external-tool rules in harpy/snakefiles/align.smk lines
31-57 and the additional rules sort, mark_duplicates, standardize, sample_stats,
molecule_stats, molecule_coverage, and the report rules lack deployment
directives; add the align conda environment and matching align container to
each. In harpy/snakefiles/align_strobe.smk lines 30-44, update process_reference
with conda: "envs/align.yaml" and the same align container used by rule align.
In `@harpy/utils/standardize/standardize.go`:
- Around line 103-109: Update the logic around Header.Progs() and prev so the
new djinn program’s PP is derived from alignment-record program links or the
terminal active PP chain rather than the final slice element. Validate that the
header contains a single unambiguous chain and reject missing-program or
multiple-chain references before assigning prev.
- Around line 105-112: Update the program creation flow around sam.NewProgram
and hdr.AddProgram to choose a UID not already present in the input header,
instead of always using “djinn”. Generate a unique candidate before adding the
program, preserving the existing previous-program linkage while ensuring
AddProgram succeeds and output is still produced when “djinn” already exists.
In `@harpy/validation/fasta.py`:
- Around line 20-59: Restore FASTA parsing in the validation flow using
pysam.FastxFile, validating that the file opens and contains named sequence
records before reporting success. Update the validation logic around
self.print.validation(True) to report failure for unparseable, unnamed, or
otherwise invalid records, preserving the existing error-reporting conventions
and only marking validation successful after the checks pass.
In `@harpy/validation/vcf.py`:
- Around line 26-32: Update both pysam.bcftools.index calls in the BCF and VCF
indexing branches to pass self.threads as a string, while preserving the
existing indexing options and control flow.
---
Outside diff comments:
In `@harpy/notebooks/validate_fastq.ipynb`:
- Around line 96-116: Update the Metrics section in the notebook report to list
all generated TSV columns, including noVX and bxNotLast, and correct every
standard-format reference from VX:Z to VX:i. Keep the existing validation
descriptions and footnote structure consistent with the corrected schema.
---
Minor comments:
In `@CHANGELOG.md`:
- Around line 4-15: Correct the specified CHANGELOG.md release-note spelling and
capitalization: use “compatibility” in the minimap2 entry, “accommodate” in the
report entry, “VS Code” on the referenced later line, and “etc.” on the final
referenced line.
In `@docs/Commands/align/align.md`:
- Around line 12-16: Update the publication link in the bwa row of the alignment
command table to use the minibwa preprint URL, such as
https://arxiv.org/abs/2606.15357, while keeping the existing GitHub repository
link unchanged.
In `@docs/Commands/align/arachne.md`:
- Around line 130-145: Update the output directory example tree to use the
documented concrete path Align/arachne instead of Align/{aligner}, keeping the
surrounding folder structure unchanged.
In `@docs/Commands/align/standard.md`:
- Around line 35-48: Update the shared Running Options documentation to list
minibwa, strobealign, and minimap2 as supported aligners, and replace
BWA-specific wording for --extra-params with aligner-neutral wording. Also
update the minibwa section’s repeated stale BWA label to use generic aligner
terminology.
- Around line 28-30: Update the usage example for the align command to replace
the misspelled “stobe” subcommand with “strobe,” matching the command name used
elsewhere on the page.
- Around line 147-152: Update the documented aligner report filename in the
tree/table and the corresponding references in arachne.md from
reports/{aligner}.stats.ipynb to reports/{aligner}.summary.ipynb, matching the
workflow output.
In `@harpy/commands/align.py`:
- Line 152: Update the epilog URL on the align command declaration to point to
the align command’s documentation page rather than the bwa page, while
preserving the existing command configuration and documentation URL format.
- Around line 180-182: Update the minimap help text in the relevant command
definition or docstring in align.py so it describes minimap rather than BWA,
while preserving the existing linked-read and molecule-distance behavior
details.
In `@harpy/notebooks/align_lrstats.ipynb`:
- Around line 411-413: Remove the unnecessary f-string prefix from the
downloadFileName value in the chart properties configuration, keeping the
literal filename unchanged.
In `@harpy/notebooks/validate_bam.ipynb`:
- Around line 109-125: The validate_bam notebook’s documented BAM TSV schema is
outdated. Update the column list and supporting validation table to use file,
records, nameMismatch, noMI, noBX, noVX, bxNotLast, and badBX; remove alignments
and format, and add records and bxNotLast. Revise the noMI description to state
that it applies to BX-tagged alignment records, matching check_bam’s behavior
when BX is missing.
- Around line 72-74: Update the noVXtag calculation in the check_bam results to
compare noVX against the number of BX-tagged records, using the existing
noBX-derived value or equivalent BX count rather than total records. Leave the
noMItag and noBXtag calculations unchanged.
In `@harpy/notebooks/validate_fastq.ipynb`:
- Around line 65-70: Update the attention calculation in the notebook’s
`attention` expression to include the `noVX` column in the horizontal failure
sum alongside `badBX`, `badSamSpec`, and `allmissing`, so missing VX tags
contribute to `Needs Review`.
In `@harpy/snakefiles/align_arachne.smk`:
- Around line 293-298: Rename the arachne workflow’s report artifacts and labels
from bwa to arachne: update the summary notebook paths, report log path, aligner
placeholder literal, and downstream summary request associated with the visible
rule. Preserve the existing workflow structure and parameters while replacing
each specified bwa/minibwa reference consistently.
In `@harpy/utils/check_fastq.py`:
- Around line 11-26: Update the SAMSPEC pattern used by check_samspec to accept
valid SAM tag names with an uppercase first character and either an
uppercase/lowercase letter or digit as the second character, while preserving
the existing type validation and BX ordering checks.
---
Nitpick comments:
In `@docs/Commands/align/arachne.md`:
- Around line 220-324: Remove the strobealign and minimap2 documentation
sections, including their parameter tables, from the Arachne page. Keep the
consolidated sections only in the standard alignment documentation and leave the
surrounding Arachne-specific content unchanged.
In `@harpy/snakefiles/align.smk`:
- Line 184: Update the IPYTHONDIR assignments in the aligner statistics and LR
execution blocks to derive paths from the current aligner and a run-unique
component, rather than hardcoded BWA names. Ensure both assignments use the same
run-specific naming scheme so concurrent Harpy runs do not share temporary
directories.
🪄 Autofix
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 Plus
Run ID: 53811aaa-c2a1-4eca-ace9-a85e433aec57
⛔ Files ignored due to path filters (1)
docs/static/molecule_coverage.svgis excluded by!**/*.svg
📒 Files selected for processing (57)
.github/filters.ymlCHANGELOG.mddocs/Commands/align/align.mddocs/Commands/align/arachne.mddocs/Commands/align/bwa.mddocs/Commands/align/standard.mddocs/Commands/align/strobe.mddocs/Commands/phase/phase_snp.mddocs/Commands/sv/leviathan.mddocs/Commands/sv/naibr.mddocs/Getting_Started/Guides/wgs_data.mddocs/Getting_Started/common_options.mddocs/Getting_Started/inputformat.mddocs/Getting_Started/reports.mddocs/index.mddocs/retype.ymlharpy/commands/align.pyharpy/commands/impute.pyharpy/commands/phase.pyharpy/commands/preprocess.pyharpy/commands/report.pyharpy/common/cli_filetypes.pyharpy/common/environments.pyharpy/common/workflow.pyharpy/notebooks/__init__.pyharpy/notebooks/align_lrstats.ipynbharpy/notebooks/align_stats.ipynbharpy/notebooks/bcftools_stats.ipynbharpy/notebooks/fastp_qc.ipynbharpy/notebooks/hapcut.ipynbharpy/notebooks/impute.ipynbharpy/notebooks/preproc_stats.ipynbharpy/notebooks/qc_bx_stats.ipynbharpy/notebooks/samtools_stats.ipynbharpy/notebooks/stitch_collate.ipynbharpy/notebooks/sv.ipynbharpy/notebooks/validate_bam.ipynbharpy/notebooks/validate_fastq.ipynbharpy/report/static.pyharpy/report/utilities.pyharpy/snakefiles/align.smkharpy/snakefiles/align_arachne.smkharpy/snakefiles/align_bwa.smkharpy/snakefiles/align_minimap.smkharpy/snakefiles/align_strobe.smkharpy/snakefiles/snp_freebayes.smkharpy/snakefiles/snp_mpileup.smkharpy/snakefiles/validate_bam.smkharpy/snakefiles/validate_fastq.smkharpy/utils/check_bam.pyharpy/utils/check_fastq.pyharpy/utils/standardize/standardize.goharpy/validation/fasta.pyharpy/validation/fastq.pyharpy/validation/vcf.pyharpy/validation/xam.pypyproject.toml
💤 Files with no reviewable changes (3)
- harpy/commands/preprocess.py
- docs/Commands/align/strobe.md
- docs/Commands/align/bwa.md
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
harpy/notebooks/validate_fastq.ipynb (1)
96-116: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the FASTQ report schema and VX tag type.
The generated TSV also contains
noVXandbxNotLast, but the Metrics section omits both. The standard validation tag isVX:i, notVX:Z.🤖 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 `@harpy/notebooks/validate_fastq.ipynb` around lines 96 - 116, Update the Metrics section in the notebook report to list all generated TSV columns, including noVX and bxNotLast, and correct every standard-format reference from VX:Z to VX:i. Keep the existing validation descriptions and footnote structure consistent with the corrected schema.
🟡 Minor comments (14)
harpy/commands/align.py-152-152 (1)
152-152: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the epilog documentation URL.
The
minimapcommand points to thebwadocumentation page.📝 Proposed fix
-@click.command(no_args_is_help = True, context_settings={"allow_interspersed_args" : False}, epilog= "Documentation: https://pdimens.github.io/harpy/workflows/align/bwa/") +@click.command(no_args_is_help = True, context_settings={"allow_interspersed_args" : False}, epilog= "Documentation: https://pdimens.github.io/harpy/workflows/align/minimap/")🤖 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 `@harpy/commands/align.py` at line 152, Update the epilog URL on the align command declaration to point to the align command’s documentation page rather than the bwa page, while preserving the existing command configuration and documentation URL format.harpy/commands/align.py-180-182 (1)
180-182: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the minimap docstring.
The help text for
minimapdescribes BWA. This text is printed byharpy align minimap --help.📝 Proposed fix
- BWA is a fast, robust, and reliable aligner that does not use barcodes when mapping. + minimap2 is a fast, versatile aligner for short and long reads that does not use barcodes when mapping. + Use `--technology` to select the minimap2 preset that matches your sequence type.🤖 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 `@harpy/commands/align.py` around lines 180 - 182, Update the minimap help text in the relevant command definition or docstring in align.py so it describes minimap rather than BWA, while preserving the existing linked-read and molecule-distance behavior details.harpy/snakefiles/align_arachne.smk-293-298 (1)
293-298: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReport names and labels say
bwain the arachne workflow.Lines 293-294 write
reports/bwa.summary.ipynb, line 298 logs tologs/reports/bwa.report.log, line 330 passes the literalminibwaas the aligner placeholder, and line 360 requestsreports/bwa.summary.ipynb. Users of the arachne workflow receive reports labelled for a different aligner. Replace these witharachne.Also applies to: 330-330, 360-360
🤖 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 `@harpy/snakefiles/align_arachne.smk` around lines 293 - 298, Rename the arachne workflow’s report artifacts and labels from bwa to arachne: update the summary notebook paths, report log path, aligner placeholder literal, and downstream summary request associated with the visible rule. Preserve the existing workflow structure and parameters while replacing each specified bwa/minibwa reference consistently.harpy/utils/check_fastq.py-11-26 (1)
11-26: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAccept valid SAM tag names.
Line 11 only accepts two uppercase letters. Valid SAM tags can use a lowercase letter or digit as the second character. For example,
q1:i:3is valid but incrementsBAD_SAM_SPEC.Proposed fix
- SAMSPEC = re.compile(r'[A-Z][A-Z]:[AifZHB]:') + SAMSPEC = re.compile(r'[A-Za-z][A-Za-z0-9]:[AifZHB]:')🤖 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 `@harpy/utils/check_fastq.py` around lines 11 - 26, Update the SAMSPEC pattern used by check_samspec to accept valid SAM tag names with an uppercase first character and either an uppercase/lowercase letter or digit as the second character, while preserving the existing type validation and BX ordering checks.harpy/notebooks/validate_bam.ipynb-109-125 (1)
109-125: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the documented BAM TSV schema.
The workflow emits
file,records,nameMismatch,noMI,noBX,noVX,bxNotLast, andbadBX. The report instead listsalignmentsand nonexistentformat, and omitsrecordsandbxNotLast.Also describe
noMIas applying to BX-tagged alignment records.check_bamskips MI checks after a missing BX tag.Also applies to: 169-174
🤖 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 `@harpy/notebooks/validate_bam.ipynb` around lines 109 - 125, The validate_bam notebook’s documented BAM TSV schema is outdated. Update the column list and supporting validation table to use file, records, nameMismatch, noMI, noBX, noVX, bxNotLast, and badBX; remove alignments and format, and add records and bxNotLast. Revise the noMI description to state that it applies to BX-tagged alignment records, matching check_bam’s behavior when BX is missing.harpy/notebooks/validate_bam.ipynb-72-74 (1)
72-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCalculate
noVXtagfrom BX-tagged records.
check_bamincrementsnoVXonly for records that already haveBX. Therefore,noVX == recordsis false whenever a file contains unbarcoded records, even if every BX-tagged record lacks VX. TheNo VX Tagmetric then underreports affected files.Proposed fix
- "noVXtag = (data['noVX'] == data['records']).sum()\n", + "bx_records = data['records'] - data['noBX']\n", + "noVXtag = ((bx_records > 0) & (data['noVX'] == bx_records)).sum()\n",🤖 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 `@harpy/notebooks/validate_bam.ipynb` around lines 72 - 74, Update the noVXtag calculation in the check_bam results to compare noVX against the number of BX-tagged records, using the existing noBX-derived value or equivalent BX count rather than total records. Leave the noMItag and noBXtag calculations unchanged.harpy/notebooks/validate_fastq.ipynb-65-70 (1)
65-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude
noVXin the review calculation.The workflow now reports
noVX, and the metrics table defines it as a failed condition. This calculation excludes it, so files with BX tags but missing VX tags do not increaseNeeds Review.Proposed fix
- " .select(pl.sum_horizontal(['badBX', 'badSamSpec', 'allmissing']).alias('attention_sum'))\n", + " .select(pl.sum_horizontal(['badBX', 'badSamSpec', 'allmissing', 'noVX']).alias('attention_sum'))\n",🤖 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 `@harpy/notebooks/validate_fastq.ipynb` around lines 65 - 70, Update the attention calculation in the notebook’s `attention` expression to include the `noVX` column in the horizontal failure sum alongside `badBX`, `badSamSpec`, and `allmissing`, so missing VX tags contribute to `Needs Review`.harpy/notebooks/align_lrstats.ipynb-411-413 (1)
411-413: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unnecessary f-string prefix.
f'alignments.fragspermol'has no replacement field. Ruff reports F541 for this expression.Proposed fix
- usermeta={'embedOptions': {'downloadFileName': f'alignments.fragspermol'}}\n", + usermeta={'embedOptions': {'downloadFileName': 'alignments.fragspermol'}}\n",🤖 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 `@harpy/notebooks/align_lrstats.ipynb` around lines 411 - 413, Remove the unnecessary f-string prefix from the downloadFileName value in the chart properties configuration, keeping the literal filename unchanged.Source: Linters/SAST tools
CHANGELOG.md-4-15 (1)
4-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the release-note text before publication.
Use
compatibilityon Line 8,accommodateon Line 14,VS Codeon Line 27, andetc.on Line 34.Also applies to: 26-35
🤖 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 `@CHANGELOG.md` around lines 4 - 15, Correct the specified CHANGELOG.md release-note spelling and capitalization: use “compatibility” in the minimap2 entry, “accommodate” in the report entry, “VS Code” on the referenced later line, and “etc.” on the final referenced line.Source: Linters/SAST tools
docs/Commands/align/arachne.md-130-145 (1)
130-145: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse the documented output path in the example tree.
The paragraph says the default output directory is
Align/arachne, but the tree starts withAlign/{aligner}. Replace the placeholder withAlign/arachne, or label the tree as a generic template.🤖 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 `@docs/Commands/align/arachne.md` around lines 130 - 145, Update the output directory example tree to use the documented concrete path Align/arachne instead of Align/{aligner}, keeping the surrounding folder structure unchanged.docs/Commands/align/align.md-12-16 (1)
12-16: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the
minibwapreprint URL in the publication column.The repository link is already in the
repositorycolumn; thepreprintlink should point to the minibwa preprint, for examplehttps://arxiv.org/abs/2606.15357, instead of reusing the GitHub URL.🤖 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 `@docs/Commands/align/align.md` around lines 12 - 16, Update the publication link in the bwa row of the alignment command table to use the minibwa preprint URL, such as https://arxiv.org/abs/2606.15357, while keeping the existing GitHub repository link unchanged.docs/Commands/align/standard.md-35-48 (1)
35-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake shared aligner options generic.
This page covers minibwa, strobealign, and minimap2, but the shared section lists only BWA and strobealign. It also describes
--extra-paramsas “Additional BWA arguments,” and the minibwa section repeats the stale BWA label. Use aligner-neutral wording and list all supported aligners.Also applies to: 193-193
🤖 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 `@docs/Commands/align/standard.md` around lines 35 - 48, Update the shared Running Options documentation to list minibwa, strobealign, and minimap2 as supported aligners, and replace BWA-specific wording for --extra-params with aligner-neutral wording. Also update the minibwa section’s repeated stale BWA label to use generic aligner terminology.docs/Commands/align/standard.md-28-30 (1)
28-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the
stobecommand typo.The usage example should use
strobe, as used elsewhere in this page.Proposed fix
-harpy align bwa|stobe|minimap OPTIONS... REFERENCE INPUTS... +harpy align bwa|strobe|minimap OPTIONS... REFERENCE INPUTS...🤖 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 `@docs/Commands/align/standard.md` around lines 28 - 30, Update the usage example for the align command to replace the misspelled “stobe” subcommand with “strobe,” matching the command name used elsewhere on the page.docs/Commands/align/standard.md-147-152 (1)
147-152: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the documented aligner report names with the generated workflow output.
The workflow generates
reports/{aligner}.summary.ipynb, but this page listsreports/{aligner}.stats.ipynb. The tree/table anddocs/Commands/align/arachne.mdshould both use the workflow’s{aligner}.summary.ipynbname.🤖 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 `@docs/Commands/align/standard.md` around lines 147 - 152, Update the documented aligner report filename in the tree/table and the corresponding references in arachne.md from reports/{aligner}.stats.ipynb to reports/{aligner}.summary.ipynb, matching the workflow output.
🧹 Nitpick comments (2)
harpy/snakefiles/align.smk (1)
184-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
IPYTHONDIRpaths are hardcoded tobwa.Line 184 sets
/tmp/ipython-bwa-statsand line 230 sets/tmp/ipython-bwa.lrfor every aligner. Two concurrent Harpy runs on the same machine also share these directories. Include the aligner and a run-unique component.♻️ Proposed change
- export IPYTHONDIR=/tmp/ipython-bwa-stats + export IPYTHONDIR=$(mktemp -d)Also applies to: 230-230
🤖 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 `@harpy/snakefiles/align.smk` at line 184, Update the IPYTHONDIR assignments in the aligner statistics and LR execution blocks to derive paths from the current aligner and a run-unique component, rather than hardcoded BWA names. Ensure both assignments use the same run-specific naming scheme so concurrent Harpy runs do not share temporary directories.docs/Commands/align/arachne.md (1)
220-324: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unrelated standard-aligner sections from the Arachne page.
Lines 220-324 document strobealign and minimap2, although
CHANGELOG.mdLine 37 says their documentation was consolidated intodocs/Commands/align/standard.md. Keep those sections instandard.mdonly so the command documentation has one source of truth.🤖 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 `@docs/Commands/align/arachne.md` around lines 220 - 324, Remove the strobealign and minimap2 documentation sections, including their parameter tables, from the Arachne page. Keep the consolidated sections only in the standard alignment documentation and leave the surrounding Arachne-specific content unchanged.
🤖 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 @.github/filters.yml:
- Line 5: Update the filters in .github/filters.yml: replace notebook paths
under harpy/report/notebooks with harpy/notebooks, add align.smk,
align_arachne.smk, and align_minimap.smk to the align filter, and include
harpy/notebooks/fastp_qc.ipynb and harpy/notebooks/qc_bx_stats.ipynb in each
relevant filter.
In `@docs/Commands/align/arachne.md`:
- Around line 41-42: Update the Arachne documentation’s running-options header
and help text to describe the actual `arachne align` workflow and its options
instead of `align bwa`/`align strobe` and BWA-only behavior. Clearly label the
`minibwa map` configuration as the fallback for invalid-barcoded reads, while
preserving the output path as `Align/arachne`.
In `@harpy/commands/align.py`:
- Line 154: Update the --extra-params option in the alignment command to use a
MinimapParams validator instead of BwaParams, and add or reuse MinimapParams in
the CLI parameter definitions so minimap2-specific flags are accepted and
validation messages reference minimap2 rather than bwa-mem2.
In `@harpy/notebooks/samtools_stats.ipynb`:
- Around line 323-331: Update the sample loop that reads each
`{samplename}.regions.bed.gz` file so coverage depth is averaged by interval
length: calculate each row’s width from `Position End - Position`, use those
widths as weights for `Read Depth`, and append the resulting global weighted
mean rounded to two decimals in `depths`.
In `@harpy/report/static.py`:
- Around line 89-133: Replace the fixed tmp_nb_path construction in the
conversion flow with a unique temporary notebook path created under
nb_path.parent, preserving the original notebook’s directory for relative
resource resolution. Ensure the path is reserved before writing and remains
compatible with the existing finally cleanup via tmp_nb_path.unlink.
In `@harpy/snakefiles/align_arachne.smk`:
- Line 191: Add the linked-read declarations for lr_type and ignore_bx near the
existing top-level configuration declarations in align_arachne.smk, matching the
definitions and defaults used by align.smk. Ensure these symbols are available
before the rule expressions assign bx_mode and evaluate lr_type, without
changing those existing expressions.
- Around line 209-218: Update the combine_alignments workflow to use
coordinate-sorted inputs: route arachne/align/{sample}.arachne.bam through the
existing sort rule, then replace samtools cat in combine_alignments with
samtools merge using the sorted arachne BAM and markdup BAM. Preserve the
existing output and log targets.
- Around line 114-118: Update the arachne_align shell block to use the declared
R1/R2 inputs instead of undefined input.fastq, and correct the samtools view
invocation to specify BAM as the -O value and consume stdin explicitly. Verify
the arachne align parameter expansion uses supported -s and -d options for
RG_tag, dist, and extra.
In `@harpy/snakefiles/align_bwa.smk`:
- Around line 37-39: Update process_reference in harpy/snakefiles/align_bwa.smk
lines 37-39 by removing ".fai" and ".gzi" from bwa_idx, leaving only ".l2b" and
".mbw"; retain the conditional gzi output. Apply the corresponding change in
harpy/snakefiles/align_arachne.smk lines 42-44, removing both FASTA suffixes so
multiext contains only the arachne and minibwa index suffixes.
- Line 66: Update the minibwa command usage in the relevant alignment workflow
after checking the installed minibwa --help output: make mapping flags such as
-y, -R, and -q match the implementation’s supported interface, and document or
remove unsupported options. If the command remains only a placeholder migration
rather than a verified invocation, remove sync:True.
In `@harpy/snakefiles/align_minimap.smk`:
- Line 13: Align minimap technology naming across the workflow: in
harpy/snakefiles/align_minimap.smk lines 13 and 78, read the
“aligner-technology” parameter into tech and use tech for the -ax preset instead
of the undefined technology variable. In harpy/commands/align.py line 198,
preserve workflow.param(technology.lower(), "aligner-technology") and ensure it
matches the workflow key.
In `@harpy/snakefiles/align.smk`:
- Around line 31-57: The external-tool rules in harpy/snakefiles/align.smk lines
31-57 and the additional rules sort, mark_duplicates, standardize, sample_stats,
molecule_stats, molecule_coverage, and the report rules lack deployment
directives; add the align conda environment and matching align container to
each. In harpy/snakefiles/align_strobe.smk lines 30-44, update process_reference
with conda: "envs/align.yaml" and the same align container used by rule align.
In `@harpy/utils/standardize/standardize.go`:
- Around line 103-109: Update the logic around Header.Progs() and prev so the
new djinn program’s PP is derived from alignment-record program links or the
terminal active PP chain rather than the final slice element. Validate that the
header contains a single unambiguous chain and reject missing-program or
multiple-chain references before assigning prev.
- Around line 105-112: Update the program creation flow around sam.NewProgram
and hdr.AddProgram to choose a UID not already present in the input header,
instead of always using “djinn”. Generate a unique candidate before adding the
program, preserving the existing previous-program linkage while ensuring
AddProgram succeeds and output is still produced when “djinn” already exists.
In `@harpy/validation/fasta.py`:
- Around line 20-59: Restore FASTA parsing in the validation flow using
pysam.FastxFile, validating that the file opens and contains named sequence
records before reporting success. Update the validation logic around
self.print.validation(True) to report failure for unparseable, unnamed, or
otherwise invalid records, preserving the existing error-reporting conventions
and only marking validation successful after the checks pass.
In `@harpy/validation/vcf.py`:
- Around line 26-32: Update both pysam.bcftools.index calls in the BCF and VCF
indexing branches to pass self.threads as a string, while preserving the
existing indexing options and control flow.
---
Outside diff comments:
In `@harpy/notebooks/validate_fastq.ipynb`:
- Around line 96-116: Update the Metrics section in the notebook report to list
all generated TSV columns, including noVX and bxNotLast, and correct every
standard-format reference from VX:Z to VX:i. Keep the existing validation
descriptions and footnote structure consistent with the corrected schema.
---
Minor comments:
In `@CHANGELOG.md`:
- Around line 4-15: Correct the specified CHANGELOG.md release-note spelling and
capitalization: use “compatibility” in the minimap2 entry, “accommodate” in the
report entry, “VS Code” on the referenced later line, and “etc.” on the final
referenced line.
In `@docs/Commands/align/align.md`:
- Around line 12-16: Update the publication link in the bwa row of the alignment
command table to use the minibwa preprint URL, such as
https://arxiv.org/abs/2606.15357, while keeping the existing GitHub repository
link unchanged.
In `@docs/Commands/align/arachne.md`:
- Around line 130-145: Update the output directory example tree to use the
documented concrete path Align/arachne instead of Align/{aligner}, keeping the
surrounding folder structure unchanged.
In `@docs/Commands/align/standard.md`:
- Around line 35-48: Update the shared Running Options documentation to list
minibwa, strobealign, and minimap2 as supported aligners, and replace
BWA-specific wording for --extra-params with aligner-neutral wording. Also
update the minibwa section’s repeated stale BWA label to use generic aligner
terminology.
- Around line 28-30: Update the usage example for the align command to replace
the misspelled “stobe” subcommand with “strobe,” matching the command name used
elsewhere on the page.
- Around line 147-152: Update the documented aligner report filename in the
tree/table and the corresponding references in arachne.md from
reports/{aligner}.stats.ipynb to reports/{aligner}.summary.ipynb, matching the
workflow output.
In `@harpy/commands/align.py`:
- Line 152: Update the epilog URL on the align command declaration to point to
the align command’s documentation page rather than the bwa page, while
preserving the existing command configuration and documentation URL format.
- Around line 180-182: Update the minimap help text in the relevant command
definition or docstring in align.py so it describes minimap rather than BWA,
while preserving the existing linked-read and molecule-distance behavior
details.
In `@harpy/notebooks/align_lrstats.ipynb`:
- Around line 411-413: Remove the unnecessary f-string prefix from the
downloadFileName value in the chart properties configuration, keeping the
literal filename unchanged.
In `@harpy/notebooks/validate_bam.ipynb`:
- Around line 109-125: The validate_bam notebook’s documented BAM TSV schema is
outdated. Update the column list and supporting validation table to use file,
records, nameMismatch, noMI, noBX, noVX, bxNotLast, and badBX; remove alignments
and format, and add records and bxNotLast. Revise the noMI description to state
that it applies to BX-tagged alignment records, matching check_bam’s behavior
when BX is missing.
- Around line 72-74: Update the noVXtag calculation in the check_bam results to
compare noVX against the number of BX-tagged records, using the existing
noBX-derived value or equivalent BX count rather than total records. Leave the
noMItag and noBXtag calculations unchanged.
In `@harpy/notebooks/validate_fastq.ipynb`:
- Around line 65-70: Update the attention calculation in the notebook’s
`attention` expression to include the `noVX` column in the horizontal failure
sum alongside `badBX`, `badSamSpec`, and `allmissing`, so missing VX tags
contribute to `Needs Review`.
In `@harpy/snakefiles/align_arachne.smk`:
- Around line 293-298: Rename the arachne workflow’s report artifacts and labels
from bwa to arachne: update the summary notebook paths, report log path, aligner
placeholder literal, and downstream summary request associated with the visible
rule. Preserve the existing workflow structure and parameters while replacing
each specified bwa/minibwa reference consistently.
In `@harpy/utils/check_fastq.py`:
- Around line 11-26: Update the SAMSPEC pattern used by check_samspec to accept
valid SAM tag names with an uppercase first character and either an
uppercase/lowercase letter or digit as the second character, while preserving
the existing type validation and BX ordering checks.
---
Nitpick comments:
In `@docs/Commands/align/arachne.md`:
- Around line 220-324: Remove the strobealign and minimap2 documentation
sections, including their parameter tables, from the Arachne page. Keep the
consolidated sections only in the standard alignment documentation and leave the
surrounding Arachne-specific content unchanged.
In `@harpy/snakefiles/align.smk`:
- Line 184: Update the IPYTHONDIR assignments in the aligner statistics and LR
execution blocks to derive paths from the current aligner and a run-unique
component, rather than hardcoded BWA names. Ensure both assignments use the same
run-specific naming scheme so concurrent Harpy runs do not share temporary
directories.
🪄 Autofix
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 Plus
Run ID: 53811aaa-c2a1-4eca-ace9-a85e433aec57
⛔ Files ignored due to path filters (1)
docs/static/molecule_coverage.svgis excluded by!**/*.svg
📒 Files selected for processing (57)
.github/filters.ymlCHANGELOG.mddocs/Commands/align/align.mddocs/Commands/align/arachne.mddocs/Commands/align/bwa.mddocs/Commands/align/standard.mddocs/Commands/align/strobe.mddocs/Commands/phase/phase_snp.mddocs/Commands/sv/leviathan.mddocs/Commands/sv/naibr.mddocs/Getting_Started/Guides/wgs_data.mddocs/Getting_Started/common_options.mddocs/Getting_Started/inputformat.mddocs/Getting_Started/reports.mddocs/index.mddocs/retype.ymlharpy/commands/align.pyharpy/commands/impute.pyharpy/commands/phase.pyharpy/commands/preprocess.pyharpy/commands/report.pyharpy/common/cli_filetypes.pyharpy/common/environments.pyharpy/common/workflow.pyharpy/notebooks/__init__.pyharpy/notebooks/align_lrstats.ipynbharpy/notebooks/align_stats.ipynbharpy/notebooks/bcftools_stats.ipynbharpy/notebooks/fastp_qc.ipynbharpy/notebooks/hapcut.ipynbharpy/notebooks/impute.ipynbharpy/notebooks/preproc_stats.ipynbharpy/notebooks/qc_bx_stats.ipynbharpy/notebooks/samtools_stats.ipynbharpy/notebooks/stitch_collate.ipynbharpy/notebooks/sv.ipynbharpy/notebooks/validate_bam.ipynbharpy/notebooks/validate_fastq.ipynbharpy/report/static.pyharpy/report/utilities.pyharpy/snakefiles/align.smkharpy/snakefiles/align_arachne.smkharpy/snakefiles/align_bwa.smkharpy/snakefiles/align_minimap.smkharpy/snakefiles/align_strobe.smkharpy/snakefiles/snp_freebayes.smkharpy/snakefiles/snp_mpileup.smkharpy/snakefiles/validate_bam.smkharpy/snakefiles/validate_fastq.smkharpy/utils/check_bam.pyharpy/utils/check_fastq.pyharpy/utils/standardize/standardize.goharpy/validation/fasta.pyharpy/validation/fastq.pyharpy/validation/vcf.pyharpy/validation/xam.pypyproject.toml
💤 Files with no reviewable changes (3)
- harpy/commands/preprocess.py
- docs/Commands/align/strobe.md
- docs/Commands/align/bwa.md
🛑 Comments failed to post (2)
.github/filters.yml (1)
5-5: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail fd -t f -e ipynb harpy/notebooks rg -n 'harpy/(report/)?notebooks|harpy/snakefiles/align(\.smk|_(arachne|minimap)\.smk)' .github/filters.ymlRepository: pdimens/harpy
Length of output: 517
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== .github/filters.yml ==" cat -n .github/filters.yml echo echo "== notebook files ==" find . -type d -name notebooks 2>/dev/null | sort | sed -n '1,120p' find . -path '*/notebooks/*.ipynb' -type f 2>/dev/null | sort | sed -n '1,200p' echo echo "== snakefiles align ==" find . -path '*snakefiles*align*.smk' -type f 2>/dev/null | sort | sed -n '1,200p' find . -path '*snakefiles*' -type f 2>/dev/null | sort | sed -n '1,200p' echo echo "== filter references in .github/filters.yml ==" rg -n 'notebooks|snakefiles|report|align(\.smk|_(arachne|minimap)\.smk)|harpy/snakefiles|harpy/report/notebooks' .github/filters.yml || trueRepository: pdimens/harpy
Length of output: 7379
Update the filters to cover the moved notebooks and missing align Snakefiles.
The report pattern now only covers
harpy/report/*.py, so changes to notebooks underharpy/notebooksand newly addedalign.smk,align_arachne.smk, oralign_minimap.smkdo not trigger the intended workflow checks. Replaceharpy/report/notebooks/...entries withharpy/notebooks/..., add the missing align Snakefile entries to thealignfilter, and addharpy/notebooks/fastp_qc.ipynbandharpy/notebooks/qc_bx_stats.ipynbto the relevant filters.🤖 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 @.github/filters.yml at line 5, Update the filters in .github/filters.yml: replace notebook paths under harpy/report/notebooks with harpy/notebooks, add align.smk, align_arachne.smk, and align_minimap.smk to the align filter, and include harpy/notebooks/fastp_qc.ipynb and harpy/notebooks/qc_bx_stats.ipynb in each relevant filter.harpy/report/static.py (1)
89-133: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use a unique temporary notebook path.
Line 91 always uses
.<stem>-tmp.ipynb. If that file already exists, line 101 overwrites it and line 132 deletes it. Concurrent conversions of the same notebook have the same failure mode. Create a unique temporary file innb_path.parentso relative notebook resources continue to resolve correctly.Proposed fix
- tmp_nb_path = nb_path.with_name(f".{nb_name}-tmp.ipynb") + with tempfile.NamedTemporaryFile( + dir=nb_path.parent, + prefix=f".{nb_name}-", + suffix=".ipynb", + delete=False, + ) as tmp_file: + tmp_nb_path = Path(tmp_file.name)📝 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.# Transformed notebook copy lives NEXT TO the original (not in /tmp). # It's cleaned up in `finally` and the original file is never modified. with tempfile.NamedTemporaryFile( dir=nb_path.parent, prefix=f".{nb_name}-", suffix=".ipynb", delete=False, ) as tmp_file: tmp_nb_path = Path(tmp_file.name) workdir = Path(tempfile.mkdtemp(prefix="nb2html_")) if self.static: intermediate_html = workdir / f"{nb_name}.html" else: intermediate_html = out_path try: nb_json = json.loads(nb_path.read_text(encoding="utf-8")) self.render_frontmatter_cell(nb_json) tmp_nb_path.write_text(json.dumps(nb_json), encoding="utf-8") nbconvert_cmd = [ "jupyter", "nbconvert", str(tmp_nb_path), "--to", "html", "--embed-images", f"--log-level={self.nbc_log}", "--TagRemovePreprocessor.enabled=True", "--TagRemovePreprocessor.remove_cell_tags=remove-cell", "--TagRemovePreprocessor.remove_input_tags=remove-input", "--TagRemovePreprocessor.remove_all_outputs_tags=remove-output", "--output", intermediate_html.name, "--output-dir", str(intermediate_html.parent), ] self.run(nbconvert_cmd) if not intermediate_html.is_file(): self.hp.error( "Missing nbconvert output", f"The expected nbconvert output was not found at {intermediate_html}" ) # nbconvert's HTML has no code-split/dynamically-imported JS, so # monolith can reliably inline everything it references (CDN # vega-embed, MathJax, fonts, etc.) into one working file. if self.static: monolith_cmd = ["monolith", str(intermediate_html), "-o", str(out_path)] if self.quiet: monolith_cmd.append('-q') self.run(monolith_cmd) finally: tmp_nb_path.unlink(missing_ok=True) shutil.rmtree(workdir, ignore_errors=True)🧰 Tools
🪛 ast-grep (0.45.0)
[info] 100-100: use jsonify instead of json.dumps for JSON output
Context: json.dumps(nb_json)
Note: [CWE-116] Improper Encoding or Escaping of Output.(use-jsonify)
🤖 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 `@harpy/report/static.py` around lines 89 - 133, Replace the fixed tmp_nb_path construction in the conversion flow with a unique temporary notebook path created under nb_path.parent, preserving the original notebook’s directory for relative resource resolution. Ensure the path is reserved before writing and remains compatible with the existing finally cleanup via tmp_nb_path.unlink.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
docs/Getting_Started/Resources/software.md (1)
21-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReconcile the BWA software entry with the alignment environment.
harpy/common/environments.pystill declaresbioconda::bwa, whiledocs/Getting_Started/Resources/software.mdlists onlyminibwaandminimap2. If BWA remains an active tool, add it back to the software catalog; otherwise, remove the stale BWA dependency and workflow references.🤖 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 `@docs/Getting_Started/Resources/software.md` at line 21, Reconcile the BWA entry across the software catalog and alignment environment: verify whether the declared bioconda::bwa dependency is still used by active workflows. If it is active, add BWA to the software table in software.md; otherwise remove the stale dependency and its workflow references while preserving minibwa and minimap2 entries.
🤖 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 @.github/filters.yml:
- Around line 49-60: Update the align filter anchored by &align to include
harpy/snakefiles/align_arachne.smk alongside the other alignment workflow
Snakefiles, ensuring changes to the Arachne workflow select the align CI job.
In `@harpy/common/cli_params.py`:
- Around line 84-87: Update the validation error messages in the
option-processing logic to consistently reference minimap2 instead of minibwa,
including the documentation link text and the available-options label in both
self.fail calls.
- Around line 71-85: Update MinimapParams.convert’s option validation to accept
the --secondary=<value> form before applying the exact valid_options allowlist,
while continuing to reject Harpy-owned -a, -x, and -ax options through
harpy_options.
In `@harpy/common/summaries.py`:
- Line 41: Remove the unused f-string prefix from the samtools sort string in
the duplicate-summary construction, leaving the string content and existing
`duplicates` behavior unchanged.
- Around line 35-36: Update the summary headings in the alignment-report
construction, including the `align` text near the `minibwa` command and the
`align_minimap` heading, to identify the tools as `minibwa` and `minimap2`
respectively instead of BWA or the strobe workflow. Keep the existing command
details and formatting unchanged.
In `@harpy/notebooks/validate_bam.ipynb`:
- Around line 75-76: Update the notebook’s tag-count calculations so noMItag
compares data['noMI'] against bx_records rather than all data['records'],
matching the existing noVXtag logic and counting BX-bearing records without MI
tags.
In `@harpy/validation/vcf.py`:
- Line 93: Update the VariantFile invocation in check_phase to pass self.threads
directly as the threads argument, removing the string conversion while
preserving the surrounding VCF validation flow.
---
Nitpick comments:
In `@docs/Getting_Started/Resources/software.md`:
- Line 21: Reconcile the BWA entry across the software catalog and alignment
environment: verify whether the declared bioconda::bwa dependency is still used
by active workflows. If it is active, add BWA to the software table in
software.md; otherwise remove the stale dependency and its workflow references
while preserving minibwa and minimap2 entries.
🪄 Autofix
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 Plus
Run ID: 25eb0a2e-addb-441f-874e-86333ce24718
📒 Files selected for processing (22)
.github/filters.yml.github/workflows/tests.ymlCHANGELOG.mddocs/Commands/align/arachne.mddocs/Commands/align/standard.mddocs/Getting_Started/Resources/software.mddocs/Getting_Started/reports.mdharpy/commands/align.pyharpy/common/cli_params.pyharpy/common/summaries.pyharpy/notebooks/align_lrstats.ipynbharpy/notebooks/align_stats.ipynbharpy/notebooks/samtools_stats.ipynbharpy/notebooks/validate_bam.ipynbharpy/notebooks/validate_fastq.ipynbharpy/snakefiles/align.smkharpy/snakefiles/align_arachne.smkharpy/snakefiles/align_bwa.smkharpy/snakefiles/align_minimap.smkharpy/utils/check_fastq.pyharpy/validation/fasta.pyharpy/validation/vcf.py
🚧 Files skipped from review as they are similar to previous changes (9)
- harpy/notebooks/align_stats.ipynb
- harpy/notebooks/validate_fastq.ipynb
- harpy/snakefiles/align_minimap.smk
- docs/Commands/align/standard.md
- CHANGELOG.md
- harpy/snakefiles/align.smk
- harpy/snakefiles/align_arachne.smk
- harpy/snakefiles/align_bwa.smk
- harpy/notebooks/align_lrstats.ipynb
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@pyproject.toml`:
- Line 69: Add ipykernel to the run requirements in resources/meta.yaml,
matching the declared version constraint in pyproject.toml, so
resources/build.sh can execute the ipykernel installation during conda
activation.
In `@resources/register-kernel.sh`:
- Around line 1-4: Update resources/register-kernel.sh lines 1-4 and
resources/build.sh lines 21-24 to remove output redirection from the ipykernel
install commands and explicitly fail when CONDA_PREFIX is missing or unset,
ensuring registration errors are visible and stop the activation flow.
🪄 Autofix
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 Plus
Run ID: 83d2504e-9fe8-44e8-aebc-32f610af4161
📒 Files selected for processing (20)
harpy/commands/diagnose.pyharpy/common/cli_params.pyharpy/common/summaries.pyharpy/notebooks/validate_bam.ipynbharpy/snakefiles/align.smkharpy/snakefiles/align_arachne.smkharpy/snakefiles/impute.smkharpy/snakefiles/phase_snp.smkharpy/snakefiles/preprocess_gih.smkharpy/snakefiles/qc.smkharpy/snakefiles/snp_freebayes.smkharpy/snakefiles/snp_mpileup.smkharpy/snakefiles/sv_leviathan.smkharpy/snakefiles/sv_naibr.smkharpy/snakefiles/validate_bam.smkharpy/snakefiles/validate_fastq.smkharpy/validation/vcf.pypyproject.tomlresources/build.shresources/register-kernel.sh
🚧 Files skipped from review as they are similar to previous changes (7)
- harpy/notebooks/validate_bam.ipynb
- harpy/common/cli_params.py
- harpy/snakefiles/snp_freebayes.smk
- harpy/common/summaries.py
- harpy/snakefiles/align.smk
- harpy/validation/vcf.py
- harpy/snakefiles/align_arachne.smk
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/Getting_Started/Resources/software.md (1)
20-20: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the software inventory aligned with the alignment environment.
The
bwaentry was removed, butharpy/common/environments.pystill provisionsbioconda::bwain thealignenvironment. Restore the documented entry, or remove the package from the environment ifbwais no longer required.🤖 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 `@docs/Getting_Started/Resources/software.md` at line 20, Align the software inventory with the align environment by restoring the missing bwa entry in the documented resources, or remove bioconda::bwa from the align environment configuration if it is no longer required; keep both sources consistent.
🧹 Nitpick comments (1)
harpy/validation/fasta.py (1)
24-26: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNarrow the exception handler to FASTA input errors.
except Exceptionconverts unrelated runtime failures frompysam.FastxFilesetup or cleanup into"bad FASTA file"and hides the original defect. Catch the documented open or parse exceptions forpysam==0.23.0; let unexpected exceptions propagate.🤖 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 `@harpy/validation/fasta.py` around lines 24 - 26, In the FASTA validation flow, replace the broad exception handler around pysam.FastxFile setup/parsing with handlers for only the documented pysam==0.23.0 open and parse exception types. Keep the existing validation and error reporting for those input failures, while allowing unrelated setup or cleanup exceptions to propagate.Source: Linters/SAST tools
🤖 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/commands/report.py`:
- Line 145: Update the installation message in the Group call to replace “It can
be installed with using one of these methods:” with “It can be installed using
one of these methods:”.
- Around line 135-145: Update the dependency check in static() to probe `jupyter
nbconvert --version` rather than only checking `check_tool('jupyter')`, while
preserving the existing missing-dependency guidance and conversion flow.
---
Outside diff comments:
In `@docs/Getting_Started/Resources/software.md`:
- Line 20: Align the software inventory with the align environment by restoring
the missing bwa entry in the documented resources, or remove bioconda::bwa from
the align environment configuration if it is no longer required; keep both
sources consistent.
---
Nitpick comments:
In `@harpy/validation/fasta.py`:
- Around line 24-26: In the FASTA validation flow, replace the broad exception
handler around pysam.FastxFile setup/parsing with handlers for only the
documented pysam==0.23.0 open and parse exception types. Keep the existing
validation and error reporting for those input failures, while allowing
unrelated setup or cleanup exceptions to propagate.
🪄 Autofix
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 Plus
Run ID: 13ddf501-f942-4947-a900-fa9a07bb01fe
📒 Files selected for processing (10)
docs/Getting_Started/Resources/software.mdharpy/commands/report.pyharpy/common/summaries.pyharpy/notebooks/validate_bam.ipynbharpy/report/utilities.pyharpy/snakefiles/align_minimap.smkharpy/validation/fasta.pyresources/build.shresources/meta.yamlresources/register-kernel.sh
🚧 Files skipped from review as they are similar to previous changes (4)
- resources/build.sh
- harpy/snakefiles/align_minimap.smk
- harpy/notebooks/validate_bam.ipynb
- harpy/common/summaries.py
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
1 participant
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.
closes #292
Summary by CodeRabbit
New Features
Improvements
Documentation