Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add analysis script #13

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add analysis script #13

wants to merge 9 commits into from

Conversation

gadenbuie
Copy link
Member

This PR extracts the key elements from the Shiny app into an RScript template. The input values of the app are written into the template before downloading (just the relevant inputs). The script is as self-contained as possible - I used pacman to automatically install missing packages on load and data is downloaded from online sources (including our repo for the HiC contact matrix).

@jhcreed The script does need some attention both to make the variable names more descriptive and to add comments and notes that would be relevant to an end user.

@tgerke
Copy link
Member

tgerke commented Jan 2, 2019

Nice feature for users wanting to tweak (I wish all Shiny apps I use came with this!); script looks great. I was able to successfully step through it and generate the plot.

  • In header section, near "Generated on..." would it be helpful to include the bookmark link for the query as well?
  • The as.numeric operations on tad$chr and lad$chr throw warnings (NAs by coercion). Pretty certain these are harmless, but best to anticipate/eliminate if possible
  • Agree on better naming descriptions, section commenting, and perhaps even some order reorganizing for clarity (e.g. the plot parameters might most helpfully appear just before the plot is defined, rather than at the top)

dat <- dat[dat$rsID %in% snps, ]
snp_pos <- dat$pos_hg38
tad <- tad[tad$chr == max(dat$chr, na.rm = TRUE), ]
tad[tad$start_position <= snp_pos & tad$end_position >= snp_pos, ]
Copy link
Member Author

Choose a reason for hiding this comment

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

@jhcreed This line (and the similar line in in_lad() throw warnings when snp_pos has length > 1 (both in script and app). When length(snp_pos) > 1, I think only the first element is used, so in_tad() only checks the first SNP. You may want to investigate to verify that this is the case or the desired behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants