Skip to content
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.

Sign up for GitHub

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

on plots tab add sorting for barcharts #4893

Closed
wants to merge 6 commits into from
Closed

on plots tab add sorting for barcharts #4893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6cfd7c7
sorting-functionality-enabled
SURAJ-SHARMA27 Apr 16, 2024
a05006d
Update PlotsTab.tsx
SURAJ-SHARMA27 Apr 16, 2024
51e085b
Merge branch 'cBioPortal:master' into sorting-functionality
SURAJ-SHARMA27 Apr 27, 2024
1c2a8f4
sorting-functionality-enhanced
SURAJ-SHARMA27 Apr 27, 2024
f92d988
enhancements and optimizations done
SURAJ-SHARMA27 Apr 30, 2024
ece5a45
sortByalphabet-fixed
SURAJ-SHARMA27 Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 58 additions & 2 deletions src/pages/groupComparison/MultipleCategoryBarPlot.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ export interface IMultipleCategoryBarPlotProps {
svgRef?: (svgContainer: SVGElement | null) => void;
pValue: number | null;
qValue: number | null;
sortOption?: string;
}
export interface TotalSumItem {
majorCategory: string;
sum: number;
minorCategory: {
name: string;
count: number;
percentage: number;
}[];
}

export interface IMultipleCategoryBarPlotData {
Expand Down Expand Up @@ -274,6 +284,7 @@ export default class MultipleCategoryBarPlot extends React.Component<
} else if (this.props.plotData) {
data = this.props.plotData;
}

return data;
}

Expand Down Expand Up @@ -424,12 +435,56 @@ export default class MultipleCategoryBarPlot extends React.Component<
}

@computed get labels() {
if (this.props.sortOption == 'sortByCount') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SURAJ-SHARMA27 this is going to actually mutate the sorting of counts in this.data. i.e. sort it in place. instead, we should use functional/immutable approach to return a sorted list of labels without mutating underlying data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed the mutable approach and created a separate totalSumArray, which does not make any changes to this.data

_.forEach(this.data, item => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm curious why you need to sort alphabetical first. you should put comments explaining each step.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was sorting the elements alphabetically first so that all the elements in each object would come in the same order, allowing me to calculate the count corresponding to each element accurately. For example:

data = {
    0: { minorCategory: 'Male', counts: Array(42) },
    1: { minorCategory: 'Female', counts: Array(42) }
}

Let's say, in the array containing 42 elements in the Male minorCategory, the first index is X, and in the Female minorCategory, the first index is Y in counts. If I calculate sum_of_total_counts = data[0].counts[0].count + data[1].counts[1].count, the sum_of_total_counts calculated is incorrect because it adds the counts for X and Y. However, if I sort it alphabetically, the counts array for each minorCategory will be in the same order, and I can find the total count easily.

item.counts.sort((a, b) =>
a.majorCategory.localeCompare(b.majorCategory)
);
});
const totalSumArray: TotalSumItem[] = [];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code is quite confusing. it would be clearer if you used _.sortBy(collection, ()=>); where the return of the callback is a _.sum of the item counts within.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is that I will traverse this.data and will make one totalSumArray array which has a schema like:

{
    majorCategory: countItem.majorCategory,
    sum: countItem.count,
    minorCategory: [
        {
            name: item.minorCategory,
            count: countItem.count,
            percentage: countItem.percentage,
        },
    ],
}

Now, I will traverse this.data and go to each majorCategory to update the totalSum in which i will store the count of each object. Then, I will sort the totalSumArray with respect to totalSum and extract the information as per the requirements like label and majorCategory.

Let's take an example. For one configuration, this.data contains:

{
    0: { minorCategory: 'Male', counts: Array(42) },
    1: { minorCategory: 'Female', counts: Array(42) }
}

I will traverse each object, go to the counts array, and create one object for like Melanoma in totalSumArray in which I will store the information and sum of counts for each object. Then, I will sort the totalSumArray according to sum and retrieve the labels from there.

_.forEach(this.data, item => {
_.forEach(item.counts, countItem => {
const existingItem = _.find(
totalSumArray,
sumItem =>
sumItem.majorCategory === countItem.majorCategory
);
if (existingItem) {
existingItem.sum += countItem.count;
existingItem.minorCategory.push({
name: item.minorCategory,
count: countItem.count,
percentage: countItem.percentage,
});
} else {
totalSumArray.push({
majorCategory: countItem.majorCategory,
sum: countItem.count,
minorCategory: [
{
name: item.minorCategory,
count: countItem.count,
percentage: countItem.percentage,
},
],
});
}
});
});

totalSumArray.sort((a, b) => b.sum - a.sum);

const sortedLabels = totalSumArray.map(item => item.majorCategory);
return sortedLabels;
}

if (this.data.length > 0) {
return sortDataByCategory(
const CategorizedData = sortDataByCategory(
this.data[0].counts.map(c => c.majorCategory),
x => x,
this.majorCategoryOrder
);
return CategorizedData;
} else {
return [];
}
Expand Down Expand Up @@ -739,7 +794,8 @@ export default class MultipleCategoryBarPlot extends React.Component<
this.categoryCoord,
!!this.props.horizontalBars,
!!this.props.stacked,
!!this.props.percentage
!!this.props.percentage,
this.props.sortOption || 'sortByAlphabet'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should probably apply this default inside the makeBarSpec

);
return barSpecs.map(spec => (
<VictoryBar
Expand Down
Loading
Loading