README.md Update and Documentation Proposal

[maxradx]

Hi Team,

This Pull Request updates the README.md and adds essential Documentation for testing, from which further comments/documentation can be built on.

How to test this PR? (3 steps):

1- Start from scratch, do as it is the first time you use slicer. Follow Installation steps from the README.
2- Try QuickSTART in the README.md.
3- Read Documentation from Documentation Welcome Page.

Question to ask yourself (examples):

A. Was the installation process without obstruction and difficulties (Slicer and SlicerCART)?
B. Was I able to load images from my dataset? Once loaded, was I able to do manual segmentation as written in the QuickStart?
C. Any feedback/comments regarding the organization set-up of the documentation? When you read those instructions, are they easy for you to understand?

Important Note.
This PR is a pre-beta-release version of SlicerCART. That means the actual main script is really NOT READY for deployment. Only one basic scenario is intended to be tested for now (viewing images and annotate them). Essential functionalities remain to be implemented (bugs fixing e.g. changing labels, load previous segmentations, compare segments, go automatically to the next case, etc.). Accordingly, you should approach this PR as a Documentation PR as mentioned above: are the steps for begin using SlicerCART clear? (Your feedback will be wanted in future PR related to specific functionalities)

This PR addresses issue #62.

Thank you for your consideration.

Maxime

[sandrinebedard]

Thank you for putting all of this together! I followed the instructions, I left some minor comments about the instructions, in general, it is okay!

However, I cannot load the module (step 7):

when I search for it, it is says it can't be loaded:

[maxradx]

Thank you for putting all of this together! I followed the instructions, I left some minor comments about the instructions, in general, it is okay!

However, I cannot load the module (step 7):

when I search for it, it is says it can't be loaded:

