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.

Sign up for GitHub

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

Jump to bottom

Add fastqc and fix serotypefinder #380

Merged
merged 28 commits into from
Jan 22, 2025
Merged

Add fastqc and fix serotypefinder #380

merged 28 commits into from
Jan 22, 2025

Conversation

ryanjameskennedy
Copy link
Collaborator

@ryanjameskennedy ryanjameskennedy commented Jan 15, 2025 •
edited
Loading

Description

Summary of the changes made:

  • Added fastqc
  • Updated serotypefinder to take assembly fasta input instead of reads
  • Added hostile
  • Update formating of module variables i.e. sample_id instead of sampleID

Primary function of PR

  • Major functionality improvement / New type of analysis

Testing

Tested on mtuberculosis, saureus & ecoli

@ryanjameskennedy ryanjameskennedy requested a review from a team as a code owner January 15, 2025 19:29
This was linked to issues Jan 15, 2025
Poor performance of Serotypefinder using fastq files as input #329
Closed
Add human read depletion as optional pre-processing step #361
Closed
Add Fastqc as a module #379
Closed
samuell
samuell requested changes Jan 20, 2025
View reviewed changes
Copy link
Contributor

@samuell samuell left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great! Mostly nitpicks, but had some questions regarding the channel connectivity in workflows/bacterial_base.nf that might need to be carefully reviewed, as I don't see if it completely correct? See below!

configs/nextflow.base.config Outdated Show resolved Hide resolved
configs/nextflow.ci.config Outdated Show resolved Hide resolved
configs/nextflow.dev.config Outdated Show resolved Hide resolved
configs/nextflow.dev.config Outdated Show resolved Hide resolved
configs/nextflow.dev.config Outdated Show resolved Hide resolved
workflows/bacterial_base.nf Show resolved Hide resolved
workflows/bacterial_base.nf Outdated Show resolved Hide resolved
nextflow-modules/modules/tbprofiler/main.nf Outdated Show resolved Hide resolved
nextflow-modules/modules/tbprofiler/main.nf Outdated Show resolved Hide resolved
workflows/bacterial_base.nf Show resolved Hide resolved
samuell
samuell approved these changes Jan 20, 2025
View reviewed changes
Copy link
Contributor

@samuell samuell left a comment

Choose a reason for hiding this comment

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

Ok, LGTM overall now so approving.
Leaving for you to decide on any remaining minor comments.

This was linked to issues Jan 21, 2025
Additional requirements #327
Closed
Makefile species-specific installation #351
Closed
@ryanjameskennedy ryanjameskennedy merged commit 6a6504d into master Jan 22, 2025
3 checks passed
@ryanjameskennedy ryanjameskennedy deleted the 379-add-fastqc-and-fix-serotypefinder branch January 22, 2025 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samuell samuell samuell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@ryanjameskennedy @samuell