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
[Module] Freewater module #61
Conversation
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.
All good! 💯
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
I updated the tests for the new reconst/freewater module! Should be ready to re-review @Manonedde @gagnonanthony @arnaudbore |
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.
Nice job! 😄 I added minor comments. It also seems that snapshots aren't generated for the output files (other than the kernels).
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.
All good! 💯
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.
Update your container to 2.0.2, then LGTM 🚢
Describe your changes
I added a freewater module! It is really close to the NODDI module. @Manonedde @gagnonanthony
Also, I corrected a few typos in the documentation.
Say which test data are used by your module
It uses the commit_amico test data.
Checklist before requesting a review
./modules/nf-scil/<category>/<tool>/main.nf
./modules/nf-scil/<category>/<tool>/meta.yml
./tests/modules/nf-scil/<category>/<tool>/main.nf
./tests/modules/nf-scil/<category>/<tool>/nextflow.config