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

Add SlicerFreeSurferCommands extension ➡️ Add SlicerFreeSurferCommands to SlicerFreeSurfer extension #1943

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

Conversation

benzwick
Copy link

New extension

  • Extension has a reasonable name (not too general, not too narrow, suggests what the extension is for)
  • Repository name is Slicer+ExtensionName
  • 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
  • Extension description summarizes in 1-2 sentences what the extension is usable (should be understandable for non-experts)
  • Any known related patents must be mentioned in the extension description.
  • 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.
  • 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
  • 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)
  • Screenshot URLs (screenshoturls) are correct, contains at least one
  • Homepage URL points to valid webpage containing the following:
    • Extension name
    • Short description: 1-2 sentences, which summarizes what the extension is usable for
    • At least one nice, informative image, that illustrates what the extension can do. It may be a screenshot.
    • Description of contained modules: at one sentence for each module
    • 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)
    • 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.
    • 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:
    • Click Settings and in repository settings uncheck Wiki, Projects, and Discussions (if they are currently not used)
    • 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)

Copy link
Member

@pieper pieper left a comment

Choose a reason for hiding this comment

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

This looks like a very useful extension, thanks 👍

I agree getting this in sooner and getting feedback will be better than trying to spend a lot of time refining everything in this interface.

One thing I think would be very helpful to people though would be to load then results as a segmentation and set the names of the segments.

I thought there were more segments than this, but I believe these are the segment names.
https://github.com/BBillot/SynthSeg/blob/0369118b9a0dbd410b35d1abde2529f0f46f9341/data/labels%20table.txt#L17

Also since we have the AMPSCZ team at project week we should ask some of them about how neuroimaging researchers might use this.

@jcfr
Copy link
Member

jcfr commented Jun 13, 2023

Thanks for the contribution.

Since there is already a SlicerFreeSurfer extension, should we consider updating the existing one ?

See https://github.com/PerkLab/SlicerFreeSurfer

If we move forward with this, we could look into moving the repository into the Slicer the GitHub organization and add an Acknowledgements section.

Cc: @lassoan

@benzwick
Copy link
Author

One thing I think would be very helpful to people though would be to load then results as a segmentation and set the names of the segments.

This would be very nice. I might need some hints on how to implement this @pieper.

I thought there were more segments than this, but I believe these are the segment names. https://github.com/BBillot/SynthSeg/blob/0369118b9a0dbd410b35d1abde2529f0f46f9341/data/labels%20table.txt#L17

The cortical parcellation option adds more segments but I don't know if these have named labels.

Also since we have the AMPSCZ team at project week we should ask some of them about how neuroimaging researchers might use this.

This is a good idea.

Since there is already a SlicerFreeSurfer extension, should we consider updating the existing one ?

See https://github.com/PerkLab/SlicerFreeSurfer

If we move forward with this, we could look into moving the repository into the Slicer the GitHub organization and add an Acknowledgements section.

Cc: @lassoan

One thing to consider is that the existing SlicerFreeSurfer extension doesn't have any external dependencies as far as I'm aware, whereas this SlicerFreeSurferCommands extension depends on FreeSurfer which needs to be installed and configured by the user, takes up about 14 GB of disk space, and can only be installed on Linux/Unix OS (on Windows WSL can be used).

@pieper
Copy link
Member

pieper commented Jun 13, 2023

This would be very nice. I might need some hints on how to implement this @pieper.

here's a similar script:
https://gist.github.com/pieper/17cf79b8ea852693bde0b63bafbaa74f

Regarding existing SlicerFreeSurfer, I think this makes sense to bundle, making it clear that these commands are only available if freesurfer is installed. That is, make it clear to the user at runtime, rather than in documentation, what features would be available if freesurfer is installed along with pointers to the installation instructions (even a button that opens the web page).

@pieper
Copy link
Member

pieper commented Jun 13, 2023

The cortical parcellation option adds more segments but I don't know if these have named labels.

If you don't find it, you could post a link on their issue page.

It may be this table: https://surfer.nmr.mgh.harvard.edu/fswiki/FsTutorial/AnatomicalROI/FreeSurferColorLUT

@benzwick
Copy link
Author

benzwick commented Jun 13, 2023

I opened a new issue at SlicerFreeSurfer to discuss the merge of SlicerFreeSurfer and SlicerFreeSurferCommands, and the move to the Slicer GitHub organization:

@benzwick
Copy link
Author

One thing I think would be very helpful to people though would be to load then results as a segmentation and set the names of the segments.

@pieper I've added this feature allowing the segmentation to be loaded as either a LabelMapVolume or a Segmentation. Both use the FreeSurferColorLUT converted to a 3D Slicer color table to assign colors and labels.

@pieper
Copy link
Member

pieper commented Jun 14, 2023

@pieper I've added this feature allowing the segmentation to be loaded as either a LabelMapVolume or a Segmentation. Both use the FreeSurferColorLUT converted to a 3D Slicer color table to assign colors and labels.

This is great! 👍

@jcfr
Copy link
Member

jcfr commented Aug 16, 2023

To move forward with this, I suggest to work on PerkLab/SlicerFreeSurfer#15 (comment)

@benzwick How does that sound ?

@jcfr jcfr added the Status: Awaiting Response ⏳ Waiting for a response/more information label Aug 16, 2023
@jcfr jcfr changed the title Add SlicerFreeSurferCommands extension Add SlicerFreeSurferCommands extension ➡️ Add SlicerFreeSurferCommands to SlicerFreeSurfer extension May 2, 2024
@jcfr
Copy link
Member

jcfr commented May 2, 2024

To follow-up, this pull request will not be integrated. Instead the following need to be finalized:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting Response ⏳ Waiting for a response/more information
Development

Successfully merging this pull request may close these issues.

3 participants