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

Release 2.3.0 #137

Merged
merged 90 commits into from
Jun 12, 2024
Merged

Release 2.3.0 #137

merged 90 commits into from
Jun 12, 2024

Conversation

Daniel-VM
Copy link
Contributor

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
  • If necessary, also make a PR on the nf-core/bacass branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --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).

PR description

This PR is for release 2.3.0

Major changes are:

  • Implementation of Kmerfinder subworkflow for read quality control, purity assessment and sample grouping based on reference genome estimation.
  • Enhancement of QUAST allowig to run a general QUAST (without reference genomes) and a reference genome-based quast analysis when kmerfinder is invoked.
  • Replacement of MULTIQC module by CUSTOM_MULTIQC module allowing custom reports based on assembly type (short, long, hybrid).
  • Update of the nf-core template v2.14.1

Daniel-VM and others added 4 commits May 23, 2024 16:51
Implementation of KmerFinder subworkflow Custom Quast, and Custom MultiQC Reports
Copy link

github-actions bot commented May 24, 2024

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit dafb9f4

+| ✅ 195 tests passed       |+
#| ❔   4 tests were ignored |#
!| ❗   1 tests had warnings |!

❗ Test warnings:

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-06-12 14:30:27

@Daniel-VM Daniel-VM marked this pull request as ready for review May 24, 2024 10:50
Copy link

@FriederikeHanssen FriederikeHanssen left a comment

Choose a reason for hiding this comment

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

Looks super good. If you could add the missing licenses that would be great. People often just leave a note at the top with MIT or whatever they choose. i can point you to examples if you like

bin/csv_to_yaml.py Outdated Show resolved Hide resolved
Comment on lines +42 to +53
## Collect additional files to be included in the report
if [ -d extra/ ]; then
cp extra/* multiqc_data/
fi

## Create multiqc custom data
multiqc_to_custom_csv.py --assembly_type $params.assembly_type

## Avoid the custom Multiqc table when the kmerfinder process is not invoked.
if grep ">skip_kmerfinder<" workflow_summary_mqc.yaml; then
rm *_assembly_metrics_mqc.csv
fi

Choose a reason for hiding this comment

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

I was wondering about this when looking at the changelog: so the choice to go with a custom version is to run mqc twice to get modified data and plot it? Just in case you didn't know about this, there is also the nf-core diff command that allows you to modify an nf-core/module, but keep getting the updates, like new versions etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First, it generates multiqc_data from the input tools to parse tool logs. Second, it collects extra data (such as from KmerFinder) and formats it to be parsed by the second MultiQC run, creating custom tables for inclusion in the custom MultiQC report. Finally, it runs a second MultiQC with all the generated data and emits a custom report.

It is inspired by and sourced from nf-core/viralrecon custom multiqc

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 tried to make a patch, but MultiQC version 1.21 does not properly parse this "file set up", and it might require a deeper refactor. (it worked with version v1.19).

Choose a reason for hiding this comment

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

Interesting, might be worth bringing up with the MutliQC devs to see what the issue is and fix it soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, that would be great. Is it OK if I add this to a new git issue?

Choose a reason for hiding this comment

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

yes absolutely, you can also ask in the #multiqc channel on slack

subworkflows/local/kmerfinder_subworkflow.nf Show resolved Hide resolved
workflows/bacass.nf Show resolved Hide resolved
@Daniel-VM
Copy link
Contributor Author

Thank you @FriederikeHanssen :). Reviewer suggestions are in #138

implementation of reviewer suggestions before v2.3.0 release
@Daniel-VM Daniel-VM requested a review from d4straub May 27, 2024 08:50
Copy link
Collaborator

@d4straub d4straub left a comment

Choose a reason for hiding this comment

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

Looks good to me, just low priority comments that I trust you will solve if you deem necessary.

README.md Outdated Show resolved Hide resolved
bin/multiqc_to_custom_csv.py Outdated Show resolved Hide resolved
conf/test_full.config Outdated Show resolved Hide resolved
docs/output.md Outdated Show resolved Hide resolved
nextflow_schema.json Outdated Show resolved Hide resolved
subworkflows/local/kmerfinder_subworkflow.nf Show resolved Hide resolved
@Daniel-VM
Copy link
Contributor Author

Thanks @d4straub. Due to Git issues I couldn't apply your suggestions directly on this PR. I have created a new PR to add the code review: #139.

Daniel-VM and others added 2 commits June 3, 2024 08:39
Reviewer 2 suggestions before v2.3.0 release
@Daniel-VM
Copy link
Contributor Author

Hi @FriederikeHanssen, I believe all the suggestions have been addressed except for the MultiQC issue. (We can create a specific issue to resolve that.) If there is anything else pending for review, please pin me and I will address it.

Added missing license in `kmerfinder_summary.py`
CHANGELOG.md Outdated Show resolved Hide resolved
Update date of release in CHANGELOG.md
@Daniel-VM Daniel-VM merged commit d230ebf into master Jun 12, 2024
21 checks passed
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.

5 participants