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

Update snv-callers to be able to be run from places besides the root of repo #1420

Merged

Conversation

jaclyn-taroni
Copy link
Member

⚠️ Review #1419 first! Unfortunately, I can't stack these in earnest because I'm using a fork!

Closes #1406.

The shell scripts in snv-callers made an assumption that you'd run it from the root of the repository. That will be a problem when the script for generating analysis files (run from scripts/!!) is including in an upcoming PR. This PR introduces the following changes to the snv-callers module:

  • Set the working directories of the PBTA and TCGA shell scripts to always be where the script is located, and update the paths in those scripts accordingly.
  • Remove the assumption that input files will have paths relative to the root of the repository in the Rscripts analyses/snv-callers/scripts/02-merge_callers.R and analyses/snv-callers/scripts/03-calculate_tmb.R. We still need to make use of rprojroot, however, to make sure we're able to source() appropriately.

@jaclyn-taroni jaclyn-taroni requested a review from sjspielman May 19, 2022 15:32
@jaclyn-taroni
Copy link
Member Author

Okay, two updates since filing:

  • 3b40a91 removes short_histology from the TMB file. It's not necessary and we'd rather have folks joining short_histology from the appropriate histologies file, rather than using that one. I'm not 100% sure if all the downstream ramifications are going to get caught until we have v22 CI data with TMB files that do not contain short_histology to be frank!
  • 4e53522 adds functionality to use the base histologies file when running for a data release (related to Update logic in modules where analysis files included in the data releases are generated #1419!)

# The sqlite database made from the callers will be called:
dbfile=scratch/snv_db.sqlite
dbfile=../../scratch/snv_db.sqlite
Copy link
Member

Choose a reason for hiding this comment

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

When we made the S2 figures, this path got written in during review. To my knowledge figures are the only thing that needs to use these databases after this module. So, we can either revert back the figures path, or save the databases there in the first place.

scratch_dir <- file.path(root_dir, "scratch", "snv-callers-panels")

Copy link
Member Author

Choose a reason for hiding this comment

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

I split the difference (kind of) in 054616b. It's a good call for modules to have their own directories in scratch/, rather than just hoping their are no file name collisions in scratch/! But I also think snv-callers is more appropriate than snv-callers-panels.

# The sqlite database made from the callers will be called:
dbfile=scratch/tcga_snv_db.sqlite
dbfile=../../scratch/tcga_snv_db.sqlite
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as before for the scratch path.

@sjspielman sjspielman self-requested a review May 23, 2022 13:25
Copy link
Member

@sjspielman sjspielman left a comment

Choose a reason for hiding this comment

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

👍

@jaclyn-taroni jaclyn-taroni merged commit 0a08cd4 into AlexsLemonade:master May 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the shell scripts in snv-callers to have a consistent working directory
3 participants