Skip to content

2.0 - #1021

Open
gongyixiao wants to merge 490 commits into
masterfrom
develop
Open

2.0#1021
gongyixiao wants to merge 490 commits into
masterfrom
develop

Conversation

@gongyixiao

@gongyixiao gongyixiao commented Dec 13, 2024

Copy link
Copy Markdown
Collaborator

Here is the revised release note for PR #1021 (Tempo v2.0) with the citation marks removed:

anoronh4 and others added 30 commits March 31, 2022 13:03
…ates

bugfix for single bam input into MergeBamsAndMarkDuplicates
…andalone process upstream of SomaticMergeSVs
@anoronh4
anoronh4 self-requested a review December 10, 2025 18:10

@anoronh4 anoronh4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Found PRs that need to be included in the description:
#1013
#993
#986
#973
#972
#958
#951
i found these using the following filters on github is:pr is:closed base:develop merged:>2022-01-04

i also believe that the documentation needs some updates:

  • pipeline-flowchart.png
  • output.md

@anoronh4 anoronh4 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should we also update the following line in this PR? https://github.com/mskcc/tempo/blob/develop/nextflow.config#L19

as well as nextflowVersion above it?

@gongyixiao
gongyixiao requested a review from anoronh4 January 16, 2026 18:44
@gongyixiao

Copy link
Copy Markdown
Collaborator Author

Found PRs that need to be included in the description: #1013 #993 #986 #973 #972 #958 #951 i found these using the following filters on github is:pr is:closed base:develop merged:>2022-01-04

i also believe that the documentation needs some updates:

  • pipeline-flowchart.png
  • output.md

Updated the original PR list to include these missing PRs.

@gongyixiao

Copy link
Copy Markdown
Collaborator Author

should we also update the following line in this PR? https://github.com/mskcc/tempo/blob/develop/nextflow.config#L19

as well as nextflowVersion above it?

Done with #1042

if "MuTect2" in info:
t_fw = var.samples[tumor]['F1R2']
t_rev = var.samples[tumor]['F2R1']
n_fw = var.samples[tumor]['F1R2']

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should this be
n_fw = var.samples[normal]['F1R2']
n_rev = var.samples[normal]['F2R1']

Has our strand bias filtering not been working properly?

@anoronh4 anoronh4 Feb 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i understand how this could be confusing, but there are actually two copies of this script and we should probably consolidate. the script that is actually used is this one: https://github.com/mskcc/tempo/blob/develop/containers/bcftools-vt/filter-vcf.py#L93-L94 and those lines were actually corrected in this script about 5 years ago. the one you're referencing is bcftools-vt-mergesvvcf, in a different folder.

i am not sure i remember exactly the reason i duplicated this folder other than that i was adding mergesvvcf at the time and i think it had some overlapping dependencies with the bcftools-vt container.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I remember similar discussion before. Not sure it was documented somewhere in other issues. I would leave it this way and reference it somewhere and correct it during the nf-core modules conversion process. Scripts will need to be outside of the container anyway under the new standard I believe.

Comment thread dsl2.nf
doWF_SV = 'sv' in WFs ? true : false
doWF_facets = doWF_SV && params.assayType == "genome" && ["hisens","purity"].contains(params.svcnv) ? true : doWF_facets
doWF_loh = ['lohhla', 'snv', 'mutsig'].any(it -> it in WFs) ? true : false
doWF_SNV = ['snv', 'mutsig'].any(it -> it in WFs) ? true : false ? true : false

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

`

? true : false ? true : false

`
This is unnecessary. We could just have

doWF_SNV = ['snv', 'mutsig'].any { it in WFs }

And it will do the same thing. Not worth holding up release over, just a syntax thing. We will probably just fix it when we move to nf-core style.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

minor issues in logic for dsl2

4 participants