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

Fix the stacksize and pretty much everything else #311

Merged
merged 17 commits into from
Feb 5, 2024
Merged

Fix the stacksize and pretty much everything else #311

merged 17 commits into from
Feb 5, 2024

Conversation

apeltzer
Copy link
Member

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/smrnaseq 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 <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).

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 30, 2024 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 59eea65

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

❗ Test warnings:

❔ Tests ignored:

  • nextflow_config - Config default ignored: params.fastp_known_mirna_adapters

✅ Tests passed:

Run details

  • nf-core/tools version 2.12.1
  • Run at 2024-02-02 14:49:52

@christopher-mohr
Copy link
Contributor

Added FASTP length filtering for UMI cases after UMI extraction. I think because of the changes of yesterday, the cases where we do not have UMIs still need some more testing.

@apeltzer apeltzer changed the title Fix the stacksize Fix the stacksize and pretty much everything else Feb 1, 2024
@christopher-mohr
fix module config name of FASTP process, add
8c1143d 
publish dir for second FASTP process
@christopher-mohr
change default parameter values of FASTP max
f83f0a5 
length and three prime adapter sequence, set adapter fasta
based on with_umi parameter, change included adapter fasta
@christopher-mohr
change FASTP module publish dir names
18b6aa8
@christopher-mohr
adapt default value of param fastp_max_length
59eea65
@christopher-mohr
Copy link
Contributor

christopher-mohr commented Feb 2, 2024 •
edited
Loading

There were a couple of issues with the module FASTP / how it was used. First of all, the withName definition for the FASTP process of the subworkflow did not match the actual process name.

Furthermore FASTP has an auto-detect adapter function that can only be disabled by explicitly setting the option --adapter_sequence.

Since by default, this option was not used in the FASTP call, FASTP was auto-detecting the adapter from the first 1M reads, and only then started trimming the adapters from the known_adapter.fasta.

Therefore the three_prime_adapter parameter of the pipeline should always be set. The default is now set to the Illumina sequencing adapter.

The adapter_fasta now contains the miRNA seq adapters for the different kits again. This allows to have zero impact on users of smranseq that do not use UMIs. We are now also setting the known mirna adapters based on the parameter option with_umi. No adapter fasta will be provided to FASTP if with_umi is set to true.

@apeltzer apeltzer marked this pull request as ready for review February 5, 2024 09:25
@apeltzer apeltzer merged commit 3ef02e7 into dev Feb 5, 2024
8 checks passed
@apeltzer apeltzer deleted the fix-stack-size branch February 5, 2024 09:55
@apeltzer apeltzer mentioned this pull request Feb 22, 2024
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christopher-mohr christopher-mohr Awaiting requested review from christopher-mohr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@apeltzer @christopher-mohr