SV: support genomic charts #2890

kalletlak · 2019-11-21T19:41:40Z

frontend changes for cBioPortal/cbioportal#6278

Changes proposed in this pull request:

a add a gene level tab in the add chart button

Feedback from Tali

After I add a chart, the chart appears in the Genomic tab as a greyed out option with 0% frequency. This is not an accurate reflection of the data
The added charts don’t have a “Compare Groups” option
The info button on the added charts should show from the profile description
It would be helpful to show the number of samples with data for each profile in the molecular profile dropdown (like on the co-expression tab)
In the “Gene Level” view, the text says “Click gene symbols below or enter here” but there are no gene symbols below.
The box to enter gene symbols doesn’t need to be that big
The box allows me to enter genes with OQL, but the OQL is ignored. If we’re not going to listen to the OQL, better to not allow it
When I add a chart, it doesn’t add to the end of the page, it adds somewhere in the middle. This makes it hard to find. When I add a clinical/genomic chart it adds to the end with a yellow outline… can we replicate that behavior?

Chart position depends on the priority. Not always charts are added at the end. Yes, yellow outline is shown for new added charts. Priority is set to 0 for custom charts
Download data option doesn’t work in the added charts

How to test:

To test go to https://cbioportal-fix-6278-2vax5qy0co.herokuapp.com and set localStorage.setItem("netlify","deploy-preview-2890--cbioportalfrontend")

kalletlak · 2020-01-30T15:38:02Z

To test https://cbioportal-fix-6278-2vax5qy0co.herokuapp.com and set localStorage.setItem("netlify","deploy-preview-2890--cbioportalfrontend")

src/pages/studyView/StudyViewPageStore.ts

zhx828 · 2020-02-03T16:46:00Z

src/pages/studyView/StudyViewPageStore.ts

+            this.chartsType.set(uniqueKey, ChartTypeEnum.BAR_CHART);
+            this.chartsDimension.set(uniqueKey, { w: 2, h: 1 });
+
+            this._genomicDataBinFilterSet.delete(uniqueKey);


Do you think it will have this uniqueKey in the set?

yes, if user tries to add same genomic chart multiple times

@kalletlak could you give me details about this multiple times? Does it mean user could quickly add the same chart multiple times? Or we can actually add multiple charts for the same genomic data?

User can try to add the same genomic chart(same gene and molecular profile(s)). I think this won't be required since it is override in next step. I'll clean this up

@kalletlak is this designed on purpose? Will the user ever want to have multiple same chart?

Design is not yet finalized. User can same genomic chart(same gene and molecular profile(s)) multiple times, but they can see only one genomic chart for a gene and molecular profile(s)

src/pages/studyView/StudyViewUtils.tsx

src/pages/studyView/UserSelections.tsx

src/pages/studyView/StudyViewUtils.tsx

jjgao

Looks good to me from product perspective. It would be cool to support discrete profiles (e.g. mut, cna) in a follow up pr.

src/pages/studyView/addChartButton/geneLevelSelection/GeneLevelSelection.tsx

alisman · 2020-02-10T16:23:53Z

src/pages/studyView/addChartButton/geneLevelSelection/GeneLevelSelection.tsx

+                        gene.hugoGeneSymbol +
+                        ': ' +
+                        this.selectedOption!.profileName,
+                    description: this.selectedOption!.description,


since you are checking for undefined above, i would think you don't need ! after this.selectedOption. i think ts will know.

Its throwing error if I remove it. May be because its in map

src/pages/studyView/addChartButton/geneLevelSelection/GeneLevelSelection.tsx

zhx828

LGTM, just need to address Aaron's comments

src/pages/studyView/addChartButton/geneLevelSelection/styles.module.scss

tmazor · 2020-03-30T20:49:04Z

@kalletlak this is awesome! I have just a few comments:

