Add rworkflows #105
Closed
Add rworkflows #105
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rcannood
requested changes
Sep 18, 2023
.github/workflows/R-CMD-check.yaml
Outdated
Comment on lines
43
to
54
| - name: Install Python dependencies for reticulate | ||
| run: | | ||
| reticulate::install_miniconda() | ||
| reticulate::py_install(c("anndata", "scanpy"), pip = TRUE) | ||
| shell: Rscript {0} |
There was a problem hiding this comment.
Will neurogenomics/rworkflows@master also install these packages?
There was a problem hiding this comment.
Just to log our in-person discussion here, we talked about strategies for installing miniconda/anndata/scanpy.
Here are some of the proposed options:
- Trigger the
setup_condafunction whenever aAnnDataRfunction that requiresminiconda/anndata/scanpyis used. We use this strategy for non-R dependencies inMAGMA.Celltyping, which requires the command line toolMAGMAfor certain functions. The goal in that case is to remove any manual setup steps for the user. However, in the case of GHA, the only place that this would currently be triggered withinAnnDataRis the unit tests, which @rcannood was opposed to. From a philosophical standpoint I have less of an issue with additional installs being triggered post-AnnDataRinstallation, but I do have some practical concerns that R might have some trouble using thereticulateconda env right after it's been created. For this reasonbasiliskmight be a better option, which switches between envs much more smoothly. - Add an optional step to rworkflows action to setup a conda env (I've actually been meaning to add this feature since its inception). Might be the best option tho, and shouldn't be too hard to implement (famous last words)
- On GHA, skip any tests that require
miniconda/anndata/scanpy. Not a very good option, since it creates a huge blindspot during testing. - Revert to the original GHA workflows and remove
rworkflows. Easy short-term solution, but less so for the long-term.
|
Can you merge the main branch into this PR? |
rcannood
requested changes
Nov 29, 2023
I was a bit too optimistic about my other PR and thought that would become "main" quite soon. Seems like those changes are going to take a while to break up and review, so i can change this PR back. |
|
Closing this PR for now |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
anndataR 0.99.0
New features
rworkflowsCI.actions/checkout@v4(which has less issues).setup_condaautomatically installs minicondaand sets up conda env: Automatically setup conda env #97