Add GiftiLoader extension :arrow_right: module GiftiLoader added to SlicerNeuro

[mcespedes99]

trafficstars

New extension

• [x] Extension has a reasonable name (not too general, not too narrow, suggests what the extension is for)
• [x] Repository name is Slicer+ExtensionName
• [x] Repository is associated with 3d-slicer-extension GitHub topic so that it is listed here. To edit topics, click the settings icon in the right side of "About" section header and enter 3d-slicer-extension in "Topics" and click "Save changes". To learn more about topics, read https://help.github.com/en/articles/about-topics
• [x] Extension description summarizes in 1-2 sentences what the extension is usable (should be understandable for non-experts)
• [x] Any known related patents must be mentioned in the extension description.
• [x] LICENSE.txt is present in the repository root. MIT (https://choosealicense.com/licenses/mit/) or Apache (https://choosealicense.com/licenses/apache-2.0/) license is recommended. If source code license is more restrictive for users than MIT, BSD, Apache, or 3D Slicer license then the name of the used license must be mentioned in the extension description.
• [x] Extension URL and revision (scmurl, scmrevision) is correct, consider using a branch name (main, release, ...) instead of a specific git has to avoid re-submitting pull request whenever the extension is updated
• [x] Extension icon URL is correct (do not use the icon's webpage but the raw data download URL that you get from the download button - it should look something like this: https://raw.githubusercontent.com/user/repo/main/SomeIcon.png)
• [x] Screenshot URLs (screenshoturls) are correct, contains at least one
• [x] Homepage URL points to valid webpage containing the following:
  • [x] Extension name
  • [x] Short description: 1-2 sentences, which summarizes what the extension is usable for
  • [x] At least one nice, informative image, that illustrates what the extension can do. It may be a screenshot.
  • [x] Description of contained modules: at one sentence for each module
  • [x] Tutorial: step-by-step description of at least the most typical use case, include a few screenshots, provide download links to sample input data set
  • [ ] Publication: link to publication and/or to PubMed reference (if available)
  • [x] License: We suggest you use a permissive license that includes patent and contribution clauses. This will help protect developers and ensure the code remains freely available. We suggest you use the Slicer License or the Apache 2.0. Always mention in your README file the license you have chosen. If you choose a different license, explain why to the extension maintainers. Depending on the license we may not be able to host your work. Read here to learn more about licenses.
  • [x] Content of submitted s4ext file is consistent with the top-level CMakeLists.txt file in the repository (description, URLs, dependencies, etc. are the same)
• Hide unused features in the repository to reduce noise/irrelevant information:
  • [x] Click Settings and in repository settings uncheck Wiki, Projects, and Discussions (if they are currently not used)
  • [x] Click the settings icon next to About in the top-right corner of the repository main page and uncheck Releases and Packages (if they are currently not used)

This extension only contains 1 module, so I don't have an explicit description of each module but I do mention (on the gitub repo page) that it only has 1 module.

[pieper]

Looks handy - just a few comments:

Here you should use slicer.util.pip_install. https://github.com/mcespedes99/SlicerGiftiLoader/blob/main/GiftiLoader/GiftiLoader.py#L9-L40

Also these shouldn't be installed during module discovery or they will be run at application startup and will slow things down. Instead they should only be installed when needed (i.e. when the user invokes the functionality).

Your tests look helpful, but there are hardcoded paths. It would be good if you could register some SampleData that can be downloaded for the tests both for regression testing and so people know what you are able to support.

https://github.com/mcespedes99/SlicerGiftiLoader/blob/main/GiftiLoader/GiftiLoader.py#L791

[lassoan]

Is there a specific reason for not using the gifti reader in ITK? We would not need to install extra packages and the implementation would be probably a bit simpler. If there is some limitation of the ITK implementation that disqualifies it then we should let ITK developers know about it.

[lassoan]

Is it only (or mainly) used with Freesurfer output? If yes then it could make sense to add it to the Freesurfer extension rather than creating a new extension for it.

[mcespedes99]

In regards to the first comment, is there any specific place I should put the installation of the modules then? @pieper maybe inside the init function of the Widget class?

@lassoan, in regards to the ITK implementation, I'm not so familiar with the ITK implementation but I'll try to take a look and let you know. If you could point me to something in particular, I'd appreciate it.

The module itself is able to load any gifti files from a BIDS directory, it's not specifically related to freesurfer. For example, it also works with fmriprep or hippunfold results!

[pieper]

In regards to the first comment, is there any specific place I should put the installation of the modules then? @pieper maybe inside the init function of the Widget class?

I think a method like self.setupPythonRequirements that could be called in the callbacks before any computation that uses the packages takes place. If the installation takes a while you could give the user a warning and/or progress reporting.

Like what's done here: https://github.com/lassoan/SlicerTotalSegmentator/blob/main/TotalSegmentator/TotalSegmentator.py#L571

There's some documentation in development here, maybe we can flesh it out to describe best practices.

https://github.com/Slicer/Slicer/pull/6913/files

[mcespedes99]

https://simpleitk.readthedocs.io/en/v1.2.4/Documentation/docs/source/IO.html I was checking the documentation of itk in Python but I haven't been able to find any way to load gifti files using itk libraries in Python. Let me know if I'm missing something please.

[lassoan]

@mcespedes99 Maybe the GIFTI reader is only exposed in ITK-Python and not in SimpleITK. Would you mind posting a question about this to the ITK forum (and post the link here for reference)?

[mcespedes99]

@lassoan here it is: https://discourse.itk.org/t/reading-gifti-file-in-python/6052?u=mcespedes99

[lassoan]

Thanks for the post. I've added some comment there.

In the meantime, I had a look at GIFTI format and it seems that it is strictly neuroimaging format, so it would make sense to not start a new extension for it but add it to https://github.com/Slicer/SlicerNeuro extension to make it easier to find for users and to reduce maintenance and support overhead. If you want you can get direct write access to the repository so that you can make changes without going through review.

[mcespedes99]

@lassoan sounds good!! I'll make a fork and a PR there

[mcespedes99]

Just opened a PR in the SlicerNeuro repo (https://github.com/Slicer/SlicerNeuro/pull/1). I updated the installation as suggested as well as an update of the tests to register my images to the MRHead sample data. The installation of the packages only takes a little moment for me but I still added a progress bar that tells the user what is going on. Still no updates on the ITK questions I think. Please let me know if you have any further suggestions.

I haven't compile the SlicerNeuro updates, so I would wait until this is tested to merge my changes. Probably will do it tomorrow during the morning. Thanks for all the help!

[mcespedes99]

Following the suggestion from @lassoan, now I'm requesting to push my changes to SlicerNeuro. Then I just deleted the GiftiSlicer.s4ext and edited the SlicerNeuro one. I'm waiting for the PR to SlicerNeuro to be approved. I guess that as soon as that happens, this should be good to go.

[jcfr]

requesting to push my changes to SlicerNeuro. Then I just deleted the GiftiSlicer.s4ext and edited the SlicerNeuro one. I'm waiting for the PR to SlicerNeuro to be approved. I guess that as soon as that happens, this should be good to go.

For reference, here is the pull request:

• https://github.com/Slicer/SlicerNeuro/pull/1

[jcfr]

@mcespedes99 Additional comment were provided at https://github.com/Slicer/SlicerNeuro/pull/1#pullrequestreview-1557669636, when you have a chance could you address those ?

[mcespedes99]

Hi! Sorry, I've been busy with other things but it should be soon! Sorry for the delay

[jcfr]

Thanks for the follow up :pray:

In the meantime, let us know if you have any questions.

[jcfr]

@lassoan It turns this pull request will not be integrated. Instead, we need to finalize the following:

• https://github.com/Slicer/SlicerNeuro/pull/1