If I enter multiple genes at once, all the charts appear, but only one of them has the yellow boarder. Can they all get the yellow border?
It appears these charts will not get saved as part of a virtual study. I think they should be. If that was not part of the original spec, I'm ok with that being a follow-up PR, but I think we need to support that. (cc: @jjgao )
Similarly, I don't think I can log in right now to test this, but will these gene-level charts be saved as part of my study view in the same way that other layout settings are?
I like the new "OQL not allowed message"! But before that shows up, I get the regular "invalid OQL message" and the behavior where it automatically highlights the invalid text. @alisman has been working on an improvement to this behavior - will that improvement get reflected here?
If I add a gene+data type for which there is no data (e.g. IDH1 + RPPA in lgg_tcga), a blank chart appears. The blank chart makes me worry that there was just an issue loading. Can we add a message that says "No data available" instead?
If I try to re-add a chart that is already there, I get the regular "xxxx has been added" message. Instead, can we say "xxxx chart is already added"?

kalletlak · 2020-03-30T21:30:46Z

~~> 1. If I enter multiple genes at once, all the charts appear, but only one of them has the yellow boarder. Can they all get the yellow border?~~

~~looks like a bug, gonna fix it.~~

It appears these charts will not get saved as part of a virtual study. I think they should be. If that was not part of the original spec, I'm ok with that being a follow-up PR, but I think we need to support that. (cc: @jjgao )

Sorry, I did not get it. Charts generally doesn't get saved into virtual study.

~~> 3. Similarly, I don't think I can log in right now to test this, but will these gene-level charts be saved as part of my study view in the same way that other layout settings are?~~

~~It does, these charts are also gets saved in to user layout settings.~~
> 4. I like the new "OQL not allowed message"! But before that shows up, I get the regular "invalid OQL message" and the behavior where it automatically highlights the invalid text. @alisman has been working on an improvement to this behavior - will that improvement get reflected here?

~~It should. it uses the same component.~~
> 5. If I add a gene+data type for which there is no data (e.g. IDH1 + RPPA in lgg_tcga), a blank chart appears. The blank chart makes me worry that there was just an issue loading. Can we add a message that says "No data available" instead?

~~Data might not be available. Should we show No data available message or bar chart with all samples belonging to NA bar?~~

~~> 6. If I try to re-add a chart that is already there, I get the regular "xxxx has been added" message. Instead, can we say "xxxx chart is already added"?~~

~~Sure~~

onursumer

some package updates are not necessary, we should only update cbioportal api client version and packages depending on it, which is currently cbioportal-frontend only

onursumer · 2020-04-10T01:07:56Z

package.json

