Mutation pie chart and multi-selection table #4794

7xuanlu · 2023-12-07T18:09:46Z

This is breakdown of cBioPortal/cbioportal#6759. It covers only mutation data as part of this story.

A pie chart and a multi-selection table will be displayed.

The slices for MUTATED pie chart:
Mutated
Wild Type
NA

The data for EVENT table:
Missense_Mutation
...

Backend updates

Try adding new study-page api to get plot data instead of doing it in frontend
Support mutationDataFilter object in StudyViewFilter object
Frontend updates

Adding mutation discrete profile to gene specific charts dropdown
Main piece - plot visibility and filtering
Intersection and union selection on EVENT table
Necessary changes to show user selection
Disable Download functionality temporarily
Showing it in manage Charts
Disable group comparison is working for the time being
Changes to include it in Virtual study description and Group name

inodb · 2023-12-08T15:38:35Z

@h164654156465 is this ready for review or in progress? Charts don't seem to work yet:

7xuanlu · 2023-12-08T16:20:48Z

@h164654156465 is this ready for review or in progress? Charts don't seem to work yet:

Hi @inodb, this frontend PR needs cBioPortal/cbioportal#10395 to be able to work. It is ready for review, thanks.

inodb · 2023-12-14T15:48:04Z

@h164654156465 do you have a specific timeline for needing this out or is it ok if we review in a few weeks? We have a lot of backend movement happening with the spring boot refactoring

7xuanlu · 2023-12-18T11:53:59Z

@h164654156465 do you have a specific timeline for needing this out or is it ok if we review in a few weeks? We have a lot of backend movement happening with the spring boot refactoring

Let's review this on early to mid January next year. We're not in a hurry, but we do want to close it out as best as we can. As this PR is closely related to backend revamp. We might need to merge those changes as well. Thank you.

netlify · 2024-01-09T16:14:19Z

✅ Deploy Preview for cbioportalfrontend ready!

Name	Link
🔨 Latest commit	`62abd1e`
🔍 Latest deploy log	https://app.netlify.com/sites/cbioportalfrontend/deploys/661027ad737aa300086751f4
😎 Deploy Preview	https://deploy-preview-4794--cbioportalfrontend.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

kalletlak · 2024-01-22T17:14:21Z

Need fixes:

Filter names (remove underscores)
Bookmark links not working
Saving charts to user preference

src/pages/studyView/StudyViewPageStore.ts

alisman · 2024-02-06T20:40:38Z

src/pages/studyView/StudyViewPageStore.ts

+    }
+
+    @action.bound
+    updateMutationDataFilters(uniqueKey: string, values: string[][]): void {


can we make the name "values" more specific?

I renamed it to be valueArrays so that we know it's a two dimensional array.

alisman · 2024-02-06T20:42:05Z

src/pages/studyView/StudyViewPageStore.ts

+        if (values.every(valueArray => valueArray.length === 0)) {
+            this._mutationDataFilterSet.delete(uniqueKey);
+        } else {
+            const dataFilterValues: DataFilterValue[][] = values.map(


maybe a comment explaining why this is necessary

Comments added like so:
valueArrays represent a two-dimensional array that supports union and
intersection selection on samples

for the first condition, I also added "delete mutationDataFilter if valueArrays is empty"

src/pages/studyView/StudyViewPageStore.ts

alisman · 2024-02-06T21:21:03Z

src/pages/studyView/StudyViewPageStore.ts

+                                ...params,
+                                $queryParameters: {
+                                    projection:
+                                        chartInfo.mutationOptionType ===


what's reason for this? put comment explaining why one case needs DETAILED

There's no DETAILED condition in here, code updated

params = { ...params, $queryParameters: { projection: 'SUMMARY' }, };

alisman · 2024-02-06T21:22:42Z

src/pages/studyView/StudyViewPageStore.ts

-                                        },
-                                    ] as any,
-                                    studyViewFilter: this.filters,
+                        let result = [];


we need to avoid adding a lot of logic/bloat to the store, which already HUGE. Lets put this code inside named helper functions, which will also document it better. we need to explain the basic fork: type==MUTATION_EXTENDED or otherwise.

moved to util module

src/pages/studyView/StudyViewPageStore.ts

alisman · 2024-02-06T22:06:42Z

src/pages/studyView/StudyViewPageStore.ts

@@ -8948,6 +9225,38 @@ export class StudyViewPageStore
        } else return '';
    }