Ok! 1- Now with the new instructions adapted, can you try it again? (maybe that is because of the part of the comment you left where you did not know what I meant! --- ex drag the SlicerCART.py file in the additional module path section (see step 4 in the most recent committed version --- need to fetch and pull)

2- Can you open the python console and screenshot the line where there is the error? (there should have some errors when a slicer module cannot be instantiated).

Thanks for your feedback: greatly appreciated!

[sandrinebedard]

2- Can you open the python console and screenshot the line where there is the error? (there should have some errors when a slicer module cannot be instantiated).

same thing again, here is the error in the python console:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Applications/Slicer.app/Contents/lib/Python/lib/python3.9/imp.py", line 169, in load_source
    module = _exec(spec, sys.modules[name])
  File "<frozen importlib._bootstrap>", line 613, in _exec
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/SlicerCART.py", line 10, in <module>
    from utils import * # Import all modules, packages and global variables
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/__init__.py", line 1, in <module>
    from .global_variables import *
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/global_variables.py", line 3, in <module>
    from utils.requirements import *
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/requirements.py", line 27, in <module>
    check_and_install_python_packages()
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/install_python_packages.py", line 31, in check_and_install_python_packages
    response = qt.QMessageBox.question(slicer.util.mainWindow(),
NameError: name 'qt' is not defined
[Qt] loadSourceAsModule - Failed to load file "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/SlicerCART.py"  as module "SlicerCART" !
[Qt] Fail to instantiate module  "SlicerCART"
[Qt] The following modules failed to be instantiated:
[Qt]    SlicerCART
[Qt] Populating font family aliases took 138 ms. Replace uses of missing font family ".AppleSystemUIFont" with one that exists to avoid this cost. 

[sandrinebedard]

if it is the folder path

if what is the folder path? this sentence is unclear, I get that the SlicerCART.py needs to be in the folder, but this is not what your sentence is telling me

[maxradx]

ok I see. Adapted. Better?

[maxradx]

2- Can you open the python console and screenshot the line where there is the error? (there should have some errors when a slicer module cannot be instantiated).

same thing again, here is the error in the python console:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Applications/Slicer.app/Contents/lib/Python/lib/python3.9/imp.py", line 169, in load_source
    module = _exec(spec, sys.modules[name])
  File "<frozen importlib._bootstrap>", line 613, in _exec
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/SlicerCART.py", line 10, in <module>
    from utils import * # Import all modules, packages and global variables
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/__init__.py", line 1, in <module>
    from .global_variables import *
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/global_variables.py", line 3, in <module>
    from utils.requirements import *
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/requirements.py", line 27, in <module>
    check_and_install_python_packages()
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/install_python_packages.py", line 31, in check_and_install_python_packages
    response = qt.QMessageBox.question(slicer.util.mainWindow(),
NameError: name 'qt' is not defined
[Qt] loadSourceAsModule - Failed to load file "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/SlicerCART.py"  as module "SlicerCART" !
[Qt] Fail to instantiate module  "SlicerCART"
[Qt] The following modules failed to be instantiated:
[Qt]    SlicerCART
[Qt] Populating font family aliases took 138 ms. Replace uses of missing font family ".AppleSystemUIFont" with one that exists to avoid this cost. 

Ok! Thanks for the input.
If you copy paste this command in the slicer python console, and then try to use the module, what does it says? (success or error in python console)

import slicer
from slicer.util import qt

thanks!

[maxradx]

2- Can you open the python console and screenshot the line where there is the error? (there should have some errors when a slicer module cannot be instantiated).

same thing again, here is the error in the python console:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Applications/Slicer.app/Contents/lib/Python/lib/python3.9/imp.py", line 169, in load_source
    module = _exec(spec, sys.modules[name])
  File "<frozen importlib._bootstrap>", line 613, in _exec
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/SlicerCART.py", line 10, in <module>
    from utils import * # Import all modules, packages and global variables
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/__init__.py", line 1, in <module>
    from .global_variables import *
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/global_variables.py", line 3, in <module>
    from utils.requirements import *
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/requirements.py", line 27, in <module>
    check_and_install_python_packages()
  File "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/utils/install_python_packages.py", line 31, in check_and_install_python_packages
    response = qt.QMessageBox.question(slicer.util.mainWindow(),
NameError: name 'qt' is not defined
[Qt] loadSourceAsModule - Failed to load file "/Users/sandrinebedard/code/slicer-manual-annotation/SlicerCART/src/SlicerCART.py"  as module "SlicerCART" !
[Qt] Fail to instantiate module  "SlicerCART"
[Qt] The following modules failed to be instantiated:
[Qt]    SlicerCART
[Qt] Populating font family aliases took 138 ms. Replace uses of missing font family ".AppleSystemUIFont" with one that exists to avoid this cost. 

added in the code. see comments that you are tagged (I don't know how to refer from here to the code).

[maxradx]

@sandrinebedard from your comments "2- Can you open the python console and screenshot the line where there is the error? (there should have some errors when a slicer module cannot be instantiated).

same thing again, here is the error in the python console:"

[sandrinebedard]

Can you try:

import slicer
slicer.util.pip_install('qt')

or using the expresion 'Qt' with capital letters? (also for from slicer.util import Qt)

Does this provide an error?

does not work, even with Qt

[maxradx]

Can you try:

import slicer
slicer.util.pip_install('qt')

or using the expresion 'Qt' with capital letters? (also for from slicer.util import Qt)
Does this provide an error?

does not work, even with Qt

Ok. Will look further in the following days. If ever you know find out how to install Qt , please comment it so I can add it in the procedural steps. Thanks for your insight.

[tomDag25]

I didn't look into quickstart.md nor userguide.md because it looks like both are really close and you should just keep one in my opinion. If you think I should review both tell me but else I will wait you delete one to not review two times the same thing.
Also a general comment I feel like putting GO BACK to... at the end of your Markdown is maybe not necessary in most cases: either you want people to follow a path (for example from installation to quickstart) or they have to go back to the README.md but most links seem to be random.

[tomDag25]

People won't look at your Welcome.md they will read your README.md and go to the md files you cite in it. Would delete this file and put the information you judge primordial in README.md

[maxradx]

Ok Considered. Thanks for this input. You are right. However, when I go on successful slicer app (e.g. SlicerRT) there readme is very short and they refer to other documentation pages with more details. this is also in other slicer modules. Will keep the comment opened so others can give there input (I agree with you that this could be in the README as well)

[jcohenadad]

that's way too specific. What if 15.0.2 comes out? Can I use it? likely so. I suggest another wording, be more general to other distros (see eg how we do it in SCT).

[jcohenadad]

? it should or it should not. This statement is confusing for the user

[jcohenadad]

@maxradx a lot of suggested commits are a month-old. Can you please accept them so we can make additional commit suggestions?

[jcohenadad]

There is a lot of additional images in this PR. Although the intention is good (screenshots for the documentation to improve user experience), adding binary files to a git repository is problematic. Given that these screenshots belong to a specific version of slicerCART, which is bound to evolve with time, all these screenshots will need to be updated, again and again, with the repository accumulating fat overtime. In general, it is not a good practice to upload binaries to a git repository. See eg:

• https://www.reddit.com/r/learnprogramming/comments/p6gbms/should_i_store_images_for_my_website_on_github/
• https://softwareengineering.stackexchange.com/questions/80962/should-images-be-stored-in-a-git-repository
• https://github.com/alebedev/git-media

I suggest opening another repos to store the images, eg: slicer-cart-media.

Also important: make sure to provide the sources of your edited screenshots. Eg: what if you made a typo in the edited screenshot, that someone would like to fix later? Working with cloud solution (eg drawIO, google slide) makes it easy to modify images.

Finally: because of the above issues, I think this PR should not be merged as is with all these binaries. And if they are removed from the branch (following my recommendations), then that branch, if accepted, should absolutely be 'squashed' before merging, otherwise the binaries will exist in the git history and it will be a pain to remove them.

[jcohenadad]

don't mention what it cannot do. Just say what it can do

[jcohenadad]

can remove

[jcohenadad]

i had already clicked "new config and next" after reading line 43. Then, the following tell the same thing-- i found it confusing

[jcohenadad]

to echo my previous comment, what i find confusing is that the headings tells you what to do. So i do it. And then, what's below the heading is already done.

[jcohenadad]

sentence not finished

[jcohenadad]

some issues with the text of that screenshot-- easier to manually fix it, but I cannot (because screenshot is not editable)-- related to #105 (review)

[jcohenadad]

remove-- just put a section "Help" somewhere in your documentation.

[jcohenadad]

need to click on "Segmentation" first, to drop down that menu

[jcohenadad]

I clicked on "SegmentEditor", and now I don't see the "Paint" button anymore:

Consider telling users to click again on "Modules -> Examples -> SlicerCART"

[jcohenadad]

that "paint" button is a bit obscure to me. After switching to the SegmentEditor, I can already create the first label. So what is this button useful for? When I click on it, nothing happens.

EDIT 20241216_172912: OK, I just realized that the "Paint" button is to avoid going to the SegmentEditor. I'm not sure this is a judicious design choice, because there is already a button that brings the user to the Segment Editor. I suggest opening an issue on this to discuss it further.

[jcohenadad]

it's called "Erase mode"

[jcohenadad]

saving a JSON that is BIDS compatible with derivatives would be much better-- pls open an issue on this

[jcohenadad]

will this segment only one class? two classes are highlighted in the example above