sanjaynagi / AmpSeeker

A snakemake workflow for amplicon sequencing
https://sanjaynagi.github.io/AmpSeeker/
0 stars 3 forks source link

Fastp Implementation #26

Closed ChabbyTMD closed 1 year ago

ChabbyTMD commented 1 year ago

Added fastp implementation to multiqc rule file and other requisite changes. #25

sanjaynagi commented 1 year ago

Awesome thanks @ChabbyTMD!

Given that we have fastp inmultiQC.smk, i think it would be good to move all multiQC.smk contents to qc.smk and just have that instead for all the qc rules.

Also, how does the fastp output look?

sanjaynagi commented 1 year ago
  • modify multiqc to capture one of either fastp reports (html preferably)
  • also with fastp report captured by multiqc, not deemed necessary to capture as required in AmpSeekerOutputs()

Just to say, if we pass the entire results/ folder to multiqc, it will automatically find the fastp output. But I think its good practice to both pass results/ and explicitly also pass the fastp output aswell.

And for the second point, I think its good to keep all the outputs in AmpSeekerOutputs(), because we probably want options for all the QC steps in the config. For example, if we were to remove the fastp output from AmpSeekerOutputs(), and if we have a multiQC option and it is set to False, Fastp will never be run. I think its good to allow a user to run fastp but not run multiqc if they wanted to for whatever reason.

sanjaynagi commented 1 year ago

Hi both, Ive just moved all the contents of multiqc.smk into the qc.smk, I don't think there's a need for both :)

Will merge once the CIs completes.

sanjaynagi commented 1 year ago

@eddUG @ChabbyTMD Just merged, but then also realised - did we test fastp ability to remove adapters at all? If not ill make a new issue to test that at some point.