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

[WORKFLOWS-449] Add bbsplit to Sarek 3.2.2 #4

Merged
merged 5 commits into from
Apr 2, 2024
Merged

[WORKFLOWS-449] Add bbsplit to Sarek 3.2.2 #4

merged 5 commits into from
Apr 2, 2024

Conversation

danlu1
Copy link

@danlu1 danlu1 commented Mar 21, 2024

Problem:

Need to add bbmap_bbsplit to Sarek v3.2.2

Solution:

  1. Pulled 3.2.2 tag and created a branch v3.2.2-sage
  2. Added bbsplit module following this instruction and what was done in rnaseq pipeline
  3. Cross-check the bbsplit output from the modified sarek pipeline and rnaseq with the SAME input
  4. Included instructions on how to use bbsplit in Sarek

Note:
Test profile was modified by adding bbsplit related params and genome reference(bbsplit_fasta_list) for testing. Since there is no bbsplit test dataset available for Sarek, I use bbsplit_fasta_list from rnaseq test profile for now.

Cross-check:

  1. the output results are the same with Same input, fasta, bbsplit_fasta_list
  2. Code for cross-check will be included in another branch.
Screenshot 2024-03-19 at 9 20 54 PM Screenshot 2024-03-19 at 9 20 04 PM Screenshot 2024-03-19 at 9 19 15 PM Screenshot 2024-03-19 at 9 12 49 PM Screenshot 2024-03-19 at 9 12 24 PM

@danlu1 danlu1 requested a review from a team March 21, 2024 22:04
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 21, 2024 •
edited
Loading

nf-core lint overall result: Failed ❌

Posted for pipeline commit 68d9c5b

+| ✅ 169 tests passed       |+
#| ❔   7 tests were ignored |#
!| ❗   1 tests had warnings |!
-| ❌  19 tests failed       |-

❌ Test failures:

  • files_exist - File must be removed: lib/Utils.groovy
  • files_exist - File must be removed: lib/WorkflowMain.groovy
  • files_exist - File must be removed: lib/NfcoreTemplate.groovy
  • files_exist - File must be removed: lib/WorkflowSarek.groovy
  • nextflow_config - Config variable not found: params.validationShowHiddenParams
  • nextflow_config - Config variable not found: params.validationSchemaIgnoreParams
  • nextflow_config - Config default value incorrect: params.igenomes_base is set as s3://ngi-igenomes/igenomes/ in nextflow_schema.json but is s3://ngi-igenomes/igenomes in nextflow.config.
  • files_unchanged - CODE_OF_CONDUCT.md does not match the template
  • files_unchanged - .github/CONTRIBUTING.md does not match the template
  • files_unchanged - .github/ISSUE_TEMPLATE/bug_report.yml does not match the template
  • files_unchanged - .github/PULL_REQUEST_TEMPLATE.md does not match the template
  • files_unchanged - .github/workflows/branch.yml does not match the template
  • files_unchanged - .github/workflows/linting_comment.yml does not match the template
  • files_unchanged - .github/workflows/linting.yml does not match the template
  • files_unchanged - assets/email_template.html does not match the template
  • files_unchanged - assets/email_template.txt does not match the template
  • files_unchanged - pyproject.toml does not match the template
  • schema_params - Param schema_ignore_params from nextflow config not found in nextflow_schema.json
  • multiqc_config - 'assets/multiqc_config.yml' does not contain a matching 'report_comment'.
    The expected comment is:
    This report has been generated by the <a href="https://github.com/nf-core/sarek/releases/tag/3.2.2" target="_blank">nf-core/sarek</a> analysis pipeline. For information about how to interpret these results, please see the <a href="https://nf-co.re/sarek/3.2.2/docs/output" target="_blank">documentation</a>.
    The current comment is:
    This report has been generated by the <a href="https://github.com/nf-core/sarek" target="_blank">nf-core/sarek</a> analysis pipeline. For information about how to interpret these results, please see the <a href="https://nf-co.re/sarek" target="_blank">documentation</a>.

❗ Test warnings:

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.13.1
  • Run at 2024-04-02 18:36:59

thomasyu888
thomasyu888 reviewed Mar 25, 2024
View reviewed changes
nextflow_schema.json Outdated Show resolved Hide resolved
@danlu1 danlu1 force-pushed the add_bbsplit_to_3_2_2 branch from 46cc7b2 to 2ecf8db Compare March 25, 2024 18:14
@danlu1 danlu1 requested a review from thomasyu888 April 1, 2024 17:14
thomasyu888
thomasyu888 approved these changes Apr 1, 2024
View reviewed changes
Copy link

@thomasyu888 thomasyu888 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 work here Dan! I'm happy for us to merge this as we wait for the example dataset (late April) timeline

Although - not sure why the linting is failing. Can we look into that?

@danlu1 danlu1 closed this Apr 2, 2024
@danlu1 danlu1 reopened this Apr 2, 2024
@danlu1
Copy link
Author

danlu1 commented Apr 2, 2024

The linting errors are mainly due to the refactor of pipelines template which removes lib/ (see here.) Some other errors are to files (such as assets/multiqc_config.yml) that I didn’t change but they match what is in Sarek v3.2.2.

@danlu1 danlu1 merged commit f2c35f4 into Sage-Bionetworks-Workflows:3.2.2-sage Apr 2, 2024
7 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasyu888 thomasyu888 thomasyu888 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
@danlu1 @thomasyu888