This repository has been archived by the owner on Jun 21, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 67
add cnv interpretation #216
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
caf1419
add cnv interpretation
5b33b79
fix table
b3ab629
remove extra line spaces
f167b16
Spacing around CNV table
jaclyn-taroni cfa1875
Update README.md
021ade7
Merge branch 'master' into cnv-ploidy-explanation
jaclyn-taroni 7acff9f
Merge branch 'master' into cnv-ploidy-explanation
jaclyn-taroni File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One is supposed to look at the
tumor_ploidy
column in thepbta_histologies.tsv
file and thecopy number
column in the ControlFreeC TSV, is that correct? Two thoughts on this:copy.num
in CNVkit SEG) so an analyst has something all in one spot?tumor_ploidy
inpbta-histologies.tsv
,copy number
inpbta-cnv-controlfreec.tsv.gz
, Gain/Loss InterpretationThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was thinking it would make more sense to add it to the ControlFreeC file, but it also may be confusing because in many cases, it would contradict the
genotype
column. The ploidy in the clinical file is overall tumor ploidy, not the segment ploidy, so we can add it if you think it is useful, but maybe then we modifygenotype
tosegment_genotype
.The table was also meant to be inclusive of CNVkit, which is why I didn't label the columns specifically.
On another note, it looks like we have gain/loss info in the ControlFreeC TSV file, so those should also help the user not rely solely on copy number. The challenging thing would be - what then defines a homozygous loss, because does that mean for ploidy 3, homozygous loss is all 3 copies? (I may have to dig more into my genetics for this answer). I didn't realize this was the case when writing up the info for #182.
Thoughts on all of this - easiest for the user? Maybe we default to just using
loss
broadly, rather than categorizing as homo/hemi, and confirm total copy loss with lack of RNA expression, usegain
broadly, and set a cutoff for amplification? That way we are not relying on copy number numbers which require ploidy interpretation, but rather, the gain/loss calls for ControlFreeC?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add
tumor_ploidy
to the TSV file, and changegenotype
tosegment_genotype
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would lean toward using
loss
broadly, but I don't think that we should rely on RNA expression as the only indicator of total loss; we should indicate total loss based on the seg calls as well.The reason I am hesitant to rely on RNA expression is that it is quite possible to have a complete loss of some exons while others remain present. This could result in total loss of functionality, while the RNA level might indicate expression of the gene, as some exons are being expressed. This would be rare, I'd expect, but I feel like it is worth highlighting discrepancies between RNA and genomic data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely the case for ATRX, and we treat it a bit differently - we had been doing a coverage-based estimation of exons lost and reporting those as deletions and/or assessing SVs in this gene as a complement. However, the RNA-level loss is something we did in the past to ensure loss of genes' expression, especially in cases in which we know there should be complete loss (eg SMARCB1 in ATRT and some types of chordomas, CDKN2A/B in leukemias - not relevant here) and/or to ensure CN calls were generally lining up with expectations. It does get a bit complicated, I agree, trying to be broad, yet somehow cover all bases. In cases of hemizygous loss, I did a lot of manual inspection to ensure CN calls were accurate (which is not always the case).