Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Using mapping histology groups for plotting implementation (PR 3 of 4) #918

Merged
merged 10 commits into from
Jan 22, 2021

Conversation

cansavvy
Copy link
Collaborator

@cansavvy cansavvy commented Jan 20, 2021

Purpose/implementation Section

What scientific question is your analysis addressing?

This PR addresses Step 3 and part Step 4 discussed in #897 and #898. This PR is updating modules that should now be using the palettes/histology_label_color_table.tsv table that is created by the notebook added in #899

What was your approach?

This PR is 3 of a few PRs where I am updating things in response to the table added in #898

Here's what's accomplished in this PR and what files have changes that need review (the rest are updated plots):
These scripts have all been updated to use the colors in palettes/histology_label_color_table.tsv and display_group instead of the old color palette and short_histology:

  • oncoprint-landscape/01-plot-oncoprint.R -- I re-ran these plots by calling run-oncoprint.sh
  • sample-distribution-analysis/02-multilayer-plots.R -- I re-ran these plots by running run-sample-distribution.sh
  • figures/scripts/fig1-sample-distribution.R -- I re-ran this by re-running this script itself.

Oh and one last thing I squeaked in here is a minor change to cn_status_heatmap.Rmd that @jashapiro had commented on the previous PR in this series but it got merged before I implemented it.

*Note that the palettes/histology_color_palette.tsv file itself is not deleted yet, I will do this in the last PR of this series so the other modules that use this file don't break in CI yet.

What GitHub issue does your pull request address?

Makes more progress toward addressing #898

Directions for reviewers. Tell potential reviewers what kind of feedback you are soliciting.

  • You may notice that the fourth oncoprint plot has two of the groups switched in order, though the color key is still correct. It looks like maybe oncoprint overrides factor level order based on sample size? How much do I need to go fuss with this?

  • There's a couple questions I have about sample-distrbution-analysis/02-multilayer-plots.R since I was digging around in there:

  1. There's a stray plot Rplots.pdf. I see its gitignored, but is there some reason it hangs in this folder? Can we make it go away?
  2. The plots/histology-treemap.html doesn't work on my end (something that was in the master branch before I touched anything). It just shows a black rectangle.

Is the analysis in a mature enough form that the resulting figure(s) and/or table(s) are ready for review?

Yes See below where I pasted the updated plots.

Results

Plots are different and now use the display_group histology labels and the respective hex codes instead of short_histology.

Updated oncoprint plots:

Here's what the correctly updated plots look like (except for the ones I'm having trouble with from sample-distribution-analyses):
all_participants_primary_only_goi_oncoprint
all_participants_primary_only_oncoprint
all_participants_primary-plus_goi_oncoprint
all_participants_primary-plus_oncoprint

Non-final version of sample distribution plot:

distribution_across_cancer_types_treemap.pdf

Final version of sample distribution plot:

fig1-openpbta-distribution

Reproducibility Checklist

  • The dependencies required to run the code in this pull request have been added to the project Dockerfile. -- no new packages are needed.
  • This analysis has been added to continuous integration.

Documentation Checklist

I checked the respective READMEs for these analyses and they did not require any updates for these changes.

  • This analysis module has a README and it is up to date.
  • This analysis is recorded in the table in analyses/README.md and the entry is up to date.
  • The analytical code is documented and contains comments.

@cansavvy cansavvy marked this pull request as ready for review January 21, 2021 13:54
@cansavvy cansavvy requested a review from jashapiro January 21, 2021 13:55
@cansavvy cansavvy changed the title WIP: Using mapping histology groups for plotting implementation (PR 3 of 4) Using mapping histology groups for plotting implementation (PR 3 of 4) Jan 21, 2021
@jaclyn-taroni jaclyn-taroni requested review from jaclyn-taroni and removed request for jashapiro January 22, 2021 15:09
@jaclyn-taroni
Copy link
Member

There's a stray plot Rplots.pdf. I see its gitignored, but is there some reason it hangs in this folder? Can we make it go away?

I think we would have to track down what, of the multiple plotting steps, is opening a graphics device. To me, it doesn't seem like a big enough deal to track this down.

Copy link
Member

@jaclyn-taroni jaclyn-taroni left a comment

Choose a reason for hiding this comment

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

These changes look good to me! And it looks like you have resolved @jashapiro's one comment. To address the other questions I have not commented on yet:

  • You may notice that the fourth oncoprint plot has two of the groups switched in order, though the color key is still correct. It looks like maybe oncoprint overrides factor level order based on sample size? How much do I need to go fuss with this?

This seems like something we would want to dig into when we are getting figures polished, but seems perfectly reasonable as is at the moment!

  1. The plots/histology-treemap.html doesn't work on my end (something that was in the master branch before I touched anything). It just shows a black rectangle.

🎟️ ? I'm not sure how heavily used that is, so it may be best just to remove it so we don't have to maintain it.

@cansavvy
Copy link
Collaborator Author

cansavvy commented Jan 22, 2021

This seems like something we would want to dig into when we are getting figures polished, but seems perfectly reasonable as is at the moment!

Sounds good.

  1. The plots/histology-treemap.html doesn't work on my end (something that was in the master branch before I touched anything). It just shows a black rectangle.

🎟️ ? I'm not sure how heavily used that is, so it may be best just to remove it so we don't have to maintain it.

I don't see that its used other places, so I can just remove it while we are here.

Edit: I'll make an issue I guess since we don't want to pile on more changes to this PR. This is its own issue now: #920

@jaclyn-taroni jaclyn-taroni merged commit 0ed49b6 into AlexsLemonade:master Jan 22, 2021
@cansavvy cansavvy deleted the update-histology-colors-2 branch January 22, 2021 18:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants