-
Notifications
You must be signed in to change notification settings - Fork 10
Nf core template merge 3.3.2 #159
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
Conversation
|
5648a98 to
6fd50eb
Compare
707f034 to
db78bce
Compare
|
Looks good to me 🙂 👍 |
skrakau
left a comment
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.
Thanks a lot for updating this! I would have a two question or remarks:
- could you squash your commits at the end? that would keep the commit history clean as there are multiple back-and-forth changes that would be confusing otherwise
- why did you replace the snapshots from the
peptides,proteinsandpredictionsdb files and replace it with checking for the number of lines?
The rest looks good!
|
Hi, I changed the file checks for the gzipped files because at some point nf-test created different md5sums when testing locally vs in the ci runners. Checking for the exact number of lines aimed at making sure those files contain the same number of peptides/proteins while ignoring the gzipped headers that can differ. |
|
Did it work to check the differences between the files? |
|
Warning Newer version of the nf-core template is available. Your pipeline is using an old version of the nf-core template: 3.3.2. For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation. |
There are minor floating differences between the Here's an example: |
skrakau
left a comment
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.
nf-tests still require change
7442843 to
429592a
Compare
|
The profiles:
If assemblies get updated on NCBI the files will not be reproducible. Therefore the md5sums will be removed for now from the snapshots of the 4 affected profiles. Instead we will only test for existence of those files. |
337ea4d to
4c95a39
Compare
|
Thanks for squashing the commits! :) This makes it much easier to only check metapep test related changes together independently of all the template update changes 👍 |
tests/pipeline/test_all.nf.test
Outdated
| { assert snapshot(path("$outputDir/db_tables/alleles.tsv"), | ||
| path("$outputDir/db_tables/conditions_alleles.tsv"), | ||
| path("$outputDir/db_tables/conditions.tsv"), | ||
| path("$outputDir/db_tables/entities_proteins.tsv"), |
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.
Shouldn't here microbiomes_entities.tsv be added (because the entities, i.e. taxa, themselves do not change), while entities_proteins.tsv would be expected to change? That's what we listed in our meeting, or is this not what you observed in your tests?
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.
microbiomes_entities.tsv has a different md5sum compared to the last release. It is not clear why this happens.
I will put entities_proteins.tsv to the list of files to be only checked for existence.
4c95a39 to
bb7fdf9
Compare
Just for completeness, it turned out that nf-test can handle gzipped files and that the differences here had other reasons. |
skrakau
left a comment
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.
Thanks @SusiJo 🎉 🚀
This was a lot of hidden work, but it looks great now!
PR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).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).