@@ -1,7 +1,7 @@
 {
  "name": "cbioportal-frontend",
  "private": true,
-  "version": "3.2.13-alpha.0",
+  "version": "3.2.14-alpha.0",


better to update this to 3.2.13-alpha.1, though it is not a big deal since we never publish this package.

onursumer · 2020-04-10T01:12:32Z

packages/genome-nexus-ts-api-client/package.json

@@ -1,7 +1,7 @@
 {
  "name": "genome-nexus-ts-api-client",
  "description": "Genome Nexus API Client for TypeScript",
-  "version": "1.1.2-beta.0",
+  "version": "1.1.3-beta.0",


We shouldn't update this package version, because there is no change in this API client. yarn updatePackageVersion incorrectly prompts this package to update because of the limitation in lerna version. It always checks against the latest tag on master, not the latest commit.

onursumer · 2020-04-10T01:12:50Z

packages/oncokb-ts-api-client/package.json

@@ -1,7 +1,7 @@
 {
  "name": "oncokb-ts-api-client",
  "description": "OncoKB API Client for TypeScript",
-  "version": "1.0.0",
+  "version": "1.0.1",


we shouldn't update this package either, no changes in this package.

onursumer · 2020-04-10T01:13:33Z

package.json

@@ -155,7 +155,7 @@
    "fmin": "^0.0.2",
    "font-awesome": "^4.7.0",
    "fork-ts-checker-webpack-plugin": "^1.2.0",
-    "genome-nexus-ts-api-client": "^1.1.2-beta.0",
+    "genome-nexus-ts-api-client": "^1.1.3-beta.0",


this shouldn't be updated

onursumer · 2020-04-10T01:13:46Z

package.json

@@ -187,7 +187,7 @@
    "node-sass": "^4.9.3",
    "numeral": "^2.0.6",
    "object-sizeof": "^1.2.0",
-    "oncokb-ts-api-client": "^1.0.0",
+    "oncokb-ts-api-client": "^1.0.1",


this shouldn't be updated

onursumer · 2020-04-10T01:15:34Z

packages/cbioportal-frontend-commons/package.json

@@ -42,7 +42,7 @@
    "measure-text": "0.0.4",
    "mobxpromise": "github:cbioportal/mobxpromise#v1.0.2",
    "object-sizeof": "^1.2.0",
-    "oncokb-ts-api-client": "^1.0.0",
+    "oncokb-ts-api-client": "^1.0.1",


this shouldn't be updated

onursumer · 2020-04-10T01:16:04Z

packages/cbioportal-ts-api-client/package.json

@@ -1,7 +1,7 @@
 {
  "name": "cbioportal-ts-api-client",
  "description": "cBioPortal API Client for TypeScript",
-  "version": "1.0.0-beta.0",
+  "version": "1.1.0-beta.0",


this should be 1.0.0-beta.1

why btw should this be .1 and not 1.1.0?

onursumer · 2020-04-10T01:16:37Z

packages/react-mutation-mapper/package.json

    "classnames": "^2.2.5",
-    "genome-nexus-ts-api-client": "^1.1.2-beta.0",
+    "genome-nexus-ts-api-client": "^1.1.3-beta.0",


this shouldn't be updated

onursumer · 2020-04-10T01:16:46Z

packages/react-mutation-mapper/package.json

@@ -38,14 +38,14 @@
  },
  "dependencies": {
    "autobind-decorator": "^2.1.0",
-    "cbioportal-frontend-commons": "^0.2.7",
+    "cbioportal-frontend-commons": "^0.2.8",


this shouldn't be updated

onursumer · 2020-04-10T01:16:53Z

packages/react-mutation-mapper/package.json

    "jquery": "^3.2.1",
    "lodash": "^4.17.11",
    "mobxpromise": "github:cbioportal/mobxpromise#v1.0.2",
    "oncokb-styles": "~0.1.2",
-    "oncokb-ts-api-client": "^1.0.0",
+    "oncokb-ts-api-client": "^1.0.1",


this shouldn't be updated

SV: support genomic charts Former-commit-id: 303c351

kalletlak added DO NOT MERGE do not review labels Nov 21, 2019

kalletlak mentioned this pull request Nov 21, 2019

Study view gene level tab #2736

Closed

jjgao temporarily deployed to cbioportal-f-fix-6278-xnl2dvjt November 26, 2019 14:57 Inactive

kalletlak force-pushed the fix-6278 branch 3 times, most recently from 1f2bece to abfb6d1 Compare January 30, 2020 14:46

inodb added this to the Sprint 5 Y2020 milestone Jan 30, 2020

kalletlak force-pushed the fix-6278 branch from abfb6d1 to c42f72c Compare January 30, 2020 16:28

kalletlak removed DO NOT MERGE do not review labels Jan 30, 2020

kalletlak requested review from zhx828, alisman and jjgao January 30, 2020 19:14

kalletlak force-pushed the fix-6278 branch from c42f72c to c0f7140 Compare January 30, 2020 19:24

zhx828 requested changes Feb 3, 2020

View reviewed changes

kalletlak force-pushed the fix-6278 branch from c0f7140 to d0afe00 Compare February 3, 2020 21:15

inodb assigned tmazor and jjgao Feb 4, 2020

kalletlak force-pushed the fix-6278 branch 3 times, most recently from d851f15 to caaf8f7 Compare February 7, 2020 20:57

jjgao approved these changes Feb 9, 2020

View reviewed changes

kalletlak force-pushed the fix-6278 branch from caaf8f7 to e8248fe Compare February 10, 2020 14:59

alisman reviewed Feb 10, 2020

View reviewed changes

src/pages/studyView/addChartButton/geneLevelSelection/GeneLevelSelection.tsx Outdated Show resolved Hide resolved

alisman reviewed Feb 10, 2020

View reviewed changes

src/pages/studyView/addChartButton/geneLevelSelection/GeneLevelSelection.tsx Outdated Show resolved Hide resolved

alisman reviewed Feb 10, 2020

View reviewed changes

src/pages/studyView/addChartButton/geneLevelSelection/GeneLevelSelection.tsx Outdated Show resolved Hide resolved

zhx828 self-requested a review February 10, 2020 19:40

zhx828 approved these changes Feb 10, 2020

View reviewed changes

kalletlak modified the milestones: Sprint 5 Y2020, Sprint 7 Y2020 Feb 10, 2020

kalletlak force-pushed the fix-6278 branch from e8248fe to d940439 Compare February 10, 2020 20:37

alisman reviewed Feb 11, 2020

View reviewed changes

src/pages/studyView/addChartButton/geneLevelSelection/styles.module.scss Outdated Show resolved Hide resolved

kalletlak force-pushed the fix-6278 branch from d940439 to 6352c26 Compare February 11, 2020 20:36

jjgao modified the milestones: Sprint 7 Y2020, Sprint 9 Y2020 Feb 25, 2020

inodb removed this from the Sprint 9 Y2020 milestone Mar 10, 2020

kalletlak force-pushed the fix-6278 branch from 6352c26 to 144e965 Compare March 30, 2020 16:39

This was referenced Mar 30, 2020

limit digits when binning in quartiles cBioPortal/cbioportal#7339

Closed

study view bar chart x-axis label cut-off cBioPortal/cbioportal#7340

Closed

kalletlak force-pushed the fix-6278 branch 2 times, most recently from 3281aa1 to 9e8b981 Compare April 6, 2020 15:11

kalletlak force-pushed the fix-6278 branch 3 times, most recently from 3b9663f to 5a07929 Compare April 9, 2020 17:09

alisman force-pushed the fix-6278 branch from 34aaa6a to e1b7fec Compare April 9, 2020 23:54

onursumer requested changes Apr 10, 2020

View reviewed changes

inodb added the feature label Apr 10, 2020

inodb force-pushed the fix-6278 branch 3 times, most recently from 17ba466 to c0fa112 Compare April 10, 2020 01:45

SV: support genomic charts

889279a

inodb force-pushed the fix-6278 branch from c0fa112 to 889279a Compare April 10, 2020 01:51

inodb merged commit 303c351 into cBioPortal:master Apr 10, 2020

inodb added a commit to inodb/cbioportal-frontend that referenced this pull request Jan 12, 2022

Merge pull request cBioPortal#2890 from kalletlak/fix-6278

3fec40e

SV: support genomic charts Former-commit-id: 303c351

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SV: support genomic charts #2890

SV: support genomic charts #2890

kalletlak commented Nov 21, 2019 •

edited

kalletlak commented Jan 30, 2020

zhx828 Feb 3, 2020

kalletlak Feb 3, 2020

zhx828 Feb 4, 2020

kalletlak Feb 4, 2020 •

edited

zhx828 Feb 4, 2020

kalletlak Feb 4, 2020

jjgao left a comment

alisman Feb 10, 2020

kalletlak Feb 10, 2020

zhx828 left a comment

tmazor commented Mar 30, 2020

kalletlak commented Mar 30, 2020 •

edited

onursumer left a comment

onursumer Apr 10, 2020

onursumer Apr 10, 2020

onursumer Apr 10, 2020

onursumer Apr 10, 2020

onursumer Apr 10, 2020

onursumer Apr 10, 2020

onursumer Apr 10, 2020

inodb Apr 10, 2020

onursumer Apr 10, 2020

onursumer Apr 10, 2020

onursumer Apr 10, 2020

SV: support genomic charts #2890

SV: support genomic charts #2890

Conversation

kalletlak commented Nov 21, 2019 • edited

kalletlak commented Jan 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kalletlak Feb 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjgao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhx828 left a comment

Choose a reason for hiding this comment

tmazor commented Mar 30, 2020

kalletlak commented Mar 30, 2020 • edited

onursumer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kalletlak commented Nov 21, 2019 •

edited

kalletlak Feb 4, 2020 •

edited

kalletlak commented Mar 30, 2020 •

edited