Create SlicerBatch.s4ext

[JoostJM]

TODO:

• Add documentation in github repository
• Add documentation on the wiki page

[lassoan]

What does this extension do? It would be important to provide at least some minimal documentation.

[fedorov]

I agree with Andras. I guess there was some urgency aspect to justify the merge, but I don't know what it was!

[jcfr]

SlicerBatch is also a very generic name.

…

On Jan 12, 2018 3:58 PM, "Andras Lasso" ***@***.***> wrote: What does this extension do? It would be important to provide at least some minimal documentation. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1505 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANXo0asMXtxysir83uhXOYMblpscSIYks5tJ8RVgaJpZM4RcxdO> .

[JoostJM]

@pieper helped me to get this module registered as an extension.

@lassoan, the purpose of the module is to streamline loading images and labelmaps (and saving new/updates to labelmaps) for a batch of cases. That way the reader only has to worry about making/reviewing the segmentations and not about the loading and saving process.

I built this extension first as a sort of hack because I had to segment a lot of cases and hated the time I needed to load and save everything. Steve then asked me if I was also interested in sharing this with the community.

[JoostJM]

Any suggestions (including maybe a less generic name) are welcome!

[JoostJM]

I added a README with some instructions in the extension repository, but want to wait with adding the wikipages to leave some time to optionally change the name.

[pieper]

There wasn't any particular urgency, but there was also no obvious reason to delay. It's a simple scripted extension and the sooner @JoostJM can debug and refine it in the context of the nightly builds the better for everyone and no real downsides.

[JoostJM]

@jcfr, how about "SlicerCaseIterator"?

[lassoan]

I like the CaseIterator name. Note that typically we start the extension name with Slicer (to make it obvious that the repository contains a Slicer extension) but the Slicer prefix is redundant when you display the extension in Slicer. So, repository name is "SlicerCaseIterator" and extension name is "CaseIterator".

[JoostJM]

@lassoan, Thanks! I will update the name as you suggested. I discussed where to add/ not to add the "Slicer" prefix, and decided that for "SlicerBatch" it wouldn't really cover the load any more (and therefore stuck to "SlicerBatch"). With the name change, I will drop the prefix in the extension name though.

[lassoan]

The README.md file is really helpful! Now I understand what the extension is for and I really like the concept.

A few comments:

