AlexsLemonade / OpenPBTA-analysis

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

Update table generation notebook and excel files #1269

Closed sjspielman closed 2 years ago

sjspielman commented 2 years ago

Purpose/implementation Section

What scientific question is your analysis addressing?

From issue #1268 , supplementary tables for mutational signatures had been using the wrong file. The correct file is now used and results are re-generated. Several other changes were implemented in this notebook:

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

Which areas should receive a particularly close look?

NA

Is there anything that you want to discuss further?

NA

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

yes

Results

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

Updated output tables to use in manuscript

Reproducibility Checklist

Documentation Checklist

sjspielman commented 2 years ago

Question for @jaclyn-taroni during review - I am realizing this notebook is not in .circleci/config.yaml which would have caught the other file path bugs, presumably. I'm not sure if it should be in CI (e.g.,generate_figures.sh is not there), but this notebook seems good to add in anyways?

jaclyn-taroni commented 2 years ago

generate_figures.sh would need to be modified to be able to run on a machine with 8GB to work on CI and, presumably, many of the modules that get re-run in that script are already tested. To me, this scenario seems different because I expect it will run on a machine that size without issue (but I live to be wrong about such matters).

jaclyn-taroni commented 2 years ago

@sjspielman we need to use a param to be able to have this work in CI. I think we can make it more general than in CI y/n – we could instead make it the release where the default is the current release. In CI, perhaps you'd pass "testing" and if the param is "testing", construct the path to be data/<file> instead of data/<release param>/<file>