AlexsLemonade / OpenPBTA-analysis

The analysis repository for the Open Pediatric Brain Tumor Atlas Project
Other
99 stars 66 forks source link

V22 mut sigs - rerun scripts 07 and 08 #1452

Closed jharenza closed 2 years ago

jharenza commented 2 years ago

Purpose/implementation Section

What scientific question is your analysis addressing?

Updates for manuscript

What was your approach?

Updated some of the cancer group display selection code to match the updates in #1401 (staggered on #1401).

What GitHub issue does your pull request address?

1368

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

Which areas should receive a particularly close look?

The plot sizing/display now needs some tweaking, but I did not play with it due to the below comment.

Is there anything that you want to discuss further?

Do we want to select groups with a min N to display here rather than selecting specific groups? I do not remember how these were chosen.

UPDATE: I just this comment, so I will head there next.

Note that we are plotting here only the cancer groups used in the [`interaction-plots` module](https://github.com/AlexsLemonade/OpenPBTA-analysis/blob/master/analyses/interaction-plots/scripts/03-plot_interactions.R)

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

yes, please discuss above

Results

What types of results are included (e.g., table, figure)?

Updated plots

What is your summary of the results?

see above

Reproducibility Checklist

Documentation Checklist

sjspielman commented 2 years ago

Copying this conversation for reference regarding cancer groups to include - https://github.com/AlexsLemonade/OpenPBTA-analysis/pull/1453#discussion_r903715214

I'll also just note that there is going to be a full module re-run (see #1455), but we should still keep this PR to update the cancer groups going into the figures. Either way, more changes may occur when we re-run the actual signature fitting.

jharenza commented 2 years ago

Copying this conversation for reference regarding cancer groups to include - #1453 (comment)

I'll also just note that there is going to be a full module re-run (see #1455), but we should still keep this PR to update the cancer groups going into the figures. Either way, more changes may occur when we re-run the actual signature fitting.

@sjspielman do you also want to prioritize rerun of mutational sigs

jharenza commented 2 years ago

@sjspielman this one is updated with the new palette, but like you said, will need a module rerun first.

One other thing about this is that there are now 12, instead of 10, cancer groups being displayed to match #1453. As a results, most figures look OK and one with a tweak, but I want to defer to you for exposures_presence_barplot_file since that may change the layout/compilation. I think doing a 4 cols x 3 rows would be OK, but didn't want to mess with that specific figure until conferring with you.

sjspielman commented 2 years ago

@jharenza I'm working on a re-run of the module, which is taking a little more time because there are some funky things changing with hypermutators that I'm looking into FYI

sjspielman commented 2 years ago

@jharenza another update here. It turns out that there are quite a few bugs which occur with the new V22 data, a lot of which has to do with Oligodendroglioma mapping leading to some duplicates and bugs in setting factor levels. Furthermore, a lot of these problems are directly related to code in the visualizations, such that in order to fix the bugs, I also need to update and re-run the data viz itself. Therefore, even though this PR is already open, I have to fix a lot of the figures code in the module re-run I'm working on now which will make this PR likely no longer necessary. That said, we will separately also need to copy the final figure panels in figures/pdfs/, so that can be a second PR for figures on top of what I'm doing now!

jharenza commented 2 years ago

@jharenza another update here. It turns out that there are quite a few bugs which occur with the new V22 data, a lot of which has to do with Oligodendroglioma mapping leading to some duplicates and bugs in setting factor levels. Furthermore, a lot of these problems are directly related to code in the visualizations, such that in order to fix the bugs, I also need to update and re-run the data viz itself. Therefore, even though this PR is already open, I have to fix a lot of the figures code in the module re-run I'm working on now which will make this PR likely no longer necessary. That said, we will separately also need to copy the final figure panels in figures/pdfs/, so that can be a second PR for figures on top of what I'm doing now!

Ahhh, you know, I had to put in some unique()s when I was creating these plots, which I didn't have to do prior to yesterday, so I bet that is why. DO you want to me to close this then?

jharenza commented 2 years ago

closing for #1476

sjspielman commented 2 years ago

Ahhh, you know, I had to put in some unique()s when I was creating these plots

Ha! Yes I had the same journey!