Repository navigation
new module: longphase/modcall - #13388
Conversation
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
vagkaratzas
left a comment
There was a problem hiding this comment.
Thanks for the module, the structure and test data usage look good. Blocking points:
- The container directive was copied from
longphase/phase(longphase 2.0.1, plus samtools) and does not matchenvironment.yml(longphase 2.1.1, htslib) or thecontainers:block inmeta.yml; the snapshots were generated with the wrong version. Please runnf-core modules containers create longphase/modcall, then regenerate the snapshots. htslibis inenvironment.ymlandbgzipis called, but there is noversions_htslibemit; the longphase version is also the full usage text rather than a bare2.1.1.meta.ymlstill has the template description, noargs_id, and incomplete ontologies and keywords. Test names should follow the<organism> - <inputs> - <outputs>pattern.
While you are here (not blocking):
modules/nf-core/longphase/modcall/main.nf:49-52: trailing blank lines can be removed.
Not run: G1 and G2. The PR's CI had not run the lint or nf-test jobs when reviewed (only the size label and mergeability checks were present), and the local runs were stopped before finishing, so please make sure nf-core modules lint longphase/modcall and the nf-test CI jobs pass. G3 (nextflow lint) passed.
Diff 8.21/10 · Overall 8.21/10
nf-core-reviewer 0.3.0 / vagkaratzas-review (rubric v2) · claude-sonnet-5-5
| container "${ workflow.containerEngine in ['singularity', 'apptainer'] && !task.ext.singularity_pull_docker_container ? | ||
| 'https://community-cr-prod.seqera.io/docker/registry/v2/blobs/sha256/83/83fce1d397cf71705cc096fc0e0e52f7013bdd471ef68ee53ae765688e5c439c/data': | ||
| 'community.wave.seqera.io/library/longphase_samtools:8c61296cae7a5fc0' }" |
There was a problem hiding this comment.
The container directive does not belong to this module's environment: it is the longphase_samtools image copied from longphase/phase (the snapshot reports longphase 2.0.1 while environment.yml pins 2.1.1), it does not match the containers: block in meta.yml, and the conda lock file contains no htslib. Please run nf-core modules containers create longphase/modcall after finalising environment.yml, so the directive, meta.yml and lock files all describe the same build, then regenerate the snapshots.
| @@ -0,0 +1,120 @@ | |||
| name: "longphase_modcall" | |||
| description: write your description here | |||
There was a problem hiding this comment.
Placeholder description left over from the template. Please describe the module in one short line, e.g. Call allele-specific base modifications from a long-read BAM with MM/ML tags.
|
CI only runs automatically for nf-core members. Please ask for a GitHub invitation in the nf-core Slack #github-invitations channel; once you've joined, CI will run on your next push. Comment added by Claude |
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
….bai. No valid EDAM tag for .fai. Linted module.
|
Thank you for the comments. I have updated the tests according to the template, and I now use the correct container for the updated versions. The tests are also regenerated. I found an EDAM for .bai format but no EDAM format was found for .fai so I left that blank. I will ask for the GitHub invitation on Slack. Thanks again, I am still new to this! |
vagkaratzas
left a comment
There was a problem hiding this comment.
- Blocking: under the default prefix the output can take the name of the SNV input (
${meta.id}.vcf.gz, as clair3 writes it), which makesbgzipfail or truncates the upstream file. A_modcallsuffix on the default prefix fixes it. - Small: the longphase version in the snapshot is
LongPhase Ver 2.1.1instead of2.1.1(my earliersedwas wrong), and the BAI ontology needs its# BAIcomment.
Thanks for working through the last round. The container, the htslib version, meta.yml and the test structure all look good now, and CI passes on all three profiles.
While you are here (not blocking):
modules/nf-core/longphase/modcall/tests/main.nf.test:37-38,68-72: indentation is off and there are extra blank lines;main.nf:9-10has a double blank line.
Diff 9.11/10 · Overall 9.11/10
nf-core-reviewer 0.5.0 / vagkaratzas-review (rubric v4) · claude-opus-5-5
Comment added by Claude Opus 5.5
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
|
Alright, I have added the changes you suggested and I think the indentation is fixed, and the snapshots were regenerated. Strangely enough, I had to link the module on my local computer because the prettier hook failed to install on the cluster. xD. |
vagkaratzas
left a comment
There was a problem hiding this comment.
- Blocking: the stub default prefix does not match the script's (
_modcallsuffix missing), so stub snapshots carry the wrong file name. After fixing, regenerate the two stub snapshots. - The previous blockers are resolved: the prefix suffix avoids clobbering the SNV input, versions are bare (
2.1.1,1.24), the BAI ontology comment is there, and CI passes (lint and nf-test on the current head). Everymodcallfile option (-b,-r,--snp-file,-o) is wired.
While you are here (not blocking):
modules/nf-core/longphase/modcall/main.nf:9-10: double blank line, still there from last round.modules/nf-core/longphase/modcall/tests/main.nf.test:28,38-39,67-72: indentation of the closing]/}is off and there are extra blank lines, still there from last round.modules/nf-core/longphase/modcall/meta.yml:$args2goes tobgzipbuthtslibhas no entry undertools:; add one withargs_id: "$args2".
Diff 9.64/10 · Overall 9.64/10
nf-core-reviewer 0.5.0 / vagkaratzas-review (rubric v4) · claude-sonnet-5-5
Comment added by Claude Sonnet 5.5
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
vagkaratzas
left a comment
There was a problem hiding this comment.
- The stub default prefix now matches the script (
${meta.id}_modcall), and the stub snapshots carry the right file name. Lint, nf-test andnextflow lintall pass on the current head.
Approval should be on its way soon.
While you are here (not blocking):
modules/nf-core/longphase/modcall/meta.yml:$args2goes tobgzip, buthtslibhas no entry undertools:; add one withargs_id: "$args2".modules/nf-core/longphase/modcall/main.nf:8-9: double blank line.modules/nf-core/longphase/modcall/tests/main.nf.test:33-35,59-63,23: closing]/}indentation is off and there are extra blank lines.
Diff 10.00/10 · Overall 10.00/10
nf-core-reviewer 0.5.0 / vagkaratzas-review (rubric v4) · claude-sonnet-5-5
Comment added by Claude Code
|
Ok, you can attempt to update this PR with main and then rerun the CI. However I would personally like to see the |
|
and run the latest version of |
|
I am using the same container/version used in longphase/phase so I wlll keep that updated. |
* Add longphase/modcall module * New module: longphase/modcall * Update modules/nf-core/longphase/modcall/main.nf Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Regenerated test with new conda to match update. Added EDAM tags for .bai. No valid EDAM tag for .fai. Linted module. * Regenerated snapshot on cluster to run with minimum resource requirements * Update modules/nf-core/longphase/modcall/main.nf Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/main.nf Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * regenerated snapshot * fixed meta.yml with nf-core modules lint --fix * lint again on main machine to prevent hooks from failing? * Update modules/nf-core/longphase/modcall/main.nf Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * regenerated snap for the final time (I hope >:D) --------- Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> Co-authored-by: awgymer <24782660+awgymer@users.noreply.github.com>
* bump bcftools/htslib to 1.25 in env files * Update subworkflow tags * bcftools: bump containers * bump htslib to 1.25 * bump bcftools module snaps * bump htslib module snaps * Add license header text to all modules and subworkflow main.nf files (nf-core#13448) * Add header to all subworkflows * Add two subworkflow stragglers * Add all license header to all module main.nf files * Add 33 moduel stragglers * [skip ci] remove trailing space end of first line of license header --------- Co-authored-by: Matthias De Smet <11850640+matthdsm@users.noreply.github.com> * fix: remove nextflow shebang lines which break lint (nf-core#13472) * fix: remove nextflow shebang lines which break lint * test: regenerate tinc stub snapshot for nft-utils 1.x key order * fix typo (nf-core#13470) * Use environment variables for test data base paths (nf-core#13474) ## Feat Add optional env variables for test data base paths so devs can set it to a local path without running the risk of committing changes that shouldn't be there. ## Reason I did exactly that and want to add the env variable to my `.bashrc`. No more timeouts from github and faster tests! * new module: longphase/modcall (nf-core#13388) * Add longphase/modcall module * New module: longphase/modcall * Update modules/nf-core/longphase/modcall/main.nf Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/tests/main.nf.test Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Regenerated test with new conda to match update. Added EDAM tags for .bai. No valid EDAM tag for .fai. Linted module. * Regenerated snapshot on cluster to run with minimum resource requirements * Update modules/nf-core/longphase/modcall/main.nf Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/meta.yml Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * Update modules/nf-core/longphase/modcall/main.nf Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * regenerated snapshot * fixed meta.yml with nf-core modules lint --fix * lint again on main machine to prevent hooks from failing? * Update modules/nf-core/longphase/modcall/main.nf Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> * regenerated snap for the final time (I hope >:D) --------- Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> Co-authored-by: awgymer <24782660+awgymer@users.noreply.github.com> * align NXF_VER and NFT_VER across CI (nf-core#13477) * Update pre-commit hooks (nf-core#13479) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Add raxmlng/evaluate: optimise branch lengths and model on a fixed tree (nf-core#13475) * Add raxmlng/evaluate: optimise branch lengths and model on a fixed tree Runs raxml-ng --evaluate on an alignment, a tree and a model: the topology is kept, only branch lengths and model parameters are optimised. The seed defaults to 42 as in raxmlng/search. The environment, container and conda lock files are the same as for raxmlng/search. Generated by Claude Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup * Add EDAM ontologies to the raxmlng/evaluate meta.yml Alignment inputs are FASTA or PHYLIP, trees are Newick, and the model and log files are textual. Generated by Claude Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> * Add raxmlng/moose: RAxML-NG model selection without a tree search (nf-core#13476) * Add raxmlng/moose: RAxML-NG model selection without a tree search Runs raxml-ng --moose on an alignment and returns the selected model (*.raxml.moose.bestModel) and the log. The data type is an input, the seed defaults to 42 as in raxmlng/search. The environment, container and conda lock files are the same as for raxmlng/search. Generated by Claude Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup * Add EDAM ontologies to the raxmlng/moose meta.yml Alignment inputs are FASTA or PHYLIP, trees are Newick, and the model and log files are textual. Generated by Claude Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014eSqkuYZXfNiWy87y3srup --------- Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> * update plotvcfstats snap * fix everything ngscheckmate * fix linting * final touches * fix ngscheckmate/vafncm * drop ca-certs --------- Co-authored-by: James A. Fellows Yates <jfy133@gmail.com> Co-authored-by: awgymer <24782660+awgymer@users.noreply.github.com> Co-authored-by: bounlu <bounlu@gmail.com> Co-authored-by: Dylan House <dylan.house@tuta.com> Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Daniel Lundin <erik.rikard.daniel@gmail.com> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Adds a new module for longphase/modcall. This tool is commonly used for calling allele-specific modifications in long-read Nanopore and PacBio reads, given the aligned BAM file preserved the MM,ML tags with these modifications. The user can also optionally input a variant call file containing SNV information.
Inputs for this module is a meta-map in the format [meta, bam, bai, snv], and the reference [meta2,fasta,fai]
Output is a compressed modcalled generated vcf which can then be provided as an input to longphase/phase for phasing.
Tests cover if all inputs are provided, and another if the snv file is omitted.
PR checklist
topic: versions- See version_topicslabelnf-core modules test <MODULE> --profile dockernf-core modules test <MODULE> --profile singularitynf-core modules test <MODULE> --profile conda