• Convert ideas and known issues in WIP section to issue tracker items (https://github.com/JoostJM/SlicerCaseIterator/issues)
• Is there a module GUI? You could add a simple module GUI that would allow you to load and edit a csv file (you can load csv file as a table node and edit using a slicer.qMRMLTableView widget), show the index of current case, have buttons for prev/next/reset, jump to case.

[JoostJM]

Convert ideas and known issues in WIP section to issue tracker items (https://github.com/JoostJM/SlicerCaseIterator/issues)

Done.

Is there a module GUI? You could add a simple module GUI that would allow you to load and edit a csv file (you can load csv file as a table node and edit using a slicer.qMRMLTableView widget), show the index of current case, have buttons for prev/next/reset, jump to case.

Yes, I just finished on the code to also work with the tablenode etc. There is already a screenshot in the repository I believe (although it doesn't show a table loaded, will update this). Tomorrow a fully functional version should be available in the extension manager.

[fedorov]

FYI I renamed s4ext and updated the repository pointer: 035dcf3

[JoostJM]

@fedorov, Thanks, but there were also some URLs in the s4ext that had to be updated. I created a new PR to that effect (#1506)

[JoostJM]

I still need to add the wiki pages, but for that I need an account to the slicer wiki (just requested it). Once I have that, I'll add the documentation to the wiki pages as well...

[JoostJM]

@lassoan, here is the screenshot

[lassoan]

Looks nice!
I would add the prev/next/... buttons here as well (and maybe a toolbar) to make it more easy to discover how to switch between cases (you can display the keyboard shortcut in button tooltips).

[JoostJM]

@lassoan, Haha they are there, but are not visible when the iterator is not loaded. Good tip about the shortcuts though, I'll add that.

[fedorov]

Now that I have a better idea about this module - we have a conceptually similar module - mpReview:

1. mpReviewPreprocessor is a standalone module that takes a path to DICOM dataset, and sorts/reconstructs volumes by patient/study/series
2. then mpReview is pointed to the root directory of the resulting sorted dataset, and has interface to select subject and specific series that should be shown
3. in the segmentation step, individual series are arranged in a synchronized layout (our target application is prostate mpMRI annotation). When segmentation is saved, it is stored in the same directory as imaging data following certain directory naming conventions.

Just wanted to mention it. I don't think one will replace another, but it's interesting to compare how similar task was addressed in different contexts by different developers.

[JoostJM]

If I'm correct, SlicerCaseIterator should be built and added to the extension manager. However, I can find it (searching in latest nightly buid). Is there something I have to do to make the extension available?
Additionally, I can't find category "Utilities", is this category excluded for extensions?

[pieper]

Looks like it did not build right. You can look on cdash (go to http://slicer.cdash.org/index.php?project=Slicer4 and turn on All nightly extension results and search for iterator).

But I'm not sure how to interpret the "could not load cache" result:

http://slicer.cdash.org/viewBuildError.php?buildid=1179920

[JoostJM]

@pieper Thanks, although I don't know either. @jcfr, do you have an idea why the build is failing?

Build output:
win: http://slicer.cdash.org/buildSummary.php?buildid=1179962
mac: http://slicer.cdash.org/buildSummary.php?buildid=1179637
linux: http://slicer.cdash.org/buildSummary.php?buildid=1179637

[lassoan]

Inner-build directory name was incorrectly set to "inner-build" in the .s4ext file. I've fixed it in master branch.

I would recommend to submit the extension to the 4.8 branch of the ExtensionsIndex, too. It may take a while until nightly Slicer version will be fully functional.

[jcfr]

Thanks @lassoan

@JoostJM Also good to know that the description file is generated in the build directory of any extension.

[pieper]

For this extension we manually made the s4ext file because there was no slicer build available, so that would explain why the field wasn't correct.

Do we have a process for people who want to submit pure python extensions that doesn't require having a Slicer build just so they can make a s4ext file automatically?

[JoostJM]

Do we have a process for people who want to submit pure python extensions that doesn't require having a Slicer build just so they can make a s4ext file automatically?

Is it an idea to add this to the extension wizard in slicer?

[pieper]

Do we have a process for people who want to submit pure python extensions that doesn't require having a Slicer build just so they can make a s4ext file automatically?

Is it an idea to add this to the extension wizard in slicer?

I think that would be the easiest place, yes. For a scripted extension there shouldn't be any need for a build tree.

[lassoan]

At the same time, we should also revisit content and format of the file. I've added this idea to the relevant lab page - https://www.slicer.org/wiki/Documentation/Labs/ExtensionsMetadata.

[jcfr]

Agreed with all aspect.

[fedorov]

If that new feature is added to ExtensionWizard, I think it would be important to make sure that it uses the same code that generates s4ext when running cmake.

[jcfr]

Worth noting that this use case is already supported.

The extension wizard is available in the installer version of Slicer and already support creation of the description file and contribution to the extension index.

On linux, it can be found in bin/slicerExtensionWizard

If more people use it, we should be able to fix the remaining issues.

[jcfr]

@fedorov

it uses the same code that generates s4ext when running cmake

Agreed. We put care to ensure this is the case. The following file is packaged in the installer:

./share/Slicer-4.9/Wizard/Templates/Extensions/extension_description.s4ext.in

and also correspond to the use used by both the wizard and the CMake code.

[jcfr]

See https://github.com/Slicer/Slicer/blob/cea9dcfd94bc1b670126c60a06a6950e5ead1c01/Extensions/CMake/SlicerFunctionGenerateExtensionDescription.cmake#L108-L112

[fedorov]

Almost forgot to mention the infamous poll that attracted attention of the total of 6 developers! :-D

https://discourse.slicer.org/t/poll-how-do-you-re-generate-s4ext-file-for-slicer-extensionsindex/381

[jcfr]

@fedorov Ironically, the current discussion was suggesting improving the extension wizard .. which already support the requested feature.

I think there is no need to change anything (especially considering the resources at hand), there are different paths for different people.

Copying a existing description file and adapting it is perfectly fine. The dashboard did its work and the issue got fix a day later by a developer different from the one who implemented the system. (thanks @lassoan ) 😄

[pieper]

it's true that except for few confusing parts the whole process was very smooth and easy. Thanks especially to everyone for helping out!

[lassoan]

The extension wizard is available in the installer version of Slicer and already support creation of the description file and contribution to the extension index.

If something is not available on the GUI then it's very hard to discover or use it correctly.

Does ExtensionWizard use CMake for generating the s4ext file? CMake is not necessary for anything else when developing a Python-based extension, so it would be nice to be able to work without it.

Most of the s4ext file content is ignored by the extension build system (only repository and build directory information is used). So, the file could be very easily written in a text editor. We don't really need complex generator code.

Actually used content of a .s4ext file:

scm git
scmurl https://github.com/f-tang/Add3DText.git
scmrevision 48640ea
build_subdirectory .

I see two good options to move things forward:

• Option A: simplify s4ext files to what is really necessary (4 lines of text file), have as many "generators" as we want (in Python in ExtensionWizard GUI; using CMake in ExtensionWizard CLI or other places) - these generators would be so simple that it would be insignificant extra burden to maintain multiple of them
• Option B: adopt some standard software metadata description format (see labs page) and use that exclusively (don't define the same information in CMakeLists.txt; if info is needed for CMake then we'll implement parser for that in CMake)

I think Option B is much nicer and not too much work. I'm comfortable with "doing nothing for now" option, too, but we are in good position for implementing Option B, as we do a lot of breaking changes in Slicer 5 infrastructure anyway.