-
Notifications
You must be signed in to change notification settings - Fork 83
Conversation
breaks_file <- file.path(output_dir, "breaks_lists.RDS") | ||
uncallable_file <- file.path( | ||
"..", "copy_number_consensus_call", "ref", | ||
"cnv_excluded_regions.bed" | ||
) | ||
|
||
# set up histologies file | ||
if ( params$base_run == 0 ){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you confirm that this runs as expected if you pass 0
as the param
? I can think of other cases where params
did not exactly behave as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean within a v21 branch or within v22 branch which should error out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while in this branch (v22 data download), running:
root@334c2bfa1312:/home/rstudio/kitematic/scripts# OPENPBTA_BASE_SUBTYPING=0 bash ../analyses/chromosomal-instability/run_breakpoint_analysis.sh
Error:
Could not find needed file(s):
../../data/pbta-histologies.tsv
Check your options and set up.
Execution halted
This is expected. I will open a new PR with a v21 run from master.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems the param works if 1 is default, but not 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant did you do functional testing by passing params = list(base_run = 0)
. I just did locally.
Co-Authored-By: ewafula <3333496+ewafula@users.noreply.github.com>
Co-Authored-By: ewafula <3333496+ewafula@users.noreply.github.com>
Co-Authored-By: ewafula <3333496+ewafula@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻 tested the logic locally
Purpose/implementation Section
What scientific question is your analysis addressing?
Run chromosomal instability
What was your approach?
This module also needed to be divided into script to run for subtyping and plot scripts to not run. I added that code to the bash script.
01-localization-of-breakpoints.Rmd
was not set up to intake either histologies file, so I added that. However, when I tried to run theOPENPBTA_BASE_SUBTYPING=1 bash ../analyses/chromosomal-instability/run_breakpoint_analysis.sh
, I get an error:So I am not sure where the logic is going wrong.
In the meantime, I changed the param to 1 inside of the Rmd and ran it, to get the analysis file, in order to continue the PRs.
What GitHub issue does your pull request address?
#1426 partial
Directions for reviewers. Tell potential reviewers what kind of feedback you are soliciting.
Which areas should receive a particularly close look?
logic for choosing histology file
Is there anything that you want to discuss further?
Also note, this 01 script requires short_histology
Is the analysis in a mature enough form that the resulting figure(s) and/or table(s) are ready for review?
Results
What types of results are included (e.g., table, figure)?
What is your summary of the results?
Reproducibility Checklist
Documentation Checklist
README
and it is up to date.analyses/README.md
and the entry is up to date.