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

Move processing pipeline to Docker #18

Merged
merged 5 commits into from
Mar 16, 2018

Conversation

jaclyn-taroni
Copy link
Collaborator

Related: #3

This pull request is moving the processing pipeline to a Docker container (docker/Dockerfile and docker/user_installed_R_packages.txt). This version of the container corresponds to jtaroni/multi-plier:v1. I've now run the isolated-cell-pop, NARES and sle-wb pipelines in the container (those file updates are included in this PR).

Also includes:

  • Small tweak to sle-wb/02-calculate_spearman_affy_norm.R
  • Updated SLE WB pipeline figure (to reflect updated BrainArray version)

Copy link

@gwaybio gwaybio left a comment

Choose a reason for hiding this comment

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

LGTM

You'll probably want to add instructions to the README of how to run the pipeline using your docker image. Also, is your dockerfile on dockerhub? You may want to consider doing that and also adding docker pull info to the readme (example: https://hub.docker.com/r/gregway/hgsc_subtypes/)

wget \
curl

RUN R install2.r --error \
Copy link

Choose a reason for hiding this comment

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

What is this line doing? What is R install2.r? Curious

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The --error flag will make it such that an error is thrown if the package installation fails (instead of a warning) (see: jsta/r-docker-tutorial#41)

@jaclyn-taroni
Copy link
Collaborator Author

Good point about the README + documentation @gwaygenomics -- was considering filing a separate PR for that, do you think it makes more sense to include it here? Also, let me know if you think it's resolves the referenced issue.

@gwaybio
Copy link

gwaybio commented Mar 16, 2018

was considering filing a separate PR for that

Up to you, I think it makes sense to do separately if that's your preference

Also, let me know if you think it's resolves the referenced issue.

Looks like it does, since both try to solve the same issue. May want to update within the issue your decision to use docker, for posterity. Feel free to close

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.

2 participants