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

Volcano plot part1 #753

Merged
merged 63 commits into from
Jan 27, 2016
Merged

Volcano plot part1 #753

merged 63 commits into from
Jan 27, 2016

Conversation

aderidder
Copy link
Contributor

Volcano Plot (ploty) + Mini-onco functionality added to the Enrichtments tab's subtabs

@yichaoS since you're the author of the over representation analysis view and also working with plotly, are you willing to do the code review?

pieter added 30 commits November 9, 2015 11:17
allow selections via plot and/or via datatable filters; added special
checkbox selection scenario (checkbox selected genes); cleaned up old
part of code that is not used
style.opacity, style.selection_mode and style.special_select_color
allow selections via plot and/or via datatable filters; added special
checkbox selection scenario (checkbox selected genes); cleaned up old
part of code that is not used
Sander de Ridder added 9 commits January 4, 2016 16:20
added invisible markers to improve text layout (workaround)
- improved the positioning and margins for the scatterplot
- set the font family to the one also used by the data table
- moved table searching to views.js
- updated the search function to also show a loading icon for the table
- fixed a bug which overwrote default settings for the axis instead of deep-copying them
The extra axis labels are different for mRNA expression and protein
Switched to the newest version of plotly; removed our drag-functionality and changed to the provided drag-functionality
Mini-onco functionality only available in mutations and cna
Added a mouse-over icon to the mini-onco and made some layout improvements
GeneticProfiles now also returns the datatype
Plot subtabs will now only be generated if datatype is not z-score or id doesn't contain z-score
Removed Cytoband from the tables to save space
…he positioning of the scatterplot and tables when very few elements were selected (CNA)
@jjgao jjgao added this to the 1.1.0 milestone Jan 14, 2016
@jjgao jjgao deployed to cbioportal-pr-753 January 14, 2016 20:45 Active
@yichaoS
Copy link
Contributor

yichaoS commented Jan 19, 2016

Some suggestions:

@aderidder I tried the box selection to select a subset of the dots, but I couldn't figure out a good way to deselect it except refresh the page all together; and I don't find any reset button for that. Maybe a more convenient UI design for this part?

@aderidder maybe we can make it more explicit when click on a gene in the table, some bar charts will pop up? I found that feature by accident....otherwise I wouldn't notice. Maybe by default selecting the first gene and show the bar chart from the very beginning?

@jjgao Do you think maybe we could just get rid of q-value? And we can get rid of "tendency towards" and just say "co-occurrence"/"mutual exclusive"? Just this page is getting really crowded.

…ould select a gene.

Support for opacity in stacked-histogram
@jjgao jjgao deployed to cbioportal-pr-753 January 21, 2016 12:54 Active
@aderidder
Copy link
Contributor Author

Hi @yichaoS thanks for the feedback

Concerning your first point: yes you're right. Unfortunately I haven't been able to find a good way to do this in the current plotly version. I you'd like more details, let me know and I'll elaborate. I've created an issue for this in the plotly github (plotly/plotly.js#181).

Concerning the second issue: I've made some changes and now the mini-onco is visible straight away with the second bar suggesting that the user can select a gene in the table or in the plot. The new version is available on our test server.

Thanks,
Sander

@jjgao
Copy link
Member

jjgao commented Jan 22, 2016

@aderidder very nice work!

About Yichao's first point, I think double click on the plot reset the selection, but the table is not updated.

It's good idea to remove "tendency towards" in the last column. Let's still keep the q-value there now.

@aderidder
Copy link
Contributor Author

Hi @jjgao thanks, glad you like it!
We've come up with a workaround / fix for the double-click issue. New version is on our test-server.

@jjgao jjgao deployed to cbioportal-pr-753 January 25, 2016 10:14 Active
@jjgao jjgao deployed to cbioportal-pr-753 January 25, 2016 12:48 Active
@jjgao
Copy link
Member

jjgao commented Jan 26, 2016

@yichaoS do you have any other comments? If not, I'll move it to beta testing.

@yichaoS
Copy link
Contributor

yichaoS commented Jan 26, 2016

@jjgao I think it looks good.

jjgao added a commit that referenced this pull request Jan 27, 2016
@jjgao jjgao merged commit 309aa58 into cBioPortal:rc Jan 27, 2016
@jjgao jjgao removed the in progress label Jan 27, 2016
*/
this.hasLogData = function(){
return data_type === "LOG-VALUE";
}
Copy link
Member

Choose a reason for hiding this comment

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

@jjgao : be aware this could be a pitfall! You need to check if all the relevant genetic profiles on your server have the data type correctly set to LOG-VALUE, otherwise they will not be selected here!

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