Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-release tweaks #22

Open
wants to merge 46 commits into
base: dev
Choose a base branch
from
Open

Pre-release tweaks #22

wants to merge 46 commits into from

Conversation

prototaxites
Copy link
Contributor

@prototaxites prototaxites commented Mar 4, 2025

  • Fix MaxBin2 depth conversion
  • Add pipeline schematic
  • Add decompressed assembly size to assembly meta for better resource allocations
  • Use nf-schema to validate the input YAML sheet
  • Switch fastatocontig2bin to a pure awk program using the GAWK module
  • Use projectDir variable instead of deprecated baseDir
  • Sync nf-core template to add email, slackreport, github badges, and remove ro-crate
  • Minimap2 index size now auto-populates based on the length of the reference

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

Copy link

github-actions bot commented Mar 4, 2025

nf-core pipelines lint overall result: Passed ✅ ⚠️

Posted for pipeline commit ee0c20f

+| ✅ 208 tests passed       |+
#| ❔  36 tests were ignored |#
!| ❗   6 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: ro-crate-metadata.json
  • readme - README contains the placeholder zenodo.XXXXXXX. This should be replaced with the zenodo doi (after the first release).
  • pipeline_todos - TODO string in README.md: Add citation for pipeline after first release. Uncomment lines below and update Zenodo doi and badge at the top of this file.
  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!

❔ Tests ignored:

  • files_exist - File is ignored: .github/ISSUE_TEMPLATE/config.yml
  • files_exist - File is ignored: conf/igenomes_ignored.config
  • files_exist - File is ignored: conf/igenomes.config
  • files_exist - File is ignored: assets/multiqc_config.yml
  • files_exist - File is ignored: assets/methods_description_template.yml
  • files_exist - File is ignored: CODE_OF_CONDUCT.md
  • files_exist - File is ignored: assets/nf-core-metagenomeassembly_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-metagenomeassembly_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-metagenomeassembly_logo_dark.png
  • files_exist - File is ignored: .github/workflows/awstest.yml
  • files_exist - File is ignored: .github/workflows/awsfulltest.yml
  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • nextflow_config - Config variable ignored: process.cpus
  • nextflow_config - Config variable ignored: process.memory
  • nextflow_config - Config variable ignored: process.time
  • nextflow_config - Config variable ignored: validation.help.beforeText
  • nextflow_config - Config variable ignored: validation.help.afterText
  • nextflow_config - Config variable ignored: validation.summary.beforeText
  • nextflow_config - Config variable ignored: validation.summary.afterText
  • nextflow_config - Config default ignored: params.rfam_rrna_cm
  • nextflow_config - Config default ignored: params.hmm_gtdb_pfam
  • nextflow_config - Config default ignored: params.hmm_gtdb_tigrfam
  • files_unchanged - File ignored due to lint config: CODE_OF_CONDUCT.md
  • files_unchanged - File ignored due to lint config: .github/CONTRIBUTING.md
  • files_unchanged - File ignored due to lint config: .github/ISSUE_TEMPLATE/bug_report.yml
  • files_unchanged - File does not exist: .github/ISSUE_TEMPLATE/config.yml
  • files_unchanged - File ignored due to lint config: .github/workflows/branch.yml
  • files_unchanged - File ignored due to lint config: .github/workflows/linting_comment.yml
  • files_unchanged - File ignored due to lint config: .github/workflows/linting.yml
  • files_unchanged - File ignored due to lint config: assets/nf-core-metagenomeassembly_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-metagenomeassembly_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-metagenomeassembly_logo_dark.png
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/metagenomeassembly/metagenomeassembly/.github/workflows/awstest.yml
  • multiqc_config - multiqc_config

✅ Tests passed:

Run details

  • nf-core/tools version 3.2.0
  • Run at 2025-03-11 15:40:09

Copy link

@ksenia-krasheninnikova ksenia-krasheninnikova left a comment

Choose a reason for hiding this comment

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

Looks really good to me! I'd suggest the README could benefit from more detail on how to run a test and maybe the used nextflow version could be specified.
Also, a couple of really minor comments to the code, if you think it makes sense.
Once the remaining test works should be ready to merge.

@@ -75,8 +75,8 @@ process {
}

withName: CRAM_FILTER_ALIGN_BWAMEM2_FIXMATE_SORT {
cpus = 16
memory = { 1.GB * (reference.size() < 2e9 ? 80 : Math.ceil(( reference.size() / 1e+9) * 30) * task.attempt) }
cpus = { 16 * 1 }

Choose a reason for hiding this comment

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

16 * 1 == 16 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I blame the TreeVal folks... I copied this code verbatim from them!

include { METATOR_PIPELINE } from '../../../modules/local/metator/pipeline/main'
include { METATOR_PROCESS_INPUT_BAM } from '../../../modules/local/metator/process_input_bam/main'
include { BIN3C_MKMAP } from '../../../modules/local/bin3c/mkmap/main.nf'
include { BIN3C_CLUSTER } from '../../../modules/local/bin3c/cluster/main.nf'

Choose a reason for hiding this comment

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

Could '.nf' extensions be dropped for consistency in style?

Copy link
Contributor Author

@prototaxites prototaxites Mar 12, 2025

Choose a reason for hiding this comment

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

Nice catch 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants