Replace legacy tracks with IGV #4188

onursumer · 2022-02-23T14:10:56Z

Need to fix these before merging:

Remove the legacy tracks completely (currently for comparison both legacy tracks and IGV are rendered)
Update screenshots
Mutation colors are random each time the IGV is rendered. Should match the colors in the table.
~~Should point to https://github.com/cBioPortal/igv.js fork instead of https://github.com/onursumer/igv.js~~ Use npm package directly

Checks

Has tests or has a separate issue that describes the types of test that should be created. If no test is included it should explicitly be mentioned in the PR why there is no test.
The commit log is comprehensible. It follows 7 rules of great commit messages. For most PRs a single commit should suffice, in some cases multiple topical commits can be useful. During review it is ok to see tiny commits (e.g. Fix reviewer comments), but right before the code gets merged to master or rc branch, any such commits should be squashed since they are useless to the other developers. Definitely avoid merge commits, use rebase instead.
Is this PR adding logic based on one or more clinical attributes? If yes, please make sure validation for this attribute is also present in the data validation / data loading layers (in backend repo) and documented in File-Formats Clinical data section!

onursumer · 2022-03-25T23:08:03Z

src/pages/patientView/genomicOverview/GenomicOverview.tsx

        return (
            this.props.containerWidth - (this.shouldShowVAFPlot() ? 140 : 40)
        );
    }

-    private computeMutationFrequencyBySample(


moved to GenomicOverviewUtils

onursumer · 2022-03-25T23:09:28Z

src/pages/patientView/genomicOverview/GenomicOverviewUtils.spec.tsx

@@ -198,4 +200,236 @@ describe('GenomicOverviewUtils', () => {
            );
        });
    });
+
+    describe('computeMutationFrequencyBySample()', () => {


tests moved from GenomicOverview.spec.tsx

onursumer · 2022-03-25T23:10:05Z

src/pages/patientView/genomicOverview/GenomicOverviewUtils.ts

@@ -92,3 +95,32 @@ export function sampleIdToIconData(

    return lookupTable;
 }
+
+export function computeMutationFrequencyBySample(


moved from GenomicOverview.tsx

alisman · 2022-04-04T20:48:47Z

src/pages/patientView/genomicOverview/GenomicOverview.tsx

@@ -118,6 +385,12 @@ export default class GenomicOverview extends React.Component<
        );
    }

+    componentWillReceiveProps(nextProps: Readonly<IGenomicOverviewProps>) {


this is so it doesn't render if locus hasn't actually changed?

Right. This is to prevent resetting zoom state on window resize. Hopefully this didn't introduce some unknown side effects.

We should put a comment on this to explain

inodb

Looks awesome! I think the main thing I noticed is:

There's a short time where the component just shows up as blank. I'm guessing this is in the IGV component itself so might be hard to fix. It happens both when loading the page the first time and when clicking on a mutation
There's a lot of whitespace: (1) between the first track and the chromosome track and (2) the chromosome track has a lot. Old representation was better about space use. Not sure if we can tweak this much more

onursumer · 2022-04-27T02:23:21Z

Looks like sample mutation data do not always get sorted properly. e2e tests actually caught this problem. Here is an example where sample mutation data don't match the sample order: https://deploy-preview-4188--cbioportalfrontend.netlify.app/patient?caseId=P04&studyId=lgg_ucsf_2014. Click on any mutation and you will see that sample id displayed in the tooltip doesn't match the sample icon/number shown on the left for that specific sample.

This needs further debugging. The problem might be in the compareFn defined here:

cbioportal-frontend/src/pages/patientView/genomicOverview/GenomicOverview.tsx

Line 167 in 8198ddf

const compareFn = (a: { sampleId: string }, b: { sampleId: string }) =>

We should not merge the PR before fixing the sorting issue.

onursumer · 2022-04-29T19:33:42Z

The sort function seems working fine. Looks like IGV does not always keep the sample order intact. Need to investigate further to see if it is possible to force the sample order in IGV.

onursumer · 2022-04-29T20:21:05Z

Looks like this is a know igv.js issue. I have left a comment there as well. Let's see if we can get a quick fix. igvteam/igv.js#1480 (comment)

onursumer · 2022-04-29T23:45:16Z

Sorting issue should be fixed now.

Update screenshots Fix vaf tests Update circleci cache keys Fix another IGV track test Update cn segments tab screenshot

onursumer added the DO NOT MERGE label Feb 23, 2022

onursumer force-pushed the patient-view-igv branch 5 times, most recently from 352912f to 38c875e Compare March 1, 2022 15:36

onursumer force-pushed the patient-view-igv branch 4 times, most recently from 3c33cc4 to 554e750 Compare March 14, 2022 23:16

onursumer force-pushed the patient-view-igv branch 4 times, most recently from c2c16e8 to 6b11b77 Compare March 22, 2022 18:52

onursumer force-pushed the patient-view-igv branch 4 times, most recently from e5c176d to 75c40f9 Compare March 25, 2022 21:42

onursumer marked this pull request as ready for review March 25, 2022 23:05

onursumer requested review from alisman and inodb March 25, 2022 23:06

onursumer commented Mar 25, 2022

View reviewed changes

onursumer force-pushed the patient-view-igv branch from 75c40f9 to 9f92184 Compare March 25, 2022 23:21

onursumer added ready to review needs-product-review labels Mar 25, 2022

onursumer force-pushed the patient-view-igv branch from 9f92184 to 8484bcd Compare March 29, 2022 22:27

alisman reviewed Apr 4, 2022

View reviewed changes

onursumer force-pushed the patient-view-igv branch from bffc44d to 1c996f4 Compare April 6, 2022 00:18

inodb reviewed Apr 7, 2022

View reviewed changes

onursumer force-pushed the patient-view-igv branch from 1c996f4 to f63865d Compare April 7, 2022 23:07

onursumer force-pushed the patient-view-igv branch 4 times, most recently from 4904141 to bb3433b Compare April 19, 2022 03:06

onursumer force-pushed the patient-view-igv branch 2 times, most recently from 4ce2eff to 2d9f30d Compare April 26, 2022 22:11

onursumer removed DO NOT MERGE needs-product-review labels Apr 26, 2022

onursumer force-pushed the patient-view-igv branch from 2d9f30d to bec913f Compare April 27, 2022 02:17

onursumer added the DO NOT MERGE label Apr 27, 2022

onursumer force-pushed the patient-view-igv branch from bec913f to 2acba50 Compare April 29, 2022 22:51

onursumer force-pushed the patient-view-igv branch from 2acba50 to a953ac8 Compare April 30, 2022 00:40

onursumer removed the DO NOT MERGE label Apr 30, 2022

onursumer force-pushed the patient-view-igv branch from a953ac8 to 97e580c Compare May 3, 2022 17:06

replace legacy tracks with IGV

8756377

alisman force-pushed the patient-view-igv branch 2 times, most recently from 65d8cbb to dc34ffa Compare May 5, 2022 15:26

fix failing e2e and screenshot tests

035ff68

Update screenshots Fix vaf tests Update circleci cache keys Fix another IGV track test Update cn segments tab screenshot

alisman force-pushed the patient-view-igv branch from 5321c70 to 035ff68 Compare May 5, 2022 19:38

alisman merged commit f5e21a8 into cBioPortal:master May 5, 2022

alisman deleted the patient-view-igv branch May 5, 2022 20:43

dippindots added feature and removed ready to review labels May 10, 2022

This was referenced May 25, 2022

Support whole genome mutation track in IGV 2.0 cBioPortal/icebox#168

Closed

Add sample labels next to CNA track rows in IGV browser cBioPortal/icebox#167

Closed

onursumer mentioned this pull request Feb 28, 2022

Improve loading behavior for Genomic Overview on patient view page cBioPortal/icebox#272

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace legacy tracks with IGV #4188

Replace legacy tracks with IGV #4188

onursumer commented Feb 23, 2022 •

edited

Loading

onursumer Mar 25, 2022

onursumer Mar 25, 2022

onursumer Mar 25, 2022

alisman Apr 4, 2022

onursumer Apr 5, 2022

alisman Apr 5, 2022

inodb left a comment

onursumer commented Apr 27, 2022 •

edited

Loading

onursumer commented Apr 29, 2022

onursumer commented Apr 29, 2022

onursumer commented Apr 29, 2022

Replace legacy tracks with IGV #4188

Replace legacy tracks with IGV #4188

Conversation

onursumer commented Feb 23, 2022 • edited Loading

Checks

onursumer Mar 25, 2022

Choose a reason for hiding this comment

onursumer Mar 25, 2022

Choose a reason for hiding this comment

onursumer Mar 25, 2022

Choose a reason for hiding this comment

alisman Apr 4, 2022

Choose a reason for hiding this comment

onursumer Apr 5, 2022

Choose a reason for hiding this comment

alisman Apr 5, 2022

Choose a reason for hiding this comment

inodb left a comment

Choose a reason for hiding this comment

onursumer commented Apr 27, 2022 • edited Loading

onursumer commented Apr 29, 2022

onursumer commented Apr 29, 2022

onursumer commented Apr 29, 2022

onursumer commented Feb 23, 2022 •

edited

Loading

onursumer commented Apr 27, 2022 •

edited

Loading