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 bakta running only for one sample #178

Merged
merged 4 commits into from
Oct 25, 2024
Merged

Fix bakta running only for one sample #178

merged 4 commits into from
Oct 25, 2024

Conversation

Daniel-VM
Copy link
Contributor

@Daniel-VM Daniel-VM commented Oct 24, 2024 •
edited
Loading

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 <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

Fixed bakta that was running for one sample only.

Closes #171

@Daniel-VM Daniel-VM marked this pull request as ready for review October 24, 2024 12:04
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 24, 2024 •
edited
Loading

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

Posted for pipeline commit 5a9559c

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

❗ Test warnings:

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 3.0.2
  • Run at 2024-10-25 08:11:15

@Daniel-VM Daniel-VM mentioned this pull request Oct 24, 2024
Closed
jasmezz
jasmezz reviewed Oct 24, 2024
View reviewed changes
Copy link

@jasmezz jasmezz left a comment

Choose a reason for hiding this comment

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

I don't see how it was running for 1 sample only before 🤔 The ch_fasta channel contains meta and fasta, so it should run for all samples, no? And the inputs to BAKTA_BAKTA have the same structure as before if I read the mapping correctly. Can you explain what changed exactly?

subworkflows/local/bakta_dbdownload_run.nf Outdated Show resolved Hide resolved
@Daniel-VM
Copy link
Contributor Author

I ran a few tests and found that when providing a local path to the Bakta database, the workflow processes only a single sample. However, when I set the workflow to download the database (--baktadb_download true), it processes all samples as expected. I'm not sure why this happens, but the difference seems related to how the database is accessed locally versus being downloaded.

Do you think I should try to find a more suitable fix?

@Daniel-VM @jasmezz
remove view operator
4ed19cb 
Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
@Daniel-VM
Copy link
Contributor Author

Daniel-VM commented Oct 25, 2024 •
edited
Loading

Wait, I've just found the issue 😅 .

Daniel-VM
Daniel-VM commented Oct 25, 2024
View reviewed changes
subworkflows/local/bakta_dbdownload_run.nf
ch_versions = ch_versions.mix(UNTAR.out.versions)
} else {
ch_baktadb = Channel.from(ch_path_baktadb).map{ db -> db }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ch_path_baktadb is already a channel. Re-creating it to assign ch_bakta caused the unusual behavior of processing only a single sample.

Copy link

Choose a reason for hiding this comment

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

I see! I don't have to check how we did it in funcscan then, you already found the culprit ;)

jasmezz
jasmezz approved these changes Oct 25, 2024
View reviewed changes
Copy link

@jasmezz jasmezz left a comment

Choose a reason for hiding this comment

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

LGTM now 🚀

subworkflows/local/bakta_dbdownload_run.nf
ch_versions = ch_versions.mix(UNTAR.out.versions)
} else {
ch_baktadb = Channel.from(ch_path_baktadb).map{ db -> db }
Copy link

Choose a reason for hiding this comment

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

I see! I don't have to check how we did it in funcscan then, you already found the culprit ;)

@Daniel-VM Daniel-VM merged commit 064996b into nf-core:dev Oct 25, 2024
15 checks passed
@Daniel-VM
Copy link
Contributor Author

Thanks @jasmezz :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasmezz jasmezz jasmezz approved these changes

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
@Daniel-VM @jasmezz