-
Notifications
You must be signed in to change notification settings - Fork 234
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
Dont include CNA gain or hetloss when querying DRIVER in OQL #3150
Conversation
1abcbb3
to
6947a62
Compare
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.
Solve the issue reported cBioPortal/cbioportal#7376
@@ -1,21 +1,111 @@ | |||
/* eslint camelcase: "off" */ |
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.
@adamabeshouse what's the main change here - adding types (js -> ts)? Anything else?
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.
that plus just the logic change I just indicated below
@@ -1,83 +0,0 @@ | |||
//export function filterCBioPortalWebServiceData(oql_query, data, opt_default_oql, opt_by_oql_line, opt_mark_oql_regulation_direction) { |
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.
@adamabeshouse why can this be removed now?
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.
we no longer need a type declaration file because the actual file is now in typescript. before, this served as like a typescript interface.
); | ||
const cnaValue = accessors.cna(datum); | ||
datumWanted = | ||
cnaValue !== 'gain' && // dont include gains or hetloss here |
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.
only logic change is here @inodb
6947a62
to
55e7571
Compare
private isMutation(d: Datum): d is Mutation { | ||
return ( | ||
this.molecularAlterationType(d.molecularProfileId) === | ||
'MUTATION_EXTENDED' |
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.
Don't we have this in enum somewhere?
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.
yes, good catch, I was just copy pasting code here
55e7571
to
56ea272
Compare
Signed-off-by: Abeshouse, Adam A./Sloan Kettering Institute <abeshoua@mskcc.org>
Signed-off-by: Abeshouse, Adam A./Sloan Kettering Institute <abeshoua@mskcc.org>
56ea272
to
443048d
Compare
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.
Looks good. Thanks, @adamabeshouse
Dont include CNA gain or hetloss when querying DRIVER in OQL Former-commit-id: e74b99a
Fixes cBioPortal/cbioportal#7376
When querying
EGFR: DRIVER
From:
to: