Skip to content
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

[WIP] add module decompose connectivity #86

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ThoumyreStanislas
Copy link
Contributor

Describe your changes

new module for decompose connectivity, The module is a copy of the module in nf-pediatric created by @gagnonanthony. I didn't add the option to concatenate tractograms before decomposing because it's specific to disconets_flow. However, I'm not sure that a module dedicated to tractogram concatenation would be useful, and it could be an option in the remove invalid streamlines module.

List test packages used by your subworkflow

Checklist before requesting a review

  • Create the tool:
    • Edit ./subworkflows/nf-neuro/<category>/<tool>/main.nf
    • Edit ./subworkflows/nf-neuro/<category>/<tool>/meta.yml
  • Generate the tests:
    • Edit ./subworkflows/nf-neuro/<category>/<tool>/tests/main.nf.test
    • Run the tests to generate the main.nf.test.snap snapshots
  • Ensure the syntax is correct :
    • Run prettier and editorconfig-checker to fix common syntax issues
    • Run nf-core subworkflows lint and fix all errors
    • Ensure your variables have good, clear names

Copy link
Contributor

@gagnonanthony gagnonanthony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! 🍻 I added some minor comments. What was the reasoning behind the naming of the module? I understand that it is used for connectivity, but I tend to prefer TRACTOGRAM_DECOMPOSE since we apply it on a tractogram.

And for the concatenation of the tractogram, if you want, you can add it as an option in this module. It could then be toggle on/off by a task.ext parameter, but feel free to choose which one you prefer.

modules/nf-neuro/connectivity/decompose/meta.yml Outdated Show resolved Hide resolved
modules/nf-neuro/connectivity/decompose/meta.yml Outdated Show resolved Hide resolved
modules/nf-neuro/connectivity/decompose/main.nf Outdated Show resolved Hide resolved
Copy link
Contributor

@arnaudbore arnaudbore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants