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

Custom bins feature (rebased) #4139

Merged
merged 13 commits into from
May 25, 2022

Conversation

dianab0
Copy link
Contributor

@dianab0 dianab0 commented Jan 21, 2022

Extended custom bins menu according to rfc https://docs.google.com/document/d/10za8fgTsXInjUCJltWzAW-HHRJnpLa3IAT17W8M5a9E/edit

Added Quartiles, Median split and Generate bins options, where Generate bins allows user to define bin size and min value.

Screenshot from 2021-12-27 12-37-57

Backend PR: cBioPortal/cbioportal#9198

Same as #4102 but from my own fork.

@dianab0 dianab0 requested review from pvannierop and alisman January 21, 2022 14:03
Copy link
Contributor

@pvannierop pvannierop left a comment

Choose a reason for hiding this comment

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

A very small code improvement. However, this branch needs to be merged after acceptance of PR cBioPortal/cbioportal#9198 since this branch will contain some API changes (need to run yarn updateAPI).

src/pages/studyView/StudyViewUtils.tsx Show resolved Hide resolved
Copy link
Contributor

@pvannierop pvannierop left a comment

Choose a reason for hiding this comment

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

Thank you!

end-to-end-test/local/specs/custom-bins.spec.js Outdated Show resolved Hide resolved
src/pages/studyView/StudyViewPageStore.ts Outdated Show resolved Hide resolved
src/pages/studyView/StudyViewUtils.tsx Outdated Show resolved Hide resolved
src/pages/studyView/charts/barChart/CustomBinsModal.tsx Outdated Show resolved Hide resolved
@pvannierop pvannierop force-pushed the custom-bins-feature-rebased branch from 960d0bf to fab29f5 Compare March 21, 2022 15:15
@@ -372,6 +380,11 @@ export class StudyViewPageStore
@observable numberOfVisibleColorChooserModals = 0;
@observable userGroupColors: { [groupId: string]: string } = {};

chartsBinMethod: { [chartKey: string]: BinMethodOption } = {};
Copy link
Contributor

@adamabeshouse adamabeshouse Mar 21, 2022

Choose a reason for hiding this comment

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

I'm surprised this wouldn't be observable

Copy link
Contributor

Choose a reason for hiding this comment

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

@adamabeshouse I have refactored the use of the store for the CustomBinsModal. Now the fields are observable. You can find the changes in the last commit.

Copy link
Contributor

@adamabeshouse adamabeshouse 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, thanks for making all the requested changes

@alisman
Copy link
Collaborator

alisman commented May 6, 2022

@pvannierop i'm getting a 500 error when i select quartlies here:

https://www.cbioportal.org/study/summary?id=msk_impact_2017

I tried this with both my latest commit and also commit from prior to the changes i made.

curl 'https://www.cbioportal.org/api/clinical-data-bin-counts/fetch?dataBinMethod=STATIC'
-H 'authority: www.cbioportal.org'
-H 'accept: application/json'
-H 'accept-language: en-US,en;q=0.9'
-H 'content-type: application/json'
-H 'cookie: _ga=GA1.2.1629202144.1632427001; _ga_0JZ9C3M56S=GS1.1.1642187102.15.0.1642187102.0; _gid=GA1.2.1585942200.1651518115; amplitude_id_fef1e872c952688acd962d30aa545b9ecbioportal.org=eyJkZXZpY2VJZCI6IjljY2MxYTI5LTkzMGMtNDliNS1hMWJjLWVlMGNhYTQ1NGMwMFIiLCJ1c2VySWQiOm51bGwsIm9wdE91dCI6ZmFsc2UsInNlc3Npb25JZCI6MTY1MTY4NTA2NjYyNiwibGFzdEV2ZW50VGltZSI6MTY1MTY4NjE4NDI4MSwiZXZlbnRJZCI6MTcxLCJpZGVudGlmeUlkIjoxMywic2VxdWVuY2VOdW1iZXIiOjE4NH0=; JSESSIONID=9E2B9A442DF194D25299BEC5D7BAB69F; _gat=1'
-H 'origin: https://www.cbioportal.org'
-H 'referer: https://www.cbioportal.org/study/summary?id=msk_impact_2017'
-H 'sec-ch-ua: " Not A;Brand";v="99", "Chromium";v="101", "Google Chrome";v="101"'
-H 'sec-ch-ua-mobile: ?0'
-H 'sec-ch-ua-platform: "macOS"'
-H 'sec-fetch-dest: empty'
-H 'sec-fetch-mode: cors'
-H 'sec-fetch-site: same-origin'
-H 'user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/101.0.4951.54 Safari/537.36'
-H 'x-current-url: https://www.cbioportal.org/study/summary?id=msk_impact_2017'
--data-raw '{"attributes":[{"attributeId":"MUTATION_COUNT","disableLogScale":false,"showNA":true,"binMethod":"QUARTILE","binsGeneratorConfig":{"binSize":0,"anchorValue":0}}],"studyViewFilter":{"studyIds":["msk_impact_2017"],"alterationFilter":{"copyNumberAlterationEventTypes":{"AMP":true,"HOMDEL":true},"mutationEventTypes":{"any":true},"structuralVariants":null,"includeDriver":true,"includeVUS":true,"includeUnknownOncogenicity":true,"includeUnknownTier":true,"includeGermline":true,"includeSomatic":true,"includeUnknownStatus":true,"tiersBooleanMap":{}}}}'
--compressed

@pvannierop pvannierop force-pushed the custom-bins-feature-rebased branch 2 times, most recently from 4834589 to 4fe2527 Compare May 16, 2022 17:53
@pvannierop pvannierop force-pushed the custom-bins-feature-rebased branch 2 times, most recently from 9bd8ef0 to 1789af1 Compare May 24, 2022 13:56
@pvannierop pvannierop force-pushed the custom-bins-feature-rebased branch from 1789af1 to 9bb5bc1 Compare May 25, 2022 07:21
@alisman alisman merged commit a4aaf9f into cBioPortal:master May 25, 2022
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.

4 participants