-
Notifications
You must be signed in to change notification settings - Fork 677
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 Universc #1706
Add Universc #1706
Conversation
updates metadata, docker container source, licensing, and citations
unit tests and documentation
add inputs, outputs and example calls for UniverSC and Cell Ranger v3.0.2 calls versions for UniverSC and Cell Ranger
updates metadata, docker container source, licensing, and citations
unit tests and documentation
add inputs, outputs and example calls for UniverSC and Cell Ranger v3.0.2 calls versions for UniverSC and Cell Ranger
8f26297
to
74c4908
Compare
I'll share updates to the UniverSC module for feedback. It still needs tests but inputs and documentation have been updated. Note this module requires a docker container and a conda environment is not configured yet. |
Hi @TomKellyGenetics, thank you for this PR! We are merging as many modules as possible now due to and impending restructuring of the entire repo. This will mean you will need to update the module to reflect these changes before it can be merged in the future. It appears like this module isn't ready to be merged so if applicable, we are converting it to draft and adding the WIP label. If this isn't the case please let us know and we will try to get the module in before the changes. Thanks again! |
Hi @JoseEspinosa thanks for letting me know. I'm ok to mark it as Draft/WIP as it is but it is almost ready to review I think. What is the timeframe for the restructuring? Our publication is accepted at the proofing stage so it would be good timing to add the tool to nf-core at the same time. I still need to test the code but it is very similar to Cell Ranger so I wouldn't anticipate many problems there. The main reason for slow progress is I've changed jobs and I don't have much time to spend on this project anymore. I'd like to complete it anyway if possible. Thanks for your patience. |
No worries at all! 😄 |
Thanks for clarifying. I was wondering if templates would change making it problematic to use older ones. I'll try resolving the linting syntax errors and testing it. If the finishing touches are not done in time, it should be no problem to move files on my branch as necessary to avoid conflicts. |
I think the only change will be from where modules are included in the test |
Thanks for the explanation @JoseEspinosa, I'll update tests on the current version first and then pull updates and merge the next version from nf-core. As you can see above I've resolved linting issues and GitHub Actions passes on these. I've updated the test jobs on our podman node and it now starts without syntax errors I checked the following commands in the repo root directory with nextflow version 22.04.5.
Currently with local testing, the UniverSC module gives the same "OCI runtime error" (when running with podman) as the already merged Cell Ranger tests. I've pushed it to GitHub to pull to another node with rootless Docker installed. This should allow testing without this error. I built the UniverSC container with |
Singularity tests seem to fail due to insufficient disk space in the GitHub runner as the rootless Docker image is larger. I’ll remove changes to the GitHub Actions setting and share local test results to confirm current settings pass with Singularity or Podman. As already demonstrated, the updated image runs a container with “docker” profile without root access, resolving the issue discussed above. |
disables singularity for universc (image too large)
updatesI checked this version of the tomkellygenetics/universc:1.2.5.1 docker image and nf-core/modules test configurations run without errors. Docker$ export PROFILE="docker"
$ which docker
/usr/bin/docker
$ nextflow run tests/modules/nf-core/universc -entry test_universc_10x -c tests/config/nextflow.config -c tests/modules/nf-core/universc/nextflow.config
N E X T F L O W ~ version 22.10.4
Launching `tests/modules/nf-core/universc/main.nf` [evil_sax] DSL2 - revision: df4b1bc3c7
executor > local (3)
[77/3adb7e] process > test_universc_10x:CELLRANGER_MKGTF (genome.gtf) [100%] 1 of 1 ?
[56/2cd612] process > test_universc_10x:CELLRANGER_MKREF (genome.fasta) [100%] 1 of 1 ?
[7f/42d1fc] process > test_universc_10x:UNIVERSC (123) [100%] 1 of 1 ?
Completed at: 23-Jan-2023 14:01:28
Duration : 9m 17s
CPU hours : 0.3
Succeeded : 3 Podmanexport PROFILE="podman"
$ which podman
/usr/bin/podman
$ nextflow run tests/modules/nf-core/universc -entry test_universc_10x -c /home/DATA/PROJECT/20221221_nf_test_KellyST/000_DOC/010_scripts/nextflow.config -c tests/config/nextflow.config -c tests/modules/nf-core/universc/nextflow.config
N E X T F L O W ~ version 22.10.1
Launching `tests/modules/nf-core/universc/main.nf` [drunk_davinci] DSL2 - revision: df4b1bc3c7
executor > local (3)
[52/6bd688] process > test_universc_10x:CELLRANGER_MKGTF (gen... [100%] 1 of 1 ?
[e6/417c61] process > test_universc_10x:CELLRANGER_MKREF (gen... [100%] 1 of 1 ?
[93/e547a8] process > test_universc_10x:UNIVERSC (123) [100%] 1 of 1 ?
Completed at: 23-Jan-2023 14:19:24
Duration : 4m 55s
CPU hours : 0.2
Succeeded : 3 Singularity$ export PROFILE="singularity"
$ which singularity
/usr/bin/singularity
$ nextflow run tests/modules/nf-core/universc -entry test_universc_10x -c tests/config/nextflow.config -c tests/modules/nf-core/universc/nextflow.config
N E X T F L O W ~ version 22.10.4
Launching `tests/modules/nf-core/universc/main.nf` [nauseous_varahamihira] DSL2 - revision: df4b1bc3c7
executor > local (3)
[18/daae7c] process > test_universc_10x:CELLRANGER_MKGTF (genome.gtf) [100%] 1 of 1 ?
[07/5c3476] process > test_universc_10x:CELLRANGER_MKREF (genome.fasta) [100%] 1 of 1 ?
[6f/a56e41] process > test_universc_10x:UNIVERSC (123) [100%] 1 of 1 ?
Completed at: 23-Jan-2023 14:29:24
Duration : 4m 25s
CPU hours : 0.1
Succeeded : 3 summaryTherefore the docker image is ready to mirror on nfcore. Sorry for the inconvenience while testing this. Root permissions are no longer necessary. |
allows running rootless podman or singularity using --runtime crun or --writable-tmpfs apptainer/singularity#3220
…ore-modules into universc Conflicts: modules/nf-core/universc/main.nf tests/modules/nf-core/universc/nextflow.config
(pending mirrored version available)
Parameters to run singularity without "--user root" or "writable" are not supported by GitHub Actions: https://github.com/nf-core/modules/actions/runs/3983764318/jobs/6829355099 I've disabled automated testing and updated the code to call the "nfcore/universc:1.2.5.1" container. Once the mirror is set up this version should be ready to merge with all issues discussed above resolved. @emiller88 redundant tests have been removed as suggested. Thanks for @ggabernet for explaining how to handle this. @apeltzer could we get the mirrored image updated to v1.2.5.1? Thank you. |
@apeltzer Thanks for your help. Friendly reminder that this is on hold until the image is mirrored at nfcore/universc:1.2.5.1 (https://hub.docker.com/r/nfcore/universc/tags) Once this is available, I'll trigger GH Actions to confirm Docker runs without errors and ensure it is safe to merge. Thank you for your help. |
Sorry @TomKellyGenetics - been quite a bit inactive these weeks (crazy new year start here at work...) --> mirror is now done ✅ |
Conflicts: .github/workflows/pytest-workflow.yml
Thanks @apeltzer. It is understandable, I am often in the same situation. I've merged updates from the master branch. This resolves conflicts and confirms the updated module runs with the mirrored image. @emiller88 please note the above changes have been made later during discussions with other reviewers. In my opinion it is ready to merge. |
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.
Looks good to me now
As far as I can tell, Tom updated everything and made the code much nicer + two extra reviews.
* initialise template for new module: "universc" * update UniverSC module updates metadata, docker container source, licensing, and citations * update UniverSC module unit tests and documentation * update UniverSC module add inputs, outputs and example calls for UniverSC and Cell Ranger v3.0.2 calls versions for UniverSC and Cell Ranger * initialise template for new module: "universc" * update UniverSC module updates metadata, docker container source, licensing, and citations * update UniverSC module unit tests and documentation * update UniverSC module add inputs, outputs and example calls for UniverSC and Cell Ranger v3.0.2 calls versions for UniverSC and Cell Ranger * resolve formatting issues for UniverSC module * resolve linting errors for UniverSC module * fix test jobs to call UniverSC version without errors * correct configuration for UniverSC test jobs * correct linting errors for UniverSC module * correct docker build files for UniverSC * correct syntax errors in cellranger version call * prettier docs for UniverSC * add output to test data for UniverSC module * update UniverSC module to restructured repo nf-core#2141 remove files for restructed UniverSC module (avoids duplicate tests) * define separate outputs for Cell Ranger and UniverSC tests * remove TODO statements and update UniverSC meta.yml defines input and output variables and triggers automated tests * resolve minor linting issues with UniverSC * update paths to UniverSC module in test config * update container versions and tags for UniverSC module * simplifiy container configurations for UniverSC * update configuration for UniverSC tests (build Cell Ranger transcriptome reference first) * test UniverSC module with Cell Ranger references * update tests for UniverSC for restructured repository * update reference inputs for UniverSC module * set up references for cellranger OS test * resolve permissions errors for starting UniverSC * update input arguments for UniverSC and Cell Ranger OS (tests passing) * correct versions and checksums for UniverSC tests * resolves linting issues for UniverSC * resolves linting issues for UniverSC * update expected test outputs for UniverSC tests * migrate UniverSC tests to calling open source Cell Ranger uses Cell Ranger 3.0.2 OS implementation (MIT License) tests passing locally * migrate changes to source code to updated UniverSC container * update unit tests for UniverSC to correct output (using new container) * update output criteria for UniverSC unit tests * change output directory for UniverSC unit tests * test adding podman to GitHub actions (will revert if reviewers object to it) * correct test errors for Cell Ranger OS tests (UniverSC module) * array format for test checks (UniverSC and Cell Ranger OS) * remove unncessary files from UniverSC module * remove podman from automated testing * remove mentions of nf-core/universc container * call executable script from PATH in UniverSC container * migrate UniverSC module Cell Ranger OS count own directory * reorganise UniverSC submodules * update process names in UniverSC module for consistency * update formatting for UniverSC tests * update unit tests for UniverSC submodules running Cell Ranger OS 3.0.2 * reorganise UniverSC submodules to fit naming conventions * remove stub from UniverSC for testing * add universc/mkfastq to unit tests * correct syntax in cellranger module files * update expected output for universc and universc/count tests now consistent with cellranger/count module * correct syntax for universc/count meta.yaml (pass linting) * add stub to universc and universc/count * update unit tests for universc module * update unit tests for universc module * update unit tests for universc module * update unit tests for universc module * update universc/mkfastq test to run stub * update universc/mkfastq expected outputs when running stub * remove trailing whitespace (linting error) * restructure UniverSC main module * update configuration to run each UniverSC test once only * correct UniverSC unit test configuration * update name of tools in universc tests configuration * update expected output for Cell Ranger and UniverSC tests * updates unit tests for Cell Ranger and UniverSC uses contain for web summary HTML as suggested by @apeltzer nf-core#1706 * updates unit tests for Cell Ranger for web summary HTML * updates unit tests for Cell Ranger for web summary HTML * updates unit tests for Cell Ranger for web summary HTML to use description * correct path to Cell Ranger test output * update container options for run UniverSC with singularity runs without root priviledges in writeable container * migrate UniverSC container to mirrored image at nfcore/universc:1.2.4 adds documentation for image build configuration discussed in nf-core#1706 * remove redundant submodules from UniverSC with functions already supported by Cell Ranger module nf-core#1706 * migrate UniverSC references to generate by Cell Ranger submodules * update test configuration for universc/launch * update expected outputs for UniverSC to use Cell Ranger references * update expected outputs for UniverSC to use Cell Ranger references * remove UniverSC submodules for mkref and mkgtf (already implement in Cell Ranger module) discussed in nf-core#1706 * move universc/launch submodule to universc module * remove tests for UniverSC submodules for mkref and mkgtf (already implement in Cell Ranger module) discussed in nf-core#1706 * move tests for universc/launch submodule to universc module * migrate universc/launch submodule to universc module * update paths in unit tests from universc/launch to universc * update documentation for UniverSC module * update paths in test config from universc/launch to universc * restore cellranger module (remove changes from PR 1706) * restore cellranger module (remove changes from PR 1706) * restore cellranger module (remove changes from PR 1706) * update style of documentation to pass linting * add podman to settings and docs (passes local test) * test podman configuration * test podman configuration * restore changes to testing (removes podman discussed in nf-core#2675) * restore changes to other modules (removes cellranger discussed in nf-core#2646) * update podman settings in UniverSC docs * update podman parameters * update container version for universc to stable release 1.2.5 * remove conda tests for universc (not supported) * update container version for universc to latest release 1.2.5.1 (run tests on pushed version on personal account) * update container version for universc to use nfcore/universc:1.2.5.1 mirror * exit logic for universc module that doesn't support conda consistent with other modules exit logic for modules that dont support conda nf-core#2657 * trigger GitHub Actions test for tomkellygenetics/universc:1.2.5.1 * add log files to universc output directory (confirm running subroutines as expected) * correct UniverSC test configuration addresses singularity test issue https://github.com/nf-core/modules/actions/runs/3955706571/jobs/6774566021 * update configuration for singularity in universc tests * test running universc with singularity --fakeroot requires shadow-uidmap::newuidmap installed * update configuration for singularity in universc tests * debug GH Actions configuration for singularity in universc tests * test running singularity with —fakeroot write permissions * test singularity— * revert changes to singularity tests disables singularity for universc (image too large) * update container settings for universc allows running rootless podman or singularity using --runtime crun or --writable-tmpfs apptainer/singularity#3220 * test universc with singularity --writable-tmpfs * revert changes to singularity tests (--writable-tmpfs not supported on GH Actions) * update container settings for universc to call nfcore/universc:1.2.5.1 (pending mirrored version available) * update version in UniverSC citation --------- Co-authored-by: Simon Thomas Kelly <simonthomas.kelly@hugp.com> Co-authored-by: Gisela Gabernet <gisela.gabernet@gmail.com> Co-authored-by: TomKellyGenetics <tomkellygenetics@gmail> Co-authored-by: Alexander Peltzer <apeltzer@users.noreply.github.com>
* initialise template for new module: "universc" * update UniverSC module updates metadata, docker container source, licensing, and citations * update UniverSC module unit tests and documentation * update UniverSC module add inputs, outputs and example calls for UniverSC and Cell Ranger v3.0.2 calls versions for UniverSC and Cell Ranger * initialise template for new module: "universc" * update UniverSC module updates metadata, docker container source, licensing, and citations * update UniverSC module unit tests and documentation * update UniverSC module add inputs, outputs and example calls for UniverSC and Cell Ranger v3.0.2 calls versions for UniverSC and Cell Ranger * resolve formatting issues for UniverSC module * resolve linting errors for UniverSC module * fix test jobs to call UniverSC version without errors * correct configuration for UniverSC test jobs * correct linting errors for UniverSC module * correct docker build files for UniverSC * correct syntax errors in cellranger version call * prettier docs for UniverSC * add output to test data for UniverSC module * update UniverSC module to restructured repo nf-core/modules#2141 remove files for restructed UniverSC module (avoids duplicate tests) * define separate outputs for Cell Ranger and UniverSC tests * remove TODO statements and update UniverSC meta.yml defines input and output variables and triggers automated tests * resolve minor linting issues with UniverSC * update paths to UniverSC module in test config * update container versions and tags for UniverSC module * simplifiy container configurations for UniverSC * update configuration for UniverSC tests (build Cell Ranger transcriptome reference first) * test UniverSC module with Cell Ranger references * update tests for UniverSC for restructured repository * update reference inputs for UniverSC module * set up references for cellranger OS test * resolve permissions errors for starting UniverSC * update input arguments for UniverSC and Cell Ranger OS (tests passing) * correct versions and checksums for UniverSC tests * resolves linting issues for UniverSC * resolves linting issues for UniverSC * update expected test outputs for UniverSC tests * migrate UniverSC tests to calling open source Cell Ranger uses Cell Ranger 3.0.2 OS implementation (MIT License) tests passing locally * migrate changes to source code to updated UniverSC container * update unit tests for UniverSC to correct output (using new container) * update output criteria for UniverSC unit tests * change output directory for UniverSC unit tests * test adding podman to GitHub actions (will revert if reviewers object to it) * correct test errors for Cell Ranger OS tests (UniverSC module) * array format for test checks (UniverSC and Cell Ranger OS) * remove unncessary files from UniverSC module * remove podman from automated testing * remove mentions of nf-core/universc container * call executable script from PATH in UniverSC container * migrate UniverSC module Cell Ranger OS count own directory * reorganise UniverSC submodules * update process names in UniverSC module for consistency * update formatting for UniverSC tests * update unit tests for UniverSC submodules running Cell Ranger OS 3.0.2 * reorganise UniverSC submodules to fit naming conventions * remove stub from UniverSC for testing * add universc/mkfastq to unit tests * correct syntax in cellranger module files * update expected output for universc and universc/count tests now consistent with cellranger/count module * correct syntax for universc/count meta.yaml (pass linting) * add stub to universc and universc/count * update unit tests for universc module * update unit tests for universc module * update unit tests for universc module * update unit tests for universc module * update universc/mkfastq test to run stub * update universc/mkfastq expected outputs when running stub * remove trailing whitespace (linting error) * restructure UniverSC main module * update configuration to run each UniverSC test once only * correct UniverSC unit test configuration * update name of tools in universc tests configuration * update expected output for Cell Ranger and UniverSC tests * updates unit tests for Cell Ranger and UniverSC uses contain for web summary HTML as suggested by @apeltzer nf-core/modules#1706 * updates unit tests for Cell Ranger for web summary HTML * updates unit tests for Cell Ranger for web summary HTML * updates unit tests for Cell Ranger for web summary HTML to use description * correct path to Cell Ranger test output * update container options for run UniverSC with singularity runs without root priviledges in writeable container * migrate UniverSC container to mirrored image at nfcore/universc:1.2.4 adds documentation for image build configuration discussed in nf-core/modules#1706 * remove redundant submodules from UniverSC with functions already supported by Cell Ranger module nf-core/modules#1706 * migrate UniverSC references to generate by Cell Ranger submodules * update test configuration for universc/launch * update expected outputs for UniverSC to use Cell Ranger references * update expected outputs for UniverSC to use Cell Ranger references * remove UniverSC submodules for mkref and mkgtf (already implement in Cell Ranger module) discussed in nf-core/modules#1706 * move universc/launch submodule to universc module * remove tests for UniverSC submodules for mkref and mkgtf (already implement in Cell Ranger module) discussed in nf-core/modules#1706 * move tests for universc/launch submodule to universc module * migrate universc/launch submodule to universc module * update paths in unit tests from universc/launch to universc * update documentation for UniverSC module * update paths in test config from universc/launch to universc * restore cellranger module (remove changes from PR 1706) * restore cellranger module (remove changes from PR 1706) * restore cellranger module (remove changes from PR 1706) * update style of documentation to pass linting * add podman to settings and docs (passes local test) * test podman configuration * test podman configuration * restore changes to testing (removes podman discussed in nf-core/modules#2675) * restore changes to other modules (removes cellranger discussed in nf-core/modules#2646) * update podman settings in UniverSC docs * update podman parameters * update container version for universc to stable release 1.2.5 * remove conda tests for universc (not supported) * update container version for universc to latest release 1.2.5.1 (run tests on pushed version on personal account) * update container version for universc to use nfcore/universc:1.2.5.1 mirror * exit logic for universc module that doesn't support conda consistent with other modules exit logic for modules that dont support conda nf-core/modules#2657 * trigger GitHub Actions test for tomkellygenetics/universc:1.2.5.1 * add log files to universc output directory (confirm running subroutines as expected) * correct UniverSC test configuration addresses singularity test issue https://github.com/nf-core/modules/actions/runs/3955706571/jobs/6774566021 * update configuration for singularity in universc tests * test running universc with singularity --fakeroot requires shadow-uidmap::newuidmap installed * update configuration for singularity in universc tests * debug GH Actions configuration for singularity in universc tests * test running singularity with —fakeroot write permissions * test singularity— * revert changes to singularity tests disables singularity for universc (image too large) * update container settings for universc allows running rootless podman or singularity using --runtime crun or --writable-tmpfs apptainer/singularity#3220 * test universc with singularity --writable-tmpfs * revert changes to singularity tests (--writable-tmpfs not supported on GH Actions) * update container settings for universc to call nfcore/universc:1.2.5.1 (pending mirrored version available) * update version in UniverSC citation --------- Co-authored-by: Simon Thomas Kelly <simonthomas.kelly@hugp.com> Co-authored-by: Gisela Gabernet <gisela.gabernet@gmail.com> Co-authored-by: TomKellyGenetics <tomkellygenetics@gmail> Co-authored-by: Alexander Peltzer <apeltzer@users.noreply.github.com>
Why was this merged with linting failing? |
@maxulysse it wasn’t? A docker image version was bumped before merging but automated checks were passing again at the time of merging (as the image was mirrored on the nfcore dockerhub page). Check the above commits to verify this. Note that several commits, such as this one that failed linting were not made to this repository. They’re changes to the scRNA pipeline that referenced this PR in their commit messages. Please tell me if you find any issues that need to be addressed with this module. Currently I don’t see any so I believe this is a misunderstanding. |
Add the UniverSC single-cell pipeline
Implements a completely open-source single-cell RNA pipeline that supports > 30 technologies. Uses publicly available docker container will all dependencies installed.
✔️ near completion ✔️
PR checklist
Closes #1644
versions.yml
file.label
PROFILE=docker pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
PROFILE=singularity pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware
PROFILE=conda pytest --tag <MODULE> --symlink --keep-workflow-wd --git-aware