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

[Module] ihMT module #70

Merged
merged 14 commits into from May 14, 2024
Merged

[Module] ihMT module #70

merged 14 commits into from May 14, 2024

Conversation

karanphil
Copy link
Contributor

@karanphil karanphil commented Feb 9, 2024

Describe your changes

I added a module for ihMT. Could not test it since I am waiting for scilpy 2.0. @Manonedde

Say which test data are used by your module

It uses ihMT.zip.

Checklist before requesting a review

  • Create the tool:
    • Edit ./modules/nf-scil/<category>/<tool>/main.nf
    • Edit ./modules/nf-scil/<category>/<tool>/meta.yml
  • Generate the tests:
    • Edit ./tests/modules/nf-scil/<category>/<tool>/main.nf
    • Edit ./tests/modules/nf-scil/<category>/<tool>/nextflow.config
    • Add test data locally for tests with the fork repository
    • Generate the test infrastructure and md5sum for all outputs
  • Ensure the syntax is correct :
    • Check indentation abides with the rest of the library (don't hesitate to correct others !)
    • Lint everything. Ensure your variables have good names.

@Manonedde Manonedde self-requested a review February 9, 2024 14:15
@Manonedde Manonedde added the WIP label Feb 9, 2024
@AlexVCaron AlexVCaron mentioned this pull request Feb 22, 2024
@AlexVCaron AlexVCaron changed the title [WIP] ihmt module (waiting for scilpy 2.0) [Module][WIP] ihmt (waiting for scilpy 2.0) Mar 8, 2024
@AlexVCaron AlexVCaron added the waiting Waiting for other things to go through label Mar 8, 2024
@karanphil karanphil changed the title [Module][WIP] ihmt (waiting for scilpy 2.0) [Module] ihMT module May 7, 2024
Copy link
Contributor

@Manonedde Manonedde left a comment

Choose a reason for hiding this comment

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

LGTM!

NB: Are we going to make a module for “classic” MT images? I know there are people at Max who don't use ihMT, but could MT be a “myelin” sub-workflow?

@karanphil
Copy link
Contributor Author

LGTM!

NB: Are we going to make a module for “classic” MT images? I know there are people at Max who don't use ihMT, but could MT be a “myelin” sub-workflow?

It would be easy to almost copy paste this ihmt module and do a "mt" module. I plan to do it someday.

Copy link
Collaborator

@AlexVCaron AlexVCaron left a comment

Choose a reason for hiding this comment

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

Two things, then this should be ready :

  • Update your container to 2.0.2
  • the comp_maps output in your tests is always empty. Is it normal ?

@karanphil
Copy link
Contributor Author

Two things, then this should be ready :

  • Update your container to 2.0.2
  • the comp_maps output in your tests is always empty. Is it normal ?

Good point, there was an option missing in the scil script command line.

Copy link
Collaborator

@AlexVCaron AlexVCaron left a comment

Choose a reason for hiding this comment

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

🚀

@AlexVCaron AlexVCaron merged commit 7cb01f5 into scilus:main May 14, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting Waiting for other things to go through WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants