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

Enhancement/agfusion expansion #138

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

pintoa1-mskcc
Copy link
Collaborator

@pintoa1-mskcc pintoa1-mskcc commented Feb 13, 2025

PR checklist

  • This PR expands AGFusion for a select list of clinical genes. AGFusion will check all possible transcript combinations for any fusion which has at least one genes from the clinical gene list.
  • 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).

@pintoa1-mskcc pintoa1-mskcc requested review from anoronh4, carynhale and huyu335 and removed request for huyu335 February 13, 2025 15:35
@anoronh4
Copy link
Collaborator

can you add more description to either the top comment or a new comment, explaining in further detail what you're addressing? That way it's easier to look back through the history and figure it out.

Copy link
Collaborator

@anoronh4 anoronh4 left a comment

Choose a reason for hiding this comment

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

seems like add_flags_and_cluster_information.R and add_flags_agfusion_clinical.R are pretty similar, i think it might make sense to consolidate these two and maybe have a few flags to handle different run modes.

@@ -10,7 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [#117](https://github.com/mskcc/forte/pull/117) - add supporting-reads_gene-fusions\*.zip files to fusioncatcher outputs

- [#118](https://github.com/mskcc/forte/pull/118) - change the way the plug-n-play starfusion reference is downloaded.
- [#126](https://github.com/mskcc/forte/pull/126) - enable clinical genes prioritization in Metafusion

- [#126](https://github.com/mskcc/forte/pull/126) - enable transcript prioritization in Metafusion
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a new line for this PR 138

@@ -15,7 +15,7 @@ RT_call_filter=1
blck_filter=1
ANC_filter=1
usage() {
echo "Usage: Metafusion_forte.sh --num_tools=<minNumToolsCalled> --genome_fasta <FASTA adds SEQ to fusion> --recurrent_bedpe <blacklistFusions> --outdir <outputDirectory> --cff <cffFile> --gene_bed <geneBedFile> --gene_info <geneInfoFile> --clinical_genes <clinicalGenes>" 1>&2;
echo "Usage: Metafusion_forte.sh --num_tools=<minNumToolsCalled> --genome_fasta <FASTA adds SEQ to fusion> --recurrent_bedpe <blacklistFusions> --outdir <outputDirectory> --cff <cffFile> --gene_bed <geneBedFile> --gene_info <geneInfoFile> --clinical_genes <transcript_allowlist>" 1>&2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we also rename the parameter --clinical_genes to something like --allowed_transcripts?


usage <- function() {
message("Usage:")
message("add_flags_agfusion_clinical.R --cff-file <file.cff> --agfusion-file <agfusion.tsv> --transcript_allowlist <transcript_allowlist.txt> --out-prefix <prefix>")
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems like the parameters should actually be cff and agfusion, not cff-file and agfusion-file?

by.x = "3'_transcript",
by.y = "ensembl_transcript",
all.x = T,
all.y = F)
Copy link
Collaborator

Choose a reason for hiding this comment

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

we're not going to filter out agfusion results where one or both transcripts are not on the transcript allowlist? not saying this is wrong, just checking

@@ -38,7 +38,7 @@ process METAFUSION_RUN {
--gene_info $info \\
--genome_fasta $fasta \\
--recurrent_bedpe $blocklist \\
--clinical_genes $clinicalgenes \\
--clinical_genes $transcript_allowlist \\
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename this parameter?

@@ -129,7 +129,7 @@ workflow FORTE {
PREPROCESS_READS.out.reads_trimmed,
PREPROCESS_READS.out.reads_untrimmed,
PREPARE_REFERENCES.out.star_index,
PREPARE_REFERENCES.out.fasta,
PREPARE_REFERENCES.out.fasta,
Copy link
Collaborator

Choose a reason for hiding this comment

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

fix indentation here


container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
'ghcr.io/rocker-org/devcontainer/tidyverse:4' :
'ghcr.io/rocker-org/devcontainer/tidyverse:4' }"
Copy link
Collaborator

Choose a reason for hiding this comment

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

change this to a more specific version to make sure that it is reproducible, i would suggest: ghcr.io/rocker-org/tidyverse:4.4.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants