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.

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

Fix violin plot sample filtering and counting issues #11122

Conversation

onursumer
Copy link
Member

@onursumer onursumer commented Oct 24, 2024

Fix #11112

  • Use samples filtered without numerical clinical data filter when converting patient clinical data to sample clinical data
  • Make clinical data sample/patient ids match legacy ids, and use stable ids to count samples when internal id is null

@onursumer onursumer requested a review from alisman October 24, 2024 23:42
Copy link

@onursumer onursumer changed the title Fix a filtering issue with the violin plot patient data conversion Fix violin plot sample filtering and counting issues Oct 25, 2024
@@ -33,7 +33,7 @@ public DensityPlotData getDensityPlotData(List<ClinicalData> sampleClinicalData,
result.setBins(new ArrayList<>());

Map<String, List<ClinicalData>> clinicalDataGroupedBySampleId = sampleClinicalData.stream().
collect(Collectors.groupingBy(ClinicalData::getSampleId));
collect(Collectors.groupingBy(c -> c.getStudyId() + "_" + c.getSampleId()));
Copy link
Member Author

Choose a reason for hiding this comment

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

this may not be necessary, but since we removed the study id from the sample id for clinical data this change will keep the ids same for density plot implementation.

alisman
alisman previously approved these changes Oct 28, 2024
if (patientClinicalDataList.isEmpty()) {
combinedClinicalDataList = sampleClinicalDataList;
} else {
// fetch samples again without the numerical clinical data filter
Copy link
Contributor

Choose a reason for hiding this comment

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

@onursumer can you add to comment WHY we fetch again without clinical data filter?

Copy link
Member Author

Choose a reason for hiding this comment

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

done!

…erting patient clinical data to sample clinical data
…e ids to count samples when internal id is null
@onursumer onursumer force-pushed the fix-violin-plot-patient-data-conversion branch from c028f13 to 079919b Compare October 28, 2024 14:57
@alisman alisman merged commit 219d894 into cBioPortal:demo-rfc80-poc Oct 28, 2024
5 of 11 checks passed
@onursumer onursumer deleted the fix-violin-plot-patient-data-conversion branch October 28, 2024 18:19
haynescd pushed a commit that referenced this pull request Nov 25, 2024
* use samples filtered without numerical clinical data filter when converting patient clinical data to sample clinical data
* make clinical data sample/patient ids match legacy ids, and use stable ids to count samples when internal id is null
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants