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 bug in clinical-density-plot endpoint when mixing sample and patient attributes #8884

Merged
merged 1 commit into from
Sep 14, 2021

Conversation

adamabeshouse
Copy link
Contributor

New solution always returns counts in terms of samples

Part of #8867

Copy link
Member

@Luke-Sikina Luke-Sikina left a comment

Choose a reason for hiding this comment

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

just a few small changes. looks good to me.

Comment on lines +352 to +363
List<Sample> samplesForPatient = patientToSamples.get(
d.getPatientId()
).get(d.getStudyId());
if (samplesForPatient != null) {
for (Sample s: samplesForPatient) {
ClinicalData newData = new ClinicalData();
newData.setAttrId(d.getAttrId());
newData.setPatientId(d.getPatientId());
newData.setStudyId(d.getStudyId());
newData.setAttrValue(d.getAttrValue());
newData.setSampleId(s.getStableId());
sampleClinicalDataList.add(newData);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

If you wanted to cut down on your nesting, you could do something like this:

Suggested change
List<Sample> samplesForPatient = patientToSamples.get(
d.getPatientId()
).get(d.getStudyId());
if (samplesForPatient != null) {
for (Sample s: samplesForPatient) {
ClinicalData newData = new ClinicalData();
newData.setAttrId(d.getAttrId());
newData.setPatientId(d.getPatientId());
newData.setStudyId(d.getStudyId());
newData.setAttrValue(d.getAttrValue());
newData.setSampleId(s.getStableId());
sampleClinicalDataList.add(newData);
}
}
List<Sample> samplesForPatient = patientToSamples
.get(d.getPatientId())
.getOrDefault(d.getStudyId(), new ArrayList<>());
for (Sample s: samplesForPatient) {
ClinicalData newData = new ClinicalData();
newData.setAttrId(d.getAttrId());
newData.setPatientId(d.getPatientId());
newData.setStudyId(d.getStudyId());
newData.setAttrValue(d.getAttrValue());
newData.setSampleId(s.getStableId());
sampleClinicalDataList.add(newData);
}

This is really six in one half a dozen in the other though.

…ent attributes

New solution always returns counts in terms of samples

Signed-off-by: Adam Abeshouse <abeshoua@mskcc.org>
@sonarcloud
Copy link

sonarcloud bot commented Sep 14, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@adamabeshouse adamabeshouse merged commit 252e8a6 into cBioPortal:master Sep 14, 2021
@adamabeshouse adamabeshouse deleted the sv-plots-0 branch September 14, 2021 20:03
@inodb inodb added the bug label Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants