-
Notifications
You must be signed in to change notification settings - Fork 719
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
Bump kallistobustools to 0.28.2 #4981
Conversation
Note that is a placeholder until the |
The upstream conda distributable has been rebuilt on the relevant repositories; the Docker container (etc.) is now functional. |
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.
Great work! At the moment we are trying to migrate modules from pytest to nf-test (https://nf-co.re/docs/contributing/modules#migrating-from-pytest-to-nf-test) would you be able to follow this guide and add nf-test?
I appreciate it! Unfortunately, at this time, I do not think I am comfortable enough with the pipeline to convert the pytest checks to nf-test, and would prefer to handle that in a future PR. |
I think there is a test on the |
@gennadyFauna I went through and added the nf-tests if you are interested in taking a look! |
Here is a brief summary of how I went about this:
Totally okay if this doesn't make sense yet, I just wanted to explain the changes I made and hopefully provide an intro to how nf-tests work! |
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.
Great, another nf-test migration 🎉 Check my comments and update if you find necessary.
process.out.intron_t2c, | ||
).match() | ||
}, | ||
{ assert file(process.out.index.get(0)).exists() } |
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.
Cannot check for file content for these index files?
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 think we probably could, but I honestly didn't look into it too much haha. I think you're right so I'll see what I can do before merging
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.
Would make it slightly more robust. Checking for existence is my last resort when md5 or file content checks are impossible (e.g. a binary file with changing content due to timestamp or something).
Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
@gennadyFauna In short, the changes that @jasmezz made to the The great benefit of this is that you can very quickly make sure that your inputs/outputs chain together correctly without actually running all of the tools. |
@jasmezz The index is in some compressed format and I couldn't quickly find out which. Merging for now get this in for @gennadyFauna, but definitely something to look into in the future |
* bump version, add new options from 0.28.2 * waiting on conda * style * Update environment.yml * workflow mode is reqd * editing tests: the quantification logic has changed * fixing licenses * workflow should be specified explicitly * container fixed at bioconda * incorrectly recorded md5s * Added nf-tests for kallistobustools modules * Delete modules/nf-core/kallistobustools/ref/tests/nextflow.config * Added kallistobustools/ref tag to kallistobustools/count test * Don't include path lines in count test * Apply suggestions from code review Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com> --------- Co-authored-by: Carson J Miller <carsonjmiller@outlook.com> Co-authored-by: Carson J Miller <68351153+CarsonJM@users.noreply.github.com> Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
* bump version, add new options from 0.28.2 * waiting on conda * style * Update environment.yml * workflow mode is reqd * editing tests: the quantification logic has changed * fixing licenses * workflow should be specified explicitly * container fixed at bioconda * incorrectly recorded md5s * Added nf-tests for kallistobustools modules * Delete modules/nf-core/kallistobustools/ref/tests/nextflow.config * Added kallistobustools/ref tag to kallistobustools/count test * Don't include path lines in count test * Apply suggestions from code review Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com> --------- Co-authored-by: Carson J Miller <carsonjmiller@outlook.com> Co-authored-by: Carson J Miller <68351153+CarsonJM@users.noreply.github.com> Co-authored-by: Jasmin Frangenberg <73216762+jasmezz@users.noreply.github.com>
Changes:
nac
workflow.main.nf
forref
andcount
to match nomenclature inmeta.yml
files:intron
instead ofintrons
count
to matchref
and take in aworkflow_mode
variable. This fixes a bug: the results of the, e.g.,lamanno
pipeline are incorrect if the workflow is not specified;kb
generates one matrix instead of two and ignores the intronic content.nucleus
option and adding thenac
one, and passing the workflow variable directly intocount
.PR checklist
Closes #XXX
versions.yml
file.label
nf-core modules test <MODULE> --profile docker
nf-core modules test <MODULE> --profile singularity
nf-core modules test <MODULE> --profile conda
nf-core subworkflows test <SUBWORKFLOW> --profile docker
nf-core subworkflows test <SUBWORKFLOW> --profile singularity
nf-core subworkflows test <SUBWORKFLOW> --profile conda