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.

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

Make using COSMIC optional #547

Merged
merged 5 commits into from
Nov 29, 2024
Merged

Make using COSMIC optional #547

merged 5 commits into from
Nov 29, 2024

Conversation

rannick
Copy link
Collaborator

@rannick rannick commented Nov 28, 2024

Closes #514

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/rnafusion branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • 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).

@rannick rannick changed the base branch from master to dev November 28, 2024 15:43
@nf-core nf-core deleted a comment from github-actions bot Nov 28, 2024
Copy link

github-actions bot commented Nov 28, 2024

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

Posted for pipeline commit c5ed11b

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

❗ Test warnings:

  • files_exist - File not found: conf/igenomes.config
  • files_exist - File not found: conf/igenomes_ignored.config

❔ Tests ignored:

  • files_unchanged - File ignored due to lint config: .github/CONTRIBUTING.md
  • files_unchanged - File ignored due to lint config: .github/PULL_REQUEST_TEMPLATE.md

✅ Tests passed:

Run details

  • nf-core/tools version 3.0.2
  • Run at 2024-11-29 13:36:04

@nschcolnicov
Copy link

nschcolnicov commented Nov 28, 2024

Thanks for this very much needed change. I tried testing these changes with:
nextflow run nf-core/rnafusion -r fusion_report_optional_cosmic -latest --input 'https://raw.githubusercontent.com/nf-core/test-datasets/rnafusion/testdata/human/samplesheet_valid.csv' --outdir results -profile docker --no_cosmic

And it fails due to
Could not find transcript <=> gene map file "Homo_sapiens.GRCh38.102.gtf"

I think we need to set up igenomes first, since this value should be coming from there. Currently it comes from :

gtf = "${params.genomes_base}/ensembl/Homo_sapiens.${params.genome}.${params.ensembl_version}.gtf"

But the params.genomes_base is set to:

genomes_base = "${params.outdir}/references"

So the file is empty.

Also tried running the branch wiht:
nextflow run nf-core/rnafusion -r fusion_report_optional_cosmic -latest --input 'https://raw.githubusercontent.com/nf-core/test-datasets/rnafusion/testdata/human/samplesheet_valid.csv' --outdir results -profile docker --no_cosmic
And I get the error:
Not a valid path value: 'results/references/fusioncatcher/human_v102'

Which is due to missing reference files. Particularly the fusioncatcher_ref.

Should we add a special handling to avoid the user from using --all with --no_cosmic?

@atrigila
Copy link

@nschcolnicov you need to add --build-references.

  nextflow run rnafusion -profile test,docker \
  -stub --build_references \
  --outdir results 

@nschcolnicov
Copy link

nschcolnicov commented Nov 28, 2024

THank you @atrigila!
Running this works:
nextflow run nf-core/rnafusion -r fusion_report_optional_cosmic -latest --input 'https://raw.githubusercontent.com/nf-core/test-datasets/rnafusion/testdata/human/samplesheet_valid.csv' --outdir results -profile docker --no_cosmic --build_references

@rannick Maybe we should add into the pipeline handling or the docs that "--no_cosmic" must be used together with "--build_references" and it is not compatible with "--all"

Copy link

@atrigila atrigila left a comment

Choose a reason for hiding this comment

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

Looks good to me and it is going to be very useful when developing pipeline-level test cases.

@rannick
Copy link
Collaborator Author

rannick commented Nov 29, 2024

Thanks for this very much needed change. I tried testing these changes with: nextflow run nf-core/rnafusion -r fusion_report_optional_cosmic -latest --input 'https://raw.githubusercontent.com/nf-core/test-datasets/rnafusion/testdata/human/samplesheet_valid.csv' --outdir results -profile docker --no_cosmic

And it fails due to Could not find transcript <=> gene map file "Homo_sapiens.GRCh38.102.gtf"

I think we need to set up igenomes first, since this value should be coming from there. Currently it comes from :

gtf = "${params.genomes_base}/ensembl/Homo_sapiens.${params.genome}.${params.ensembl_version}.gtf"

But the params.genomes_base is set to:

genomes_base = "${params.outdir}/references"

So the file is empty.

Also tried running the branch wiht: nextflow run nf-core/rnafusion -r fusion_report_optional_cosmic -latest --input 'https://raw.githubusercontent.com/nf-core/test-datasets/rnafusion/testdata/human/samplesheet_valid.csv' --outdir results -profile docker --no_cosmic And I get the error: Not a valid path value: 'results/references/fusioncatcher/human_v102'

Which is due to missing reference files. Particularly the fusioncatcher_ref.

Should we add a special handling to avoid the user from using --all with --no_cosmic?

I think the problem you are seeing is from not running --build_references first before running the analysis, but does not have to do with the --all flag. I will solve this issue in #505

@rannick rannick merged commit 6bbef42 into dev Nov 29, 2024
7 checks passed
@rannick rannick deleted the fusion_report_optional_cosmic branch November 29, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fusion-report: make COSMIC optional
3 participants