-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update R #242
Conversation
Codecov Report
@@ Coverage Diff @@
## master #242 +/- ##
=======================================
Coverage 76.55% 76.55%
=======================================
Files 19 19
Lines 1557 1557
=======================================
Hits 1192 1192
Misses 365 365 Continue to review full report at Codecov.
|
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 have some questions because I'm curious about a few things, but I tested everything I could think of and everything worked as expected. The platform locally feels snappier (when starting).
I think it's good to go!
liblzma-dev | ||
libbz2-dev |
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.
Why are we adding these? they are compression libraries right?
RUN R -e "install.packages('remotes', repos = c(CRAN = 'https://cloud.r-project.org'))" && \ | ||
R -e "remotes::install_github('rstudio/[email protected]')" | ||
R -e "remotes::install_github('rstudio/[email protected]')" && \ | ||
R -e "renv::init(bare = TRUE, settings = list(use.cache = FALSE))" |
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.
why do we not use the renv cache? I suppose when building the image there is no cache available. but what happens when rebuilding locally?
R -e 'root <- renv::paths$root(); unlink(root, recursive = TRUE)' && \ | ||
strip --strip-debug $RENV_LIB/*/libs/*.so | ||
|
||
RUN R -e "renv::deactivate()" |
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.
why is this necessary?
pip install PyYAML==3.10 && \ | ||
pip install awscli==1.25.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.
why do need pyYAML? is it a dep of awscli?
RUN echo "python3-pip" >> sysdeps_run.txt && \ | ||
echo "wget" >> sysdeps_run.txt && \ | ||
cat sysdeps_run.txt | xargs ./install_debian_packages.sh && \ | ||
rm -rf * |
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 do not understand this docker magic. what does this remove?
ENTRYPOINT ["Rscript", "init.R"] | ||
|
||
|
||
# --------------------------------------------------- | ||
# DEVELOPMENT BUILD | ||
# --------------------------------------------------- | ||
FROM builder AS dev | ||
FROM common AS dev | ||
|
||
# install Radian for interactive R shell | ||
# also install watchdog to automatically restart | ||
# when source files change | ||
RUN pip install -U jedi radian PyYAML watchdog[watchmedo] |
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.
isn't pyYAML already installed?
Description
Update R to 4.2 using included script to minimize package updates as much as possible.
https://ui-alex-upgrade-r-p242-w274.scp-staging.biomage.net/
Notes:
Seurat
to makeRunUMAP(..., umap.method = 'umap-learn')
reproduciblerenv
download cacher-hub/r-minimal
but I couldn't getBiocParallel
to compile.BLAS/LAPACK
: seems to be working correctly withSeurat::FindIntegrationAnchors
Image sizes differences:
Worker (↓ ~ 50%):
Pipeline (↓ ~ 40%):
Details
URL to issue
https://biomage.atlassian.net/browse/BIOMAGE-1949
Link to staging deployment URL (or set N/A)
https://ui-alex-upgrade-r-p242-w274.scp-staging.biomage.net/
Links to any PRs or resources related to this PR
hms-dbmi-cellenics/worker#274
Integration test branch
master
Merge checklist
Your changes will be ready for merging after all of the steps below have been completed.
Code updates
Have best practices and ongoing refactors being observed in this PR
Manual/unit testing
Integration testing
You must check the box below to run integration tests on the latest commit on your PR branch.
Integration tests have to pass before the PR can be merged. Without checking the box, your PR
will not pass the required status checks for merging.
Documentation updates
Optional