-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adding the python script for manual labeling #2
base: master
Are you sure you want to change the base?
Conversation
(I'd rather have a PR to discuss the code ) |
This add the -path option to manual labeling which give the root of Bids path This changes the output of get_bids_sub to remove the BIDS root path
sop.md
Outdated
python manual-labeling/manual_labeling.py -file list_todo_update.txt -author lucas -path Path_to_duke/ \[-correct 1 -o label_tmp\] | ||
|
||
- -file: txt file that contains the list of all images inside bids root folder that you want to process separated by '\n'.The format in sub-xx/anat/sub-xxx_xxx.nii.gz. Can be obtained with get_bids_sub.py. | ||
- -path : Path th Bids root folder (duke) with a '/' at the end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comment: You can check / adjust for missing /
I think
sop.md
Outdated
- -path : Path th Bids root folder (duke) with a '/' at the end | ||
- -author: Author name that will appear on the .json file | ||
- -correct: Boolean. default is 0. If correct is 1, the script will look for existing label and open them with the -ilabel option from sct_label_utils so you can verify/correct existing label. | ||
- -o: in the output folder given by the 5th argument, you will find in BIDS convention sub-xxx/anat/sub-xxx_labels-disc-manual.nii.gz and sub-xxx/anat/sub-xxx_lables-disc-manual.json" if this is empty this will save under BIDS_PATH/derivatives/labels/sub-xx/anat/xxx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend to remove "by the 5th argument".
to end : performa keyboard interrupt from the terminal (ctrl+c). This will update the list by deleting the viewed subjects. don't forget to commit and push it on github. | ||
Specific: | ||
possible file suffix to file are _acq-sag_T2w, _T2w, _T1w, _acq-sagcerv_T2w | ||
suffix to label is labels-disc-manual |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be relevant to add a parameter that controls this suffix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's actually a remain of a first version where I asked for file suffix. I'll remove it from the SOP. Now there are no reason that I can think of. we can add a case for the constraint on it if you think this could be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was simply thinking: if one day we want to label pmj instead of the disc --> pmj-manual
instead of labels-disc-manual
.
But we can leave it as it is for now :-)
get_BIDS_sub.py
Outdated
required=True, | ||
help="path to BIDS Data", | ||
) | ||
mandatoryArguments.add_argument( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
constraint
, ope
, value
are great.
Possible avenue of improvement: add possibility to have multiple constraints (separated by commas?).
Could also use a config file to specify those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for requiring them as mandatory? I would recommend to change for optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, in my mind there was another solution to get all file but I just realized that's will not work with the 'manual labeling.py' script. (ls > file.txt would get you all the subject but not image file) will fix
Comment about the optional argument in get_ids_sub fixed in d8343ee |
I have run
--> Will correct the code to discard |
I tried to following command and it worked well!
After doing Ctrl+C, it updated the list.txt --> great! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lrouhier I tried the script and it worked well, I added some suggestion otherwise is good.
If there a way of excluding some images for which the labeling would be hard to mark. For example this one:
Co-authored-by: alexfoias <[email protected]>
Co-authored-by: alexfoias <[email protected]>
What about adding minimal resolution? something like pixdim <= 1 |
This PR adds the python script. It is here for discussion.
Added
Manual_labeling.py :
this script takes as arguments the list of image file you want to look at, the author name, and acorrect
flag.If the flag is used: this will run sct_label_utils with the -ilabel option and open the existing file for the user to check/correct. If not used the script will skip files that are already labeled.