SDEV-5394 - bugfix get_cancer_genes - cancer_genes list should includ…#111
Conversation
…e tumour_suppressors and oncogenes.
…t includes tumour_suppressors and oncogenes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #111 +/- ##
===========================================
- Coverage 83.46% 83.31% -0.16%
===========================================
Files 18 18
Lines 2546 2553 +7
===========================================
+ Hits 2125 2127 +2
- Misses 421 426 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
|
|
||
| def get_cancer_genes(conn: GraphKBConnection) -> List[Ontology]: | ||
| """Get the list of cancer genes stored in GraphKB derived from OncoKB & TSO500. |
There was a problem hiding this comment.
These changes to get_cancer_genes() make a lot of a difference: 1168 records now, 423 previously; just saying. If we go forward with these changes, we should add something in the comments so it is explicit about what a 'cancer gene' is in that context (i.e. not the same as 'cancer gene' in GraphKB!).
Right now this function is also used in get_gene_information() to flag 'cancer gene'; are we fine with that? As far as I know, get_oncokb_oncogenes(), get_oncokb_tumour_supressors() and get_cancer_genes() were meant to populate flags in get_gene_information(). Maybe we shoud instead have a distinct function for that new 'meaning' in pori_python, or maybe the probe script should do the composition?
There was a problem hiding this comment.
Maybe we shoud instead have a distinct function for that new 'meaning' in pori_python,
That makes sense if there is a reason to have the two distinct meanings & lists.
Mostly the issue seems to be that the gene lists and names of the gene lists should make sense to the clininfo group that is most interested in them.
maybe the probe script should do the composition?
Done for iprobe v10.11.0
No particular rush here, so we can take time to come up with a proper solution.
Right now this function is also used in get_gene_information() to flag 'cancer gene'; are we fine with that?
I think it's clininfo that is interested in these flags, so I think that is a question for their group.
Do you know if the gene flags used in the report templates/tables to determine what gets displayed somewhere?
There was a problem hiding this comment.
Can only see 2 references in IPR API, and the behavior shouldn't change:
| for gene in CANONICAL_ONCOGENES: | ||
| assert gene not in names | ||
| assert gene in names | ||
|
|
There was a problem hiding this comment.
Noting that these tests were explicit about a 'cancer gene' not being the same as an oncogene or a tumour suppressor gene.
There was a problem hiding this comment.
Yes, this was obviously intentional.
3 participants
…e tumour_suppressors and oncogenes.
Update the list returned by the get_cancer_genes function to include all oncogenes and tumour_suppressors.
Based on Erin's comments in:
https://www.bcgsc.ca/jira/browse/SDEV-5394