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

Genomic and Generic Assay bin counts endpoints #10964

Conversation

dippindots
Copy link
Member

@dippindots dippindots commented Aug 29, 2024

What's inside this pull request:

  • Genomic Data bin counts endpoint implementation
    • introduced a new table genetic_alteration_numerical_derived, we should be able to merge this one with genetic_alteration_cna_derived later, these two tables has similar structure, only one column' type is different
    • endpoint and filter implemented
  • Generic Assay Data bin counts endpoint implementation
    • introduced a new table generic_assay_data_derived
    • endpoint and filter implemented
  • Generic Assay Categorical filter (study view filter part to support Generic assay counts #10914)

TODO List:

  • (TODO) addressing NA counts issues (NA counts are missing from both genomic-data-bin-counts endpoint and generic-assay-bin-counts because of the way we store those data. We are storing data as a sampl)
  • (TODO) adding tests to these endpoints
  • (TODO) refactoring some class names and function names to make them more generic instead of sounding like it's for clinical data only (since lots of logic has started to be shared between services)

Related frontend:
cBioPortal/cbioportal-frontend#4988

@dippindots dippindots marked this pull request as draft August 29, 2024 14:13
@dippindots dippindots force-pushed the genomic-and-generic-assay-bin-counts-endpoints-clickhouse-rebase branch from 692f34c to dcaa432 Compare August 29, 2024 14:17
@dippindots dippindots force-pushed the genomic-and-generic-assay-bin-counts-endpoints-clickhouse-rebase branch from 99f7e96 to 415cb1a Compare August 29, 2024 15:43
@haynescd
Copy link
Collaborator

@dippindots Can you address the Sonar issues

Copy link
Collaborator

@haynescd haynescd left a comment

Choose a reason for hiding this comment

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

Looks good. Just some small changes

@dippindots
Copy link
Member Author

@haynescd Thanks for the review, I will make changes accordingly, I looked at the sonarcloud test, but I can't find related errors, so I re-run the test again and it's passing. Should I still worry about it?

@haynescd
Copy link
Collaborator

@haynescd Thanks for the review, I will make changes accordingly, I looked at the sonarcloud test, but I can't find related errors, so I re-run the test again and it's passing. Should I still worry about it?

I am talking about the issues the Sonar Cloud found here

@dippindots dippindots marked this pull request as ready for review August 30, 2024 17:23
@dippindots dippindots added the api label Aug 30, 2024
@haynescd haynescd force-pushed the genomic-and-generic-assay-bin-counts-endpoints-clickhouse-rebase branch from 030e7f1 to b204683 Compare September 10, 2024 15:06
@@ -206,7 +206,8 @@ CREATE TABLE genetic_profile
description String,
show_profile_in_analysis_tab Int32,
generic_assay_type Nullable(String),
cancer_study_id Int32
cancer_study_id Int32,
patient_level Nullable(Int32),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to be nullable? In clickhouse it creates a separate Nullable column to handle this? Just want to make sure because it will increase the data foot print

@dippindots dippindots force-pushed the genomic-and-generic-assay-bin-counts-endpoints-clickhouse-rebase branch from 36916d7 to 8c0f82b Compare September 11, 2024 18:18
Copy link

sonarcloud bot commented Sep 11, 2024

@alisman alisman merged commit 10c4a87 into cBioPortal:demo-rfc80-poc Sep 11, 2024
11 of 13 checks passed
@dippindots dippindots deleted the genomic-and-generic-assay-bin-counts-endpoints-clickhouse-rebase branch September 12, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants