Skip to content

[5.8] Add SlicerOpenLIFU #2162

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

Closed
wants to merge 4 commits into from
Closed

[5.8] Add SlicerOpenLIFU #2162

wants to merge 4 commits into from

Conversation

arhowe00
Copy link

@arhowe00 arhowe00 commented Apr 22, 2025

New extension

Tier 1

Any extension that is listed in the Extensions Catalog must fulfill these requirements.

  • Repository name is Slicer+ExtensionName (except if the repository that hosts the extension can be also used without Slicer)
  • 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 and the name of the license is mentioned in extension homepage. 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. MIT (https://choosealicense.com/licenses/mit/) or Apache (https://choosealicense.com/licenses/apache-2.0/) license is recommended. Read here to learn more about licenses. If source code license is more restrictive for users than MIT, BSD, Apache, or 3D Slicer license then describe the reason for the license choice and include the name of the used license in the extension description.
  • Extension URL and revision (scmurl, scmrevision) is correct, consider using a branch name (main, release, ...) instead of a specific git hash 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
  • Content of submitted json file is consistent with the top-level CMakeLists.txt file in the repository (dependencies, etc. are the same)
  • 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
    • Publication: link to publication and/or to PubMed reference (if available)
  • Hide unused github features (such as Wiki, Projects, and Discussions, Releases, Packages) 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)
  • The extension is safe:
    • Does not include or download binaries from unreliable sources
    • Does not send any information anywhere without user consent (explicit opt-in is required)

Tier 3

Community-supported extensions.

  • Extension has a reasonable name (not too general, not too narrow, suggests what the extension is for)
  • Documentation, tutorial, and test data are provided for most modules. A tutorial provides step-by-step description of at least the most typical use case, include a few screenshots. Any sample data sets that is used in tutorials must be registered with the Sample Data module to provide easy access to the user.
  • Follows programming and user interface conventions of 3D Slicer (e.g., GUI and logic are separated, usage of popups is minimized, no unnecessary custom GUI styling, etc.)
  • The extension can be successfully built and packaged on all supported platforms (Windows, macOS, Linux)
  • Maintainers respond to issues and pull request submitted to the extension's repository.
  • Maintainers respond to questions directly addressed to him/her via @mention on the Slicer Forum.
  • Permissive license is used for the main functions of the extension (recommended Apache or MIT). The extension can provide additional functionality in optional components that are distributed with non-permissive license, but the user has to explicitly approve those before using them (e.g., a pop-up can be displayed that explains the licensing terms and the user has to acknowledge them to proceed).
  • All requirements of tiers < 3.

Tier 5

Critically important extensions, supported by Slicer core developers. New Slicer Stable Release is released only if all Tier 5 extension packages are successfully created on all supported platforms.

  • Slicer core developers accept the responsibility of fixing any issues caused by Slicer core changes; at least one Slicer core developer (anyone who has commit right to Slicer core) must be granted commit right to the extension's repository.
  • Automated tests for all critical features.
  • Maintainers respond to questions related to the extension on the Slicer Forum.
  • All requirements of tiers < 5.

An issue was opened regarding the variable set below in the root CMakeLists.txt:

set(EXTENSION_SCREENSHOTURLS "https://github.com/OpenwaterHealth/SlicerOpenLIFU/blob/main/Screenshots/1.png")

where Screenshots is supposed to be all lowercase as in the extension repository SlicerOpenLIFU.

@pieper
Copy link
Member

pieper commented Apr 22, 2025

Thanks for your work on this and interest in contributing.

We saw that the license for the code is AGPL, which we interpret as a "non-permissive" license, since it may require people to change the license on other code it's mixed with. So from a Slicer point of view, that would make this a "tier 1" extension. In the future we plan not to display tier 1 extensions by default.

I'd encourage you to try revisiting the license in order to collaborate more closely with the rest of the 3D Slicer community. There are many points of possible mutual benefit with projects like NousNav and SlicerTMS, but those collaborations won't be possible if you stick to the AGPL license. Please consider Apache 2.0, BSD, the Slicer License, or another option that would allow collaboration.

Also, just a comment on the readme: it would be good to include a definition of LIFU and also links to your company's website to make it easier for people to quickly understand what the project is about.

@ebrahimebrahim
Copy link

The license is a fixed choice by Openwater in this case and not likely to change. We could include a copyleft warning for anyone who tries to mix the extension into their own work. It would be convenient

Also, just a comment on the readme: it would be good to include a definition of LIFU and also links to your company's website to make it easier for people to quickly understand what the project is about.

Good idea. @arhowe00 here is some introductory text we could add:

Low intensity focused ultrasound (LIFU) is a method of neuromodulation. This uses ultrasound as a non-destructive treatment as opposed to using it for imaging.

This link to early access systems may be appropriate to include for now: https://www.openwater.health/early-access-systems

@arhowe00
Copy link
Author

Made these changes (including fixing the link) and posting a PR to SlicerOpenLIFU.

@arhowe00
Copy link
Author

Changes have been pushed to main @pieper

@lassoan
Copy link
Contributor

lassoan commented Apr 24, 2025

In medical image computing, permissive (MIT, Apache, BSD) licenses are the standard for open-source tools, probably because restrictive licenses practically prevent translating prototypes into clinical use. A restrictive license is acceptable in the Extensions Index, but to increase the impact of this extension and encourage community engagement, a permissive license would be a much better fit.

Would you be able to start a discussion with your funder about their open-source strategy and goals? They may find that the extreme AGPL license is not the best choice to achieve those goals. We would be happy to provide more details or participate in meetings if that helps.

@ebrahimebrahim
Copy link

The choice of AGPL seems to be quite deliberate, but it doesn't hurt to ask and I'm not actually sure it's non-negotiable or that nothing has changed since that choice was made. We can reach out and get back to you @lassoan and @pieper. Thanks!

@ebrahimebrahim
Copy link

ebrahimebrahim commented May 6, 2025

reminder to add SlicerIGT SlicerSurfaceToolbox to the dependencies listed in the json

@arhowe00
Copy link
Author

arhowe00 commented Jun 5, 2025

Would you be able to start a discussion with your funder about their open-source strategy and goals? They may find that the extreme AGPL license is not the best choice to achieve those goals. We would be happy to provide more details or participate in meetings if that helps.

@pieper @lassoan

We've had some discussions with the folks at Openwater, and in the meantime we have decided to stick with AGPL. They acknowledge the extension will be in a low tier and accept that in the meantime. I've rebased to the 5.8 branch and bumped the version to v0.4.0.

I found an interesting post on Vitalik's (one of the funders) website, where he shares some of his opinions on open source...

@lassoan
Copy link
Contributor

lassoan commented Jun 7, 2025

Thanks for your efforts @arhowe00. This licensing issue is very unfortunate, because we strive to achieve absolute openness, without restrictions, and we have been very successful so far. This will be the only 3D Slicer extension out of 200+ that has a restrictive license (except two very old, now defunct ones - GeodesicSlicer and TOMAAT), but I guess we have to be open to non-openness, too.

I would only ask to add the license restriction note (can be a shortened version of the note in the readme) to the extension's description and I hope that at some point we'll have a chance to talk to the funder and clarify that must be some misunderstanding.

@pieper please chime in if you would be OK with this or have more requests.

@pieper
Copy link
Member

pieper commented Jun 7, 2025

Yes, the AGPL issue is a real problem mainly because there's so much potential synergy with other projects (SlicerTMS, NousNav, etc).

I'm thinking maybe the best is for OpenWater to just distribute the extension outside of the extension catalog. It's very easy to install pure-python extensions and since it's going to be tier 1 and not visible by default there's not much practical difference.

@ebrahimebrahim
Copy link

Indeed the license is unfortunate. Openwater may change it, but it seems they have a complicated internal discussion going on. Having this be the only extension in the store with a restrictive license will present a strong case for the party in favor of changing it. This is one reason we want to proceed with publishing to the extension index, and allow it to be Tier 1 for now.

I'm thinking maybe the best is for OpenWater to just distribute the extension outside of the extension catalog. It's very easy to install pure-python extensions and since it's going to be tier 1 and not visible by default there's not much practical difference.

This is what we do currently, but it's not so convenient because the distributed extension has to be built against the same Slicer revision+OS into which it's being added. Even though it is pure python, the extension package has metadata that causes it to not install correctly if there is a mismatch of revision number or OS.

@arhowe00 we can add the license restriction warning note to the extension description, and then proceed

@arhowe00
Copy link
Author

arhowe00 commented Jun 9, 2025

I would only ask to add the license restriction note (can be a shortened version of the note in the readme) to the extension's description.

See the newly merged description here. I haven't yet bumped the version here to include that bit, which I will once we make the next extension release with the extension description change.

Let us know if we are still able to merge into the index, despite the license.

@jamesobutler
Copy link
Contributor

jamesobutler commented Jun 9, 2025

@lassoan I guess the drag and drop of extension repo zip code (Slicer/Slicer#4610) is not supported yet if someone say downloaded the zip of https://github.com/OpenwaterHealth/SlicerOpenLIFU/archive/refs/tags/v1.4.0.zip? This to avoid developers having to manually publish their own Slicer extension zip bundle to support installing the extension through the "Install from file" option of the extensions manager.

I assume integration into the extensions manager here (even though tier 1 and to be hidden) is because it is seen as the "easiest" way for users to get a Slicer extension that only has scripted modules loaded into the Slicer application.

@jcfr jcfr changed the title Add SlicerOpenLIFU [5.8] Add SlicerOpenLIFU Jun 9, 2025
@jcfr
Copy link
Member

jcfr commented Jun 9, 2025

Please, consider rebasing this branch against the latest 5.8 🚀

Attempt to do so ourselves failed:

cd ExtensionsIndex
git remote add OpenwaterHealth [email protected]:OpenwaterHealth/SlicerOpenLIFU-ExtensionsIndex.git
git fetch OpenwaterHealth
git checkout -b OpenwaterHealth-5.8 OpenwaterHealth/5.8
git rebase 5.8 OpenwaterHealth-5.8
git push OpenwaterHealth OpenwaterHealth-5.8:5.8 --force
ERROR: Permission to OpenwaterHealth/SlicerOpenLIFU-ExtensionsIndex.git denied to jcfr.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

@jamesobutler
Copy link
Contributor

jamesobutler commented Jun 9, 2025

@arhowe00 @ebrahimebrahim A separate question - Is SlicerOpenLIFU being attempted for integration only into Slicer stable 5.8 extension branch for a reason? Typically all new extensions are first added into the main branch to be available for Slicer Preview and then generally once any build issues are resolved, it is then made available for Slicer stable.

I can't think of an extension right off hand that is only available for Slicer stable, but not available for latest Slicer Preview (aka available on the main branch in this repo where stable branches such as 5.8 are branched from main). When a new Slicer stable is released (e.g. 5.10.0 or 6.0.0), the SlicerOpenLIFU extension in Slicer 5.8.1 will no longer have extension updates pushed out to users. This is because the Slicer factory machines only build stable extensions for the most recent Slicer stable. In this situation updating the SlicerOpenLIFU version in the 5.8 branch wouldn't result in a change on the user's end as new extension versions won't be getting built.

@ebrahimebrahim
Copy link

Thank you for pointing this out; I hadn't noticed this was targeting a release branch! Indeed, we really want to target the main branch @arhowe00

@arhowe00
Copy link
Author

arhowe00 commented Jun 11, 2025

Good point @jamesobutler . We were sticking to 5.8.1 because we had to have control over the revision for releasing the extension in the previous manner, which we will cease to do after the extension is published on the index. I'm going to push the added tier 1 extension to OpenwaterHealth:main and then rebase and request merging into Slicer:main.

I am closing this PR. Please see the continuation of this conversation in the PR to merge OpenwaterHealth/SlicerOpenLIFU-ExtensionsIndex:main into Slicer/ExtensionsIndex:main.

@arhowe00 arhowe00 mentioned this pull request Jun 11, 2025
28 tasks
@arhowe00 arhowe00 closed this Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants