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
Download tab fetch Non-Select profiles data individually #3725
Conversation
); | ||
// find samples which share studyId with profile and add identifier | ||
const sampleIdentifiers: SampleMolecularIdentifier[] = (samples as Sample[]) | ||
.filter( |
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.
there are a couple easy optimizations here. 1. you don't need to filter first. you can just put a condition in the reducer that just returns acc if studyId doesn't appear in selected profile collection.
2. you should use acc.push instead of acc.concat. concat produces a new array each time. so, for every sample, you're producing a new array. push changes array in place, which is find 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.
Good to know the second one, thanks! I will change that.
this.unzipDownloadDataGroupByKey( | ||
downloadDataGroupByProfileName | ||
) | ||
onMobxPromise<any>( |
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.
maybe put a comment above this explaining what "other" means. i think it means non-selected, right?
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 changed those Other
to NonSelected
genes | ||
) => { | ||
// STEP 1: fetch data | ||
const selectedProfiles: MolecularProfile[] = nonSelectedDownloadableMolecularProfiles.filter( |
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.
maybe change name selectedProfiles because they are actually NOT selected, right?
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.
changed to a general term: profiles
sample.studyId in selectedProfilesGroupByStudyId | ||
) | ||
.reduce((acc: SampleMolecularIdentifier[], sample) => { | ||
acc = acc.concat( |
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.
ditto above comments
); | ||
fileDownload(textMap[profileName], `${profileName}.txt`); | ||
} | ||
); | ||
} | ||
|
||
@autobind | ||
private handleTransposedOtherMolecularProfileDownload(profileName: string) { |
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.
what's difference between this method and the one above?
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.
merged two methods together and moved into DownloadUtils
Fix cBioPortal/cbioportal#8528
Fix cBioPortal/cbioportal#8504
Test Link: try download
mRNA Expression, RSEM (Batch normalized from Illumina HiSeq_RNASeqV2)