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

Implement custom-data filtering #10827

Merged

Conversation

gblaih
Copy link
Contributor

@gblaih gblaih commented Jun 11, 2024

No description provided.

@gblaih gblaih changed the base branch from master to demo-rfc80-poc June 11, 2024 16:48
@gblaih gblaih force-pushed the demo-rfc80-poc-custom-data-bin-counts branch from 9e89e1d to 462315d Compare July 3, 2024 21:43
List<String> studyIds = new ArrayList<>();
List<String> sampleIds = new ArrayList<>();
studyViewFilterUtil.extractStudyAndSampleIds(studyViewFilter.getSampleIdentifiers(), studyIds, sampleIds);
sampleIdentifiers = sampleService.fetchSamples(studyIds, sampleIds, Projection.ID.name()).stream()
Copy link
Contributor

Choose a reason for hiding this comment

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

is this already handled in clickhouse and will result be same if we just wait to apply it there

@gblaih gblaih force-pushed the demo-rfc80-poc-custom-data-bin-counts branch from eae97c9 to de7677a Compare July 30, 2024 15:35
@gblaih gblaih force-pushed the demo-rfc80-poc-custom-data-bin-counts branch from 21cb701 to 694f149 Compare August 7, 2024 19:13
@gblaih gblaih marked this pull request as ready for review August 7, 2024 19:13
@@ -419,5 +419,37 @@ public ResponseEntity<SampleTreatmentReport> fetchSampleTreatmentCounts(
return new ResponseEntity<>(studyViewColumnarService.getSampleTreatmentReport(interceptedStudyViewFilter),
HttpStatus.OK);
}


Copy link
Contributor

Choose a reason for hiding this comment

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

@gblaih should this be commented out?

@@ -66,6 +66,45 @@
'${sampleIdentifier.studyId}_${sampleIdentifier.sampleId}'
</foreach>
</if>
<if test="studyViewFilter.customDataFilters != null and !studyViewFilter.customDataFilters.isEmpty() and customDataSamples != null">
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm not sure i understand how this customDataFilters collection (which seems to be a member of the studyViewFilter) maps back to the extracted customDataFilters which has been added as an extra parameter to the mapper methods.

@alisman
Copy link
Contributor

alisman commented Aug 9, 2024

@haynescd @gblaih, as I discussed with Bryan, I would like to pursue a way to avoid passing customDataSamples through to all the methods. This strikes me as something that should be dealt with using interception.

I prospose the following: we change the StudyViewFilter type to add a collection of custom data. The intereceptor invokes extractCustomDataSamples and populates this collection with the result. It will then automatically be available in the mappers without having to be passed through.

We might want to consider extending or wrapping StudyViewFilter if we feel that this collection of customData is not native to a "filter object".

@haynescd haynescd force-pushed the demo-rfc80-poc-custom-data-bin-counts branch from 4da8b3b to fb3da66 Compare August 14, 2024 18:23
@haynescd haynescd force-pushed the demo-rfc80-poc-custom-data-bin-counts branch from 04a6427 to fa3c45d Compare August 20, 2024 17:23
Copy link

sonarcloud bot commented Aug 20, 2024

@haynescd haynescd requested a review from alisman August 21, 2024 16:08
@alisman alisman merged commit 05de8ff into cBioPortal:demo-rfc80-poc Aug 21, 2024
10 of 13 checks passed
@gblaih gblaih changed the title Implement custom-data-bin-counts endpoint Implement custom-data filtering Aug 28, 2024
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.

3 participants