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_dysgu_sarek #1590

Open
wants to merge 19 commits into
base: dev
Choose a base branch
from
Open

add_dysgu_sarek #1590

wants to merge 19 commits into from
+411 −8

Conversation

poddarharsh15
Copy link

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/sarek branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nf-test test tests/ --verbose --profile +docker).
  • 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

This PR is against the master branch ❌

  • Do not close this PR
  • Click Edit and change the base to dev
  • This CI test will remain failed until you push a new commit

Hi @poddarharsh15,

It looks like this pull-request is has been made against the poddarharsh15/sarek master branch.
The master branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to master are only allowed if they come from the poddarharsh15/sarek dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 11, 2024 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit c5e3d65

+| ✅ 201 tests passed       |+
#| ❔  12 tests were ignored |#
!| ❗   3 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in main.nf: Optionally add in-text citation tools to this list.
  • pipeline_todos - TODO string in main.nf: Optionally add bibliographic entries to this list.
  • pipeline_todos - TODO string in main.nf: Only uncomment below if logic in toolCitationText/toolBibliographyText has been filled!

❔ Tests ignored:

  • files_exist - File is ignored: .github/workflows/awsfulltest.yml
  • files_exist - File is ignored: .github/workflows/awstest.yml
  • files_exist - File is ignored: conf/modules.config
  • files_unchanged - File ignored due to lint config: .github/PULL_REQUEST_TEMPLATE.md
  • files_unchanged - File ignored due to lint config: assets/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_logo_dark.png
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore
  • actions_ci - actions_ci
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/sarek/sarek/.github/workflows/awstest.yml
  • template_strings - template_strings
  • modules_config - modules_config

✅ Tests passed:

Run details

  • nf-core/tools version 2.14.1
  • Run at 2024-09-10 11:05:16

@poddarharsh15 poddarharsh15 changed the base branch from master to dev July 11, 2024 10:05
@poddarharsh15 @maxulysse
Update dysgu.config
21f316f 
Co-authored-by: Maxime U Garcia <[email protected]>
@maxulysse
Copy link
Member

maxulysse commented Jul 12, 2024 •
edited by poddarharsh15
Loading

CI failures are unrelated, so no worries about that.
Can you add dysgu in tools in the nextflow_schema.json ?
Also some documentation would be amazing 🙏

Could you please check the last commit to see if I added dysgu in the correct section of the nextflow_schema.json file? I am a bit confused about where I should add the documentation for dysgu—should it be in the modules section or some other section in Sarek? Please let me know :) 🙏

@poddarharsh15
Copy link
Author

@maxulysse I have updated some documentation for dysgu added manual and github page for help and output files.

@poddarharsh15
Copy link
Author

@maxulysse I have updated some documentation for dysgu added manual and github page for help and output files.

Hi @maxulysse,
Would it be possible to run some tests today using the latest version of sarek_dev with dysgu? Please let me know thanks :))

@maxulysse maxulysse added this to the 3.5 milestone Jul 15, 2024
@poddarharsh15
Copy link
Author

Hi @maxulysse @FriederikeHanssen,

I apologize for the interruption, especially if you're on vacation. I need a suggestion and some help regarding an issue I'm encountering while running dysgu locally before it gets merged. If I can use this pipeline without merging, that would be amazing.
I'm seeing errors related to inputs when running dysgu. I suspect this issue might be coming from the main.nf profile because I didn't define the interval parameter properly. Could you please take a look?

Thank you so much for your time and help, as always. I highly appreciate it.

nextflow run . -profile test,docker --tools dysgu --outdir results_dysgu

ERROR ~ Workflow NFCORE_SAREK:SAREK:BAM_VARIANT_CALLING_GERMLINE_ALL:BAM_VARIANT_CALLING_DYSGU` declares 4 input channels but 3 were given

@poddarharsh15
Copy link
Author

Hi @maxulysse @FriederikeHanssen,

I apologize for the interruption, especially if you're on vacation. I need a suggestion and some help regarding an issue I'm encountering while running dysgu locally before it gets merged. If I can use this pipeline without merging, that would be amazing. I'm seeing errors related to inputs when running dysgu. I suspect this issue might be coming from the main.nf profile because I didn't define the interval parameter properly. Could you please take a look?

Thank you so much for your time and help, as always. I highly appreciate it.

nextflow run . -profile test,docker --tools dysgu --outdir results_dysgu

ERROR ~ Workflow NFCORE_SAREK:SAREK:BAM_VARIANT_CALLING_GERMLINE_ALL:BAM_VARIANT_CALLING_DYSGU` declares 4 input channels but 3 were given

Hi @FriederikeHanssen any updates on module dysgu?

@FriederikeHanssen FriederikeHanssen removed this from the 3.5 milestone Aug 19, 2024
asp8200
asp8200 reviewed Sep 8, 2024
View reviewed changes
subworkflows/local/bam_variant_calling_germline_all/main.nf
BAM_VARIANT_CALLING_DYSGU (
cram,
fasta,
fasta_fai
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to add a fourth input channels with intervals. It is probably just called intervals.

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

@asp8200 asp8200 asp8200 left review comments

@maxulysse maxulysse maxulysse left review comments

@FriederikeHanssen FriederikeHanssen Awaiting requested review from FriederikeHanssen FriederikeHanssen is a code owner

At least 1 approving review is required to merge this pull request.

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

Successfully merging this pull request may close these issues.
4 participants
@poddarharsh15 @maxulysse @asp8200 @FriederikeHanssen