Updated tests and fusion containers and conda versions, general confi… #550
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
|
|
@rannick This is a POC for some changes in ci.yml, so that it doesn't just run the pipeline and it also checks the output it generates. Since I don't have access to COSMIC database, I added a section in the ci.yml file that will only run the test profiles that require this access, only when the nextflow secrets are not null. Let me know what you think! |
|
Perhaps apart from the |
Sounds good, lets wait until the other PR gets merged before moving on with this one then |
nschcolnicov
force-pushed
the
general_fixes
branch
from
bc0ad13 to
8c72976
Compare
nschcolnicov
force-pushed
the
general_fixes
branch
from
efafd5b to
3058f0c
Compare
|
Exactly, I think the tests should run with the |
|
#547 is merged |
atrigila
requested changes
Dec 3, 2024
conf/test_build.config
Outdated
| config_profile_description = 'Minimal test dataset to check pipeline function' | ||
|
|
||
| // Input data | ||
| build_references = true |
There was a problem hiding this comment.
shouldn't we have the --no_cosmic flag in test_build?
There was a problem hiding this comment.
Wasn't sure what could be considered "standard" for running the test profile, but I think you are right, i'll add it
There was a problem hiding this comment.
I am not sure if we are supposed to run this test in CI or somewhere else? If we are running the full --build_references, perhaps we should add the --all flag? so that all references are downloaded
conf/test_full.config
Outdated
There was a problem hiding this comment.
I imagine test full does not run --build_references. Therefore, the path to these references needs to be given (probably from an s3 bucket). I know we don't have it yet, but perhaps we can add a TODO for the future.
| @@ -2,8 +2,8 @@ process FUSIONINSPECTOR { | |||
| tag "$meta.id" | |||
| label 'process_high' | |||
|
|
|||
| conda "bioconda::dfam=3.7 bioconda::hmmer=3.4 bioconda::star-fusion=1.13.0 bioconda::trinity=2.15.1 bioconda::samtools=1.19.2 bioconda::star=2.7.11b" | |||
| container 'docker.io/trinityctat/starfusion:1.13.0' | |||
| conda "bioconda::dfam=3.7 bioconda::hmmer=3.4 bioconda::star-fusion=1.7.0 bioconda::trinity=2.15.2 bioconda::samtools=1.21 bioconda::star=2.7.11b" | |||
There was a problem hiding this comment.
the conda declaration could be with the environmentl.yml as we do in nf-core modules.
you can get it directly from the seqera containers :) they already provide it
| @@ -1,8 +1,8 @@ | |||
| process STARFUSION_BUILD { | |||
| tag 'star-fusion' | |||
|
|
|||
| conda "bioconda::dfam=3.7 bioconda::hmmer=3.4 bioconda::star-fusion=1.13.0 bioconda::trinity=2.15.1 bioconda::samtools=1.19.2 bioconda::star=2.7.11b" | |||
| container "docker.io/trinityctat/starfusion:1.13.0" | |||
| conda "bioconda::dfam=3.7 bioconda::hmmer=3.4 bioconda::star-fusion=1.7.0 bioconda::trinity=2.15.2 bioconda::samtools=1.21 bioconda::star=2.7.11b" | |||
There was a problem hiding this comment.
same thing about conda declaration, see above.
tests/test_build.nf.test.snap
Outdated
There was a problem hiding this comment.
I think you'll need to run this test as some content was not being generated and I fixed those bugs in previous PRs.
tests/test_build_stub.nf.test.snap
Outdated
Comment on lines
61
to
65
| "Homo_sapiens.GRCh38.102_rrna_intervals.gtf.bed:md5,d41d8cd98f00b204e9800998ecf8427e", | ||
| "Homo_sapiens.GRCh38.102.dna.primary_assembly.dict:md5,d41d8cd98f00b204e9800998ecf8427e", | ||
| "Homo_sapiens.GRCh38.102.gff3:md5,d41d8cd98f00b204e9800998ecf8427e", | ||
| "cytobands_hg38_GRCh38_v2.4.0.tsv:md5,d41d8cd98f00b204e9800998ecf8427e", | ||
| "protein_domains_hg38_GRCh38_v2.4.0.gff3:md5,d41d8cd98f00b204e9800998ecf8427e", |
There was a problem hiding this comment.
I don't know if it makes sense to snapshot md5sum as they are all the same because they are empty...
There was a problem hiding this comment.
I see what you mean and how this could get confusing after we update the test profiles. I'll remove it
tests/test_build.nf.test
Outdated
Comment on lines
12
to
17
| when { | ||
| params { | ||
| outdir = "$outputDir" | ||
| no_cosmic = true | ||
| } | ||
| } |
There was a problem hiding this comment.
I would pass the params in the profile, to be more clear, because you now have 2 sources of params:
- the test_build profile.config itself
- the nf-test
I would expect the nf-test to run exactly as the test profile, so I would add all necessary parameters in the config.
This comment applies to all nf-tests
tests/test_build_stub.nf.test
Outdated
| // All stable path name, with a relative path | ||
| stable_name, | ||
| // All files with stable contents | ||
| stable_path |
tests/test_stub.nf.test
Outdated
| // All stable path name, with a relative path | ||
| stable_name, | ||
| // All files with stable contents | ||
| stable_path |
atrigila
reviewed
Dec 4, 2024
atrigila
approved these changes
Dec 5, 2024
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.
…g, and ci.yml
PR checklist
nf-core lint).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).