+    public async getMutationTypesDownloadData(


move to util file

I also updated similar download functions.

alisman · 2024-02-06T22:16:11Z

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

@@ -124,18 +177,59 @@ export default class GeneLevelSelection extends React.Component<
        return [];
    }

+    private get subOptions() {


no reason to make this a class member. it's a constant really

extracted to constant under the same file

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

alisman · 2024-03-01T18:09:57Z

src/pages/studyView/tabs/SummaryTab.tsx

-                    this.store.updateScatterPlotFilterByValues(
-                        props.chartMeta!.uniqueKey
-                    );
+                const props: any = {


@7xuanlu can you fix this type? you can see that is used to be set to Partial< > something other

src/pages/studyView/StudyViewPageStore.ts

alisman · 2024-03-11T16:40:31Z

src/pages/studyView/StudyViewPageStore.ts

+        getDisplayedValue?: (value: string) => string,
+        getDisplayedColor?: (value: string) => string
+    ): ClinicalDataCountSummary[] {
+        return getClinicalDataCountWithColorByClinicalDataCount(counts).map(


in the name of reducing the bloat of this file we should move this to another file. if you would rather just put a TODO we can follow up with that when do the refactor discussed a few weeks ago. i've actually already started this.

additionally, this logic should probably should have unit tests

This function is not part of this PR although I did try to move it to util file but decided not to. The thing is it's using lots of internal data such as this.chartToUsedColors in StudyViewPageStore. I prefer to delaying it to another PR that address this.

I agree that we should have unit tests once we move it to util file.

alisman · 2024-03-11T16:44:11Z

src/pages/studyView/StudyViewPageStore.ts

+                } else if (
+                    newChart.mutationOptionType &&
+                    newChart.mutationOptionType ===
+                        MutationOptionConstants.MUTATION_TYPE


correct me if i'm wrong but it seems like we use mutation_type, mutation_event and mutationData terminology interchangeably. is it possibly to make this consistent?

We make mutation_type as the final say both in frontend and backend.

let me change all occurences in frontend

alisman · 2024-03-11T17:57:55Z

src/pages/studyView/StudyViewUtils.tsx

+    var words = eventType.split('_');
+
+    // Capitalize the first letter of each word
+    var capitalizedWords = words.map(function(word) {


i think lodash has _.capitalize that does this

alisman · 2024-03-11T17:59:20Z

src/pages/studyView/StudyViewUtils.tsx

+    chartInfo: GenericAssayChart,
+    filters: StudyViewFilter
+) {
+    let result: GenericAssayDataCountItem[] = [];


why set it to empty array and then overwrite?

const result = await internalClient ...

alisman · 2024-03-11T18:01:33Z

src/pages/studyView/StudyViewUtils.tsx

+        stableId = data.stableId;
+    }
+
+    return { stableId: stableId, counts: counts };


slightly weird that it will return empty string as stableId if no matching data is found. seems like an error case yeah?

I've made it to return undefined if no data matched

alisman · 2024-03-11T18:20:09Z

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

-                        profileName: option.label,
-                    };
-                }
+            let options: MolecularProfileOption[] = this.props


try to avoid using let uncessarily.

this will be clearer and more concise

const options = this.props.xxx.result!.map

np problem modified

alisman · 2024-03-11T18:21:41Z

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

@@ -175,7 +265,8 @@ export default class GeneLevelSelection extends React.Component<
                        <div
                            style={{
                                flex: 1,
-                                marginRight: 15,
+                                width: '50%',


concerns me slightly. why would it be 50%?

I'm not comfortable with CSS but I'm aligning molecular profile dropdown with the sub profile dropdown. Feel free to change it if you have better idea.

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

alisman · 2024-03-11T18:24:30Z

src/pages/studyView/tabs/SummaryTab.tsx

+            chartMeta: ChartMeta,
+            props: Partial<IChartContainerProps>
+        ) => ({
+            [ChartTypeEnum.PIE_CHART]: () => ({


what's reason for putting these inside of callbacks?

I'm trying to make sure every subTypeProps can be created successfully and used consistently.

Some of the subtype needs callback to enable this.store.getMutationDataFiltersByUniqueKey( chartMeta.uniqueKey ) to evaluate to some values. Otherwise, it would be an undefined/error.

[ChartTypeEnum.MUTATION_EVENT_TYPE_COUNTS_TABLE]: () => ({ filters: this.store.getMutationDataFiltersByUniqueKey( chartMeta.uniqueKey ), promise: this.store.getMutationEventChartDataCount(chartMeta), onValueSelection: this.handlers.onAddMutationDataValues, onResetSelection: this.handlers.onSetMutationDataValues, id: 'mutation-type-counts-table', title: this.store.getChartTitle( ChartTypeEnum.MUTATION_EVENT_TYPE_COUNTS_TABLE, props.title ), getData: () => getMutationTypesDownloadData( this.store.getMutationEventChartDataCount(chartMeta) ), downloadTypes: ['Data'], onChangeCancerGeneFilter: this.store .updateMutatedGenesTableByCancerGenesFilter, }),

That's why I make [ChartTypeEnum.PIE_CHART] also a callback, that way we can

const subTypeProps = this.chartTypeConfig(chartMeta, props)[ chartType ]();

tmazor · 2024-03-22T18:52:24Z

I haven't have time to fully test this yet, but from what I've done so far, I have some thoughts about colors:

If I add a gene that's on the gene panel for some but not all samples, the mutated-vs-not chart is using the same color for both not mutation & not profiled:

And the more I test, it seems like these colors can change - now I have these colors:

And I can also get to the point where Mutated is blue:

Given that this is essentially yes/no/NA, I think we need to set standard colors for these to help with interpretability. I would suggest grey for 'Not Profiled' (like the color we use for N/A). Do we have default colors for binary data? If not, I'd suggest green for mutated (since that aligns with coloring in OncoPrint).

7xuanlu · 2024-03-29T14:49:40Z

I haven't have time to fully test this yet, but from what I've done so far, I have some thoughts about colors:

If I add a gene that's on the gene panel for some but not all samples, the mutated-vs-not chart is using the same color for both not mutation & not profiled:

And the more I test, it seems like these colors can change - now I have these colors:

And I can also get to the point where Mutated is blue:

Given that this is essentially yes/no/NA, I think we need to set standard colors for these to help with interpretability. I would suggest grey for 'Not Profiled' (like the color we use for N/A). Do we have default colors for binary data? If not, I'd suggest green for mutated (since that aligns with coloring in OncoPrint).

Thank you for the feedback. I think we do have default colors for binary data, let me update it as well as NOT_PROFILED

…ype pie chart

7xuanlu added the feature label Dec 7, 2023

7xuanlu requested review from alisman, jjgao and kalletlak December 7, 2023 18:09

7xuanlu self-assigned this Dec 7, 2023

7xuanlu changed the title ~~Mutation pie chart~~ Mutation pie chart and multi-selection table Dec 7, 2023

7xuanlu force-pushed the mutation-pie-chart branch from e3690e1 to c3cce60 Compare January 18, 2024 20:11

alisman reviewed Feb 6, 2024

View reviewed changes

src/pages/studyView/StudyViewPageStore.ts Outdated Show resolved Hide resolved

alisman reviewed Feb 6, 2024

View reviewed changes

src/pages/studyView/StudyViewPageStore.ts Show resolved Hide resolved

alisman reviewed Feb 6, 2024

View reviewed changes

src/pages/studyView/StudyViewPageStore.ts Outdated Show resolved Hide resolved

alisman reviewed Feb 6, 2024

View reviewed changes

src/pages/studyView/StudyViewPageStore.ts Outdated Show resolved Hide resolved

alisman reviewed Feb 6, 2024

View reviewed changes

src/pages/studyView/StudyViewPageStore.ts Outdated Show resolved Hide resolved

alisman reviewed Feb 6, 2024

View reviewed changes

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

7xuanlu closed this Feb 9, 2024

7xuanlu deleted the mutation-pie-chart branch February 9, 2024 17:42

7xuanlu restored the mutation-pie-chart branch February 9, 2024 17:57

7xuanlu reopened this Feb 9, 2024

group gene selection options by alteration type

3f8b86b

7xuanlu mentioned this pull request Feb 23, 2024

Fix Mutation Data Counts Endpoint on Mutation Tables and Rename Return Types cBioPortal/cbioportal#10665

Merged

7xuanlu and others added 6 commits February 23, 2024 13:04

Merge branch 'master' into mutation-pie-chart

dff40ac

fix type errors and conflicts

dad6c54

fix GenomicDataCount

af4b7cb

Merge branch 'master' into mutation-pie-chart

f0bbab1

fix e2e tests on SummaryTab

40aafe0

Merge branch 'master' into mutation-pie-chart

6245234

alisman reviewed Mar 1, 2024

View reviewed changes

update any to Partial in SummaryTab

3c2ec30

7xuanlu requested a review from alisman March 7, 2024 21:58

alisman reviewed Mar 11, 2024

View reviewed changes

src/pages/studyView/StudyViewPageStore.ts Show resolved Hide resolved

alisman reviewed Mar 11, 2024

View reviewed changes

src/pages/studyView/StudyViewPageStore.ts Show resolved Hide resolved

alisman reviewed Mar 11, 2024

View reviewed changes

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

alisman reviewed Mar 11, 2024

View reviewed changes

resolve Aaron's comments

fbaed5e

qlu-cls and others added 3 commits April 3, 2024 12:42

remove categories grouping and change colors for mutation pie chart

8152f81

Merge branch 'master' into mutation-pie-chart

74c1587

fix mutation categories type assertion

94f9f9c

alisman approved these changes Apr 5, 2024

View reviewed changes

revert colors.ts changes and add callback to change Mutated vs Wild T…

62abd1e

…ype pie chart

alisman merged commit 075074e into cBioPortal:master Apr 5, 2024
14 of 15 checks passed

Mutation pie chart and multi-selection table #4794

Mutation pie chart and multi-selection table #4794

Conversation

7xuanlu commented Dec 7, 2023 • edited

inodb commented Dec 8, 2023

7xuanlu commented Dec 8, 2023

inodb commented Dec 14, 2023

7xuanlu commented Dec 18, 2023

netlify bot commented Jan 9, 2024 • edited

✅ Deploy Preview for cbioportalfrontend ready!

kalletlak commented Jan 22, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alisman Mar 11, 2024 • edited

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

alisman Mar 11, 2024 • edited

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

alisman Mar 11, 2024 • edited

Choose a reason for hiding this comment

7xuanlu Mar 11, 2024 • edited

Choose a reason for hiding this comment

tmazor commented Mar 22, 2024

7xuanlu commented Mar 29, 2024

7xuanlu commented Dec 7, 2023 •

edited

netlify bot commented Jan 9, 2024 •

edited

alisman Mar 11, 2024 •

edited

alisman Mar 11, 2024 •

edited

alisman Mar 11, 2024 •

edited

7xuanlu Mar 11, 2024 •

edited