-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add reVUE revised effect for one KIT variant #139
base: master
Are you sure you want to change the base?
Conversation
This uses some of the old "curious cases" infrastructure but replaces it by directly using the VUE data from the revue-website repo. This still needs to be fully refactored to lose all references to "curious cases".
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.
style={{ textDecoration: 'none' }} | ||
> | ||
<img | ||
src="https://www.cancerrevue.org/static/media/vue_logo.f7904d3925e95ec147ad.png" |
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.
Can we store the icon somewhere locally in case the URL changes? E.g. we have oncokb logo here https://github.com/genome-nexus/genome-nexus-frontend/blob/master/src/component/variantPage/biologicalFunction/oncokb.png
<span> | ||
{props.vue.comment}{' '} | ||
<a | ||
href={`https://pubmed.ncbi.nlm.nih.gov/${props.vue.pubmedIds[0]}/`} |
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 guess in the future we would show all PubMed ids, right?
); | ||
if (renderData === null) { | ||
return null; | ||
} | ||
if (renderData) { | ||
renderData = renderData.filter((data) => data.value != null); // remove null fields | ||
} | ||
let basicInfoList = _.map(renderData, (data) => { | ||
let basicInfoBeforeVUE = _.map(renderData.slice(0, 2), (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.
Better to find by name or id
data.category | ||
); | ||
}); | ||
let basicInfoAfterVUE = _.map(renderData.slice(2), (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.
same here
category: getMutationTypeClassName(transcript), | ||
}); | ||
|
||
// Check if variant is a VUE |
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.
As far as I understand, the logic here is trying to say, check if this variant is VUE, if it's VUE, don't add protein change and variant classification, and read from VUE component instead. If it's not VUE, add default protein change and variant classification.
So better to change the comment to indicate this is the situation that "add default protein change and variant classification when it is not VUE"
style={{ | ||
paddingLeft: 3, | ||
paddingTop: 2, | ||
paddingBottom: 2, | ||
paddingRight: 0, | ||
marginLeft: -2, | ||
marginRight: 4, | ||
}} |
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.
Good to add in css
import * as React from 'react'; | ||
import functionalGroupsStyle from '../functionalGroups.module.scss'; | ||
|
||
export declare type RevisedProteinEffectRecord = { |
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.
why use declare here?
@@ -185,7 +186,7 @@ class Variant extends React.Component<IVariantProps> { | |||
[TrackName.Exon]: 'visible', | |||
[TrackName.UniprotTopology]: 'visible', | |||
}} | |||
hugoSymbol={mutation[0].gene.hugoGeneSymbol} | |||
hugoSymbol={mutation[0]?.gene?.hugoGeneSymbol} |
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.
Not harmful, but we already have undefined checks above. https://github.com/genome-nexus/genome-nexus-frontend/blob/master/src/page/Variant.tsx#L154-L157
@@ -22,14 +22,15 @@ import { | |||
import { annotationQueryFields } from '../config/configDefaults'; | |||
import { getTranscriptConsequenceSummary } from '../util/AnnotationSummaryUtil'; | |||
import { getDataFetcher } from '../util/ApiUtils'; | |||
import genomeNexusInternalClient from '../util/genomeNexusClientInternalInstance'; | |||
//import genomeNexusInternalClient from '../util/genomeNexusClientInternalInstance'; |
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.
If it's not being used, we can remove this
import genomeNexusClient from '../util/genomeNexusClientInstance'; | ||
import oncoKbClient from '../util/oncokbClientInstance'; | ||
import { | ||
variantToGenomicLocationString, | ||
//variantToGenomicLocationString, |
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.
same here
This uses some of the old "curious cases" infrastructure but replaces it by directly using the VUE data from the revue-website repo. This still needs to be fully refactored to lose all references to "curious cases".
Works for: https://deploy-preview-139--genome-nexus-frontend.netlify.app/variant/4:g.55593576_55593606del
This introduces quite some frontend logic to manage the revising of the protein effect. Eventually we can hopefully move this to the server side instead