Allow numeric data type for custom data charts

[TJMKuijpers]

Pull Request frontend implementation of custom numerical data

This pull request adds the functionality of uploading custom numerical data to cBioPortal via the add custom graph option. In the original frontend code of cBioPortal, it is only possible to upload custom categorical data. Here, we implemented the option to upload numerical data. (Note this PR cannot be merged before Binning of custom data #10039 is merged).

New functionality

• Modal to upload custom data is extended with a radio button to select categorical/numerical data type.
• Group comparison is implemented for the numerical custom data
• Users can change the binning method based on quartile / median/ bin generator or custom bins

Screenshot test

• Screenshot test added to test if the numerical/categorical radio button is present (core functionality needed in the frontend)

Screenshots

This also allows to have numerical custom data after we query based on genes (custom data 2 in the image):

Notify reviewers

Read our Pull request merging
policy. It can help to figure out who worked on the
file before you. Please use git blame <filename> to determine that
and notify them either through slack or by assigning them as a reviewer on the PR

[alisman]

can we make "Custom_Data" into a constant somewhere?

[TJMKuijpers]

I think we can make use of the following:

if (  this.store.isUserDefinedCustomDataChart(
       chartMeta.uniqueKey
    )

In case of ChartTypeEnum.BAR_CHART and isUserDefinedCustomDataChart are both true, the custom data has a numerical type.

In other cases, we can make use of ChartMetaDataTypeEnum.CUSTOM_DATA

[alisman]

https://www.typescriptlang.org/docs/handbook/enums.html#enums-at-compile-time
this should allow you to avoid this duplication

[alisman]

this logic seems kind of painful. i would just do

// default to true
return filter.showNA === undefined ? true : filter.showNA;

[TJMKuijpers]

Updated to :

            const filter = this._customDataBinFilterSet.get(uniqueKey);
            return filter === undefined ? true : (filter!.showNA as boolean);

[alisman]

lets move all this logic into a tested helper function that accepts chartMeta and sampleKeyToData as args

[TJMKuijpers]

This has been moved to the function transformSampleDataToSelectedSampleClinicalData in StudyViewUtils.tsx with a test in StudyViewUtils.spec.tsx.

[alisman]

try to avoid using for loops. this is a mapping operation so it should be

const merged = selectedSamples.map(sample=>{...});

[alisman]

why would uniqueSampleKey ever be undefined?

if there is a valid reason, this filter operation can be chained with the map above.

const mergedWhatevers = arr.map(...)
    .filter(...);

[TJMKuijpers]

The uniqueSampleKey is undefined in the selectedSampleData when the sample is not in selectedSamples. When we filter based on the uniqueSampleKey, we only send the selected samples in the studyview filter to the group comparison. (I think an inner join would have been better)

[alisman]

also, we should almost never use the any type in our logic. i see that the existing promise above does use any. we should try to improve this. maybe we can meet about it

[TJMKuijpers]

We can define a new type comparisonCustomData with the following code to generate the selectedSampleData:

const selectedSampleData: comparisonCustomData[] = 
sampleInformation.map(
              sample =>
                ({ ...sample,                                       
                    ...selectedSamples.find(                                                   
                                           itmInner =>                                                       
                                           itmInner.sampleId ===                                                        
                                           sample.sampleId),
                } as comparisonCustomData)

and:

data = selectedSampleData.map(item => {
                     return {
                            clinicalAttribute: clinicalAttribute,
                            clinicalAttributeId:
                                              clinicalAttribute.clinicalAttributeId,
                            ...item,
                                        } as ClinicalData;
                                    })

[alisman]

seems strange that we update two different things here. lets discuss. at very least lets put some comments explaining why two different things need to be set.

[TJMKuijpers]

This now is implemented in getCustomDataNumerical(). When the user uploads a custom data set, we don't know the data bin values beforehand (unlike a custom categorical data where we know the categories). After obtaining the binned data from the API, we need to set the bins in the filter.

[alisman]

i see below that we have a method
setCustomChartFilters
if this is in fact categorical, we should change the name to avoid confusion with the new numerical option

[TJMKuijpers]

I renamed setCustomChartFilters to setCustomChartCategoricalFilters. This filter takes input values (string[]) to filter categorical data. The setCustomChartNumericalFilters takes dataBins (DataBin[]) as input. It makes sense to have a more explicit name for the custom categorical data filter.

[inodb]

@TJMKuijpers Great work! Could you help fix the conflict? I think it's good to go otherwise

[TJMKuijpers]

@inodb, resolved the conflict.

[inodb]

@TJMKuijpers there are some updated screenshots. The barcharts now show "NA" in text rather than as a bar (maybe that's the default we want but just want to make sure - if the change is desired, the screenshot needs to be updated):

https://output.circle-artifacts.com/output/job/15424c4c-7e39-40cd-bf65-4c63a78bdfea/artifacts/0/imageCompare.html