Skip to content
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

Default value of -f is confusing (it depends on label/, which must be created by sct_warp_template) #3114

Open
yanchunCoBIUS opened this issue Dec 20, 2020 · 4 comments
Labels
documentation category: readthedocs, sourceforge, or SCT courses good first issue Issues that new contributors can easily work on sct_extract_metric context:

Comments

@yanchunCoBIUS
Copy link
Contributor

Description

The directory label is not existence, so the current default and example for parameter f is not correct,

optional.add_argument(
        '-f',
        default=os.path.join("label", "atlas"),
        help=(f"Single label file, or folder that contains WM tract labels."
              f"Example: {os.path.join(__data_dir__,  'atlas')}")
    )
@yanchunCoBIUS yanchunCoBIUS added sct_extract_metric context: documentation category: readthedocs, sourceforge, or SCT courses labels Dec 20, 2020
@yanchunCoBIUS yanchunCoBIUS self-assigned this Dec 20, 2020
@jcohenadad
Copy link
Member

Thank you for your feedback @yanchunCoBIUS

The directory label is not existence

This directory is generated by sct_warp_template, which is supposed to be run before running sct_extract_metric. This is mentioned in the usage:

This program extracts metrics (e.g., DTI or MTR) within labels. Labels could be a single file or a folder generated with 'sct_warp_template' containing multiple label files and a label description file (info_label.txt). The labels should be in the same space coordinates as the input image.

If confusing we could maybe reiterate this explanation at the level of the "-f" flag.

@yanchunCoBIUS
Copy link
Contributor Author

Thanks for your detailed explanation, sorry for bothering you, I will close it.

@joshuacwnewton
Copy link
Member

joshuacwnewton commented Dec 20, 2020

Thanks for your detailed explanation, sorry for bothering you, I will close it.

That's OK! I experienced the exact same thing, and @jcohenadad had to explain the usage to me as well.

If multiple people are getting caught by this, then perhaps it's a sign that the usage is unclear. So, you mentioning this is not a bother at all, it's useful feedback. 😄

I'm going to re-open and change the issue title.

@joshuacwnewton joshuacwnewton changed the title sct_extract_metric wrong help and outdated usage path Clarify confusing 'label/' path (usage dependent on sct_warp_template) Dec 20, 2020
@joshuacwnewton joshuacwnewton changed the title Clarify confusing 'label/' path (usage dependent on sct_warp_template) Default value of -f is confusing (it depends on label/, which must be created by sct_warp_template) Jan 27, 2023
@joshuacwnewton
Copy link
Member

Recently, a new warning was added to sct_process_segmentation in #4004 to alert users when the default -vertfile is missing:

Vertebral level file ./label/template/PAM50_levels.nii.gz does not exist. Vert level information will not be displayed. To use vertebral level information, you may need to run sct_warp_template to generate the appropriate level file in your working directory.

-vertfile is similar to -f in that both default values rely on ./label being generated by sct_warp_template beforehand.

So, we could follow a similar approach here -- check for the existence of -f, and if it's missing, explicitly mention the need to run sct_warp_template. Currently, we partially do this, the error message is pretty bare right now:

raise RuntimeError(path_label + ' does not exist')

@joshuacwnewton joshuacwnewton added the good first issue Issues that new contributors can easily work on label Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation category: readthedocs, sourceforge, or SCT courses good first issue Issues that new contributors can easily work on sct_extract_metric context:
Projects
None yet
Development

No branches or pull requests

3 participants