ExtensionsIndex icon indicating copy to clipboard operation
ExtensionsIndex copied to clipboard

Add MSLesionVisualiser extension

Open AlistairMcCutcheon opened this issue 2 years ago • 8 comments

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
    • [x] 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)

AlistairMcCutcheon avatar Feb 14 '23 10:02 AlistairMcCutcheon

I'm not sure what the issue is - I thought we are supposed to put the .s4etx file in the root of the repo?

AlistairMcCutcheon avatar Feb 14 '23 11:02 AlistairMcCutcheon

Your file name was incorrect. See the necessary changes below

- MSLesionVisualiser.s4etx
+ MSLesionVisualiser.s4ext

jamesobutler avatar Feb 14 '23 12:02 jamesobutler

@AlistairMcCutcheon the orientation of the brain is incorrect in the screenshot on you readme page. We try to be very careful to avoid geometry issues in scan data so please track down an example with correct orientation. Let us know if you need help with this.

pieper avatar Feb 14 '23 16:02 pieper

@pieper Thank you for pointing this out - I didn't realise this was an issue, sorry! This is my first time working with medical image data. It should be fixed now.

AlistairMcCutcheon avatar Feb 20 '23 10:02 AlistairMcCutcheon

@pieper Thank you for pointing this out - I didn't realise this was an issue, sorry! This is my first time working with medical image data. It should be fixed now.

Looks better, thanks.

Do other reviewers have comments?

pieper avatar Feb 20 '23 15:02 pieper

The sole module in this extension is named "Main" which doesn't seem to be a descriptive name about what the module actually does.

I can also tell from https://github.com/AlistairMcCutcheon/SlicerMSLesionVisualiser/blob/main/Main/CMakeLists.txt, that this extension will not build successfully with the Slicer factory machines as the module file imports from a "packages" lib directory and none of those files are specified to be included in the extension package.

jamesobutler avatar Feb 20 '23 20:02 jamesobutler

Hi @jamesobutler, I've changed the name of the module, and I think I may have fixed the cmake issue but I'm not sure. I haven't worked with cmake files before. I've added the line "add_subdirectory(packages)" to the file.

AlistairMcCutcheon avatar Feb 21 '23 10:02 AlistairMcCutcheon

To improve readability and ensure that the number of completed or remaining tasks is properly reported, in the future, consider using [x] instead of [ x] for checkboxes:

-[ x] Repository name is Slicer+ExtensionName
+[x] Repository name is Slicer+ExtensionName

This will ensure the rendering works as expected:

Before After
image image

This comment was adapted from my GitHub saved replies, it is licensed under a Creative Commons Attribution 4.0 International License and you were welcome to reuse it. Creative Commons License

jcfr avatar Aug 15 '23 21:08 jcfr