spinalcordtoolbox / spinalcordtoolbox

Comprehensive and open-source library of analysis tools for MRI of the spinal cord.
https://spinalcordtoolbox.com
GNU Lesser General Public License v3.0
199 stars 101 forks source link

TypeError: 'NoneType' object is not iterable #3154

Closed jcohenadad closed 3 years ago

jcohenadad commented 3 years ago

After a fresh install of SCT version 3dfd4f81f9e4364dab4d94894ea687a625dbdea5, I run into the following error:

julien-macbook:~/code/sct $ sct_deepseg

--
Spinal Cord Toolbox (git-master-3dfd4f81f9e4364dab4d94894ea687a625dbdea5)

sct_deepseg 
--

Traceback (most recent call last):
  File "/Users/julien/code/sct/spinalcordtoolbox/scripts/sct_deepseg.py", line 235, in <module>
    main(sys.argv[1:])
  File "/Users/julien/code/sct/spinalcordtoolbox/scripts/sct_deepseg.py", line 144, in main
    for file in arguments.i:
TypeError: 'NoneType' object is not iterable

Probably a regression error related to the recent PR dealing with parser-related settings? @joshuacwnewton @kousu

joshuacwnewton commented 3 years ago

Ahh, interesting. When we made the changes in #3139, I believe we were working under the assumption that there was at least one mandatory argument for each script, usually -i. (That way, if a script was called with no args, an error would be thrown, which would then call the help.)

But sct_deepseg has no mandatory arguments! So it doesn't throw parser.error in this case (when it probably should).

Might be worth checking if any other scripts are lacking a mandatory argument, too.

jcohenadad commented 3 years ago

we were working under the assumption that there was at least one mandatory argument for each script, usually -i. (That way, if a script was called with no args, an error would be thrown, which would then call the help.)

why not checking if no argument is input, and if that's the case, run the help?

joshuacwnewton commented 3 years ago

why not checking if no argument is input, and if that's the case, run the help?

We did have this before, when we used parser.parse_args(argv if argv else "--help"). But, the problem with that is it returns error code 0, when we really want it to throw an error, because calling with no args is invalid usage.

This was noticed by @kousu in https://github.com/neuropoly/spinalcordtoolbox/issues/3137.

kousu commented 3 years ago

the assumption that there was at least one mandatory argument for each script

That's not quite right. The way I caught this was because sct_check_dependencies has no mandatory arguments.

argparse should be enough to handle this sort of thing, and automatically error out as needed.

You probably want nargs='+' for this one: https://docs.python.org/3/library/argparse.html#nargs

kousu commented 3 years ago

..which is already there:

https://github.com/neuropoly/spinalcordtoolbox/blob/3dfd4f81f9e4364dab4d94894ea687a625dbdea5/spinalcordtoolbox/scripts/sct_deepseg.py#L38-L32

so what's going on here?

kousu commented 3 years ago

Here's what's going on:

argparse, rightly, imo, assumes that - arguments are optional. nargs="+" is enforced (sct_deepseg -i returns 2 and prints the help) but not if you don't pass it at all.

argparse lets us break the convention, by passing required=True:

--- a/spinalcordtoolbox/scripts/sct_deepseg.py
+++ b/spinalcordtoolbox/scripts/sct_deepseg.py
@@ -38,6 +38,7 @@ def get_parser():
     input_output.add_argument(
         "-i",
         nargs="+",
+        required=True,
         help="Image to segment. Can be multiple images (separated with space).",
         metavar=Metavar.file)
     input_output.add_argument(

which gives this usage

$ sct_deepseg

--
Spinal Cord Toolbox (git-ng/3154-mandatory-options-0adeae975066247d057e3c28cce3e5f9b7c1c9ef)

sct_deepseg 
--

sct_deepseg: error: the following arguments are required: -i

usage: sct_deepseg -i <file> [<file> ...] [-c <file> [<file> ...]] [-o <str>] [-task <str>] [-list-tasks]
                   [-install-task {seg_sc_t2star,seg_mice_sc,seg_mice_gm,seg_tumor_t2,seg_tumor-edema-cavity_t1-t2}] [-thr <float>] [-r {0,1}] [-largest KEEP_LARGEST] [-fill-holes {0,1}]
                   [-remove-small REMOVE_SMALL [REMOVE_SMALL ...]] [-v <int>] [-h]

Segment an anatomical structure or pathologies according to the specified deep learning model.

INPUT/OUTPUT:
  -i <file> [<file> ...]
                        Image to segment. Can be multiple images (separated with space).
  -c <file> [<file> ...]
                        Type of image contrast. Indicates the order in which the images have been presented with -i. Optional if only one image is specified with -i. The contrasts should be
                        separated by spaces (e.g., -c t1 t2).
  -o <str>              Output file name. In case of multi-class segmentation, class-specific suffixes will be added. By default,suffix '_seg' will be added and output extension will be .nii.gz.

TASKS:
  -task <str>           Task to perform. It could either be a pre-installed task, task that could be installed, or a custom task. To list available tasks, run: sct_deepseg -list-tasks
  -list-tasks           Display a list of tasks that can be achieved. (default: False)
  -install-task {seg_sc_t2star,seg_mice_sc,seg_mice_gm,seg_tumor_t2,seg_tumor-edema-cavity_t1-t2}
                        Install models that are required for specified task.

PARAMETERS:
  -thr <float>          Binarize segmentation with specified threshold. Set to 0 for no thresholding (i.e., soft segmentation). Default value is model-specific and was set during optimization
                        (more info at https://github.com/sct-pipeline/deepseg-threshold).
  -r {0,1}              Remove temporary files. (default: 1)
  -largest KEEP_LARGEST
                        Keep the largest connected-objects from the output segmentation. Specify the number of objects to keep.To keep all objects, set to 0
  -fill-holes {0,1}     Fill small holes in the segmentation.
  -remove-small REMOVE_SMALL [REMOVE_SMALL ...]
                        Minimal object size to keep with unit (mm3 or vox). A single value can be provided or one value per prediction class. Single value example: 1mm3, 5vox. Multiple values
                        example: 10 20 10vox (remove objects smaller than 10 voxels for class 1 and 3, and smaller than 20 voxels for class 2).

MISC:
  -v <int>              Verbosity. 0: Display only errors/warnings, 1: Errors/warnings + info messages, 2: Debug mode (default: 1)
  -h, --help            Show this help message and exit

That's the patch that's currently in #3156. Joshua is right, there's probably a bunch of other scripts that need this treatment.

We are fighting upstream against linux conventions, here. argparse is designed in the GNU style: -s are options, others are not. If we were following those conventions, say with this patch

index 3ecfd674..fd11d47d 100644
--- a/spinalcordtoolbox/scripts/sct_deepseg.py
+++ b/spinalcordtoolbox/scripts/sct_deepseg.py
@@ -36,10 +36,11 @@ def get_parser():

     input_output = parser.add_argument_group("\nINPUT/OUTPUT")
     input_output.add_argument(
-        "-i",
+        "i",
         nargs="+",
         help="Image to segment. Can be multiple images (separated with space).",
-        metavar=Metavar.file)
+        #metavar=Metavar.file # unsure why I need to comment this out in this case
+       )
     input_output.add_argument(
         "-c",
         nargs="+",

then we'd have this usage:

$ sct_deepseg

--
Spinal Cord Toolbox (git-ng/3154-mandatory-options-0adeae975066247d057e3c28cce3e5f9b7c1c9ef*)

sct_deepseg 
--

sct_deepseg: error: the following arguments are required: i

usage: sct_deepseg [-c <file> [<file> ...]] [-o <str>] [-task <str>] [-list-tasks] [-install-task {seg_sc_t2star,seg_mice_sc,seg_mice_gm,seg_tumor_t2,seg_tumor-edema-cavity_t1-t2}]
                   [-thr <float>] [-r {0,1}] [-largest KEEP_LARGEST] [-fill-holes {0,1}] [-remove-small REMOVE_SMALL [REMOVE_SMALL ...]] [-v <int>] [-h]
                   i [i ...]

Segment an anatomical structure or pathologies according to the specified deep learning model.

INPUT/OUTPUT:
  i                     Image to segment. Can be multiple images (separated with space).
  -c <file> [<file> ...]
                        Type of image contrast. Indicates the order in which the images have been presented with -i. Optional if only one image is specified with -i. The contrasts should be
                        separated by spaces (e.g., -c t1 t2).
  -o <str>              Output file name. In case of multi-class segmentation, class-specific suffixes will be added. By default,suffix '_seg' will be added and output extension will be .nii.gz.

TASKS:
  -task <str>           Task to perform. It could either be a pre-installed task, task that could be installed, or a custom task. To list available tasks, run: sct_deepseg -list-tasks
  -list-tasks           Display a list of tasks that can be achieved. (default: False)
  -install-task {seg_sc_t2star,seg_mice_sc,seg_mice_gm,seg_tumor_t2,seg_tumor-edema-cavity_t1-t2}
                        Install models that are required for specified task.

PARAMETERS:
  -thr <float>          Binarize segmentation with specified threshold. Set to 0 for no thresholding (i.e., soft segmentation). Default value is model-specific and was set during optimization
                        (more info at https://github.com/sct-pipeline/deepseg-threshold).
  -r {0,1}              Remove temporary files. (default: 1)
  -largest KEEP_LARGEST
                        Keep the largest connected-objects from the output segmentation. Specify the number of objects to keep.To keep all objects, set to 0
  -fill-holes {0,1}     Fill small holes in the segmentation.
  -remove-small REMOVE_SMALL [REMOVE_SMALL ...]
                        Minimal object size to keep with unit (mm3 or vox). A single value can be provided or one value per prediction class. Single value example: 1mm3, 5vox. Multiple values
                        example: 10 20 10vox (remove objects smaller than 10 voxels for class 1 and 3, and smaller than 20 voxels for class 2).

MISC:
  -v <int>              Verbosity. 0: Display only errors/warnings, 1: Errors/warnings + info messages, 2: Debug mode (default: 1)
  -h, --help            Show this help message and exit
kousu commented 3 years ago

It would be good if we could regression-test this. We have unit_testing/test_deepseg.py but it calls the python directly rather than using subprocess, so grabbing stdout and the exit code are awkward. And then if we do grab stdout what do we look for to validate it's the help text and not something else?

joshuacwnewton commented 3 years ago

argparse lets us break the convention, by passing required=True:

While this is what we do for most of the scripts, my understanding was that -i was not made mandatory here because it's also proper usage to say sct_deepseg -list-tasks. We'd need a way to set it so that either could be required; setting just -i to be required would muddy things up.

Joshua is right, there's probably a bunch of other scripts that need this treatment.

I wouldn't necessarily sound the alarms yet. From what I've seen, -i is set to required=True in most scripts, because there isn't usually a need to have an alternate usage without -i... (Though I haven't checked each and every one, so I can't say for sure -- commenting on a not-work computer right now. 😅)

joshuacwnewton commented 3 years ago

Ah, wait, that reminds me. @jcohenadad actually came up with a workaround for exactly this problem!

https://github.com/neuropoly/spinalcordtoolbox/blob/639ca81438763346b9fd4271c3fb696e6febd44a/spinalcordtoolbox/scripts/sct_extract_metric.py#L48-L61

We faced the same problem here: We wanted to make -i required, but we also wanted to allow an argument to be used without -i, so we made an action that exits before -i could trigger an error for not being present.

joshuacwnewton commented 3 years ago

Now that I think about it, I'm wondering if this _ListLabelsAction is the best way to handle this, though.

Perhaps it would be clearer to just check if any of [-i, -list-tasks, -install-task] are present (likewise for sct_extract_metric), then throw an error if none are present.

jcohenadad commented 3 years ago

maybe @Drulex has an opinion on this?

kousu commented 3 years ago

argparse comes with https://docs.python.org/3.8/library/argparse.html#sub-commands built in. Conventionally, we would write it like this:

# testing subcommands with argparse

import sys
import argparse

def segment(args):
    pass

def list_tasks(args):
    pass

def install_task(args):
    pass

def main():
    parser = argparse.ArgumentParser()
    subcommands = parser.add_subparsers()

    segment_options = subcommands.add_parser('segment')
    segment_options.add_argument("-i", nargs="+", required=True)
    segment_options.set_defaults(func=segment)

    list_tasks_options = subcommands.add_parser('list-tasks')
    list_tasks_options.set_defaults(func=list_tasks)

    install_task_options = subcommands.add_parser('install-task')
    install_task_options.set_defaults(func=install_task)

    args = parser.parse_args(sys.argv[1:])
    args.func(args)

main()
$ python3 args.py install-task -h
usage: args.py install-task [-h]

optional arguments:
  -h, --help  show this help message and exit
$ python3 args.py segment -h
usage: args.py segment [-h] -i I [I ...]

optional arguments:
  -h, --help    show this help message and exit
  -i I [I ...]
$ 

This forces an API change (IMO an improvement because it's more in line with the standard GNUey conventions): we aren't:

  1. allowed to use a - in front of the subcommand names
  2. allowed to use a blank name for segment; there's at least one way to do default subcommands but it's tricky and I suspect it doesn't generate help text nicely; I'm still looking for a better way.
Drulex commented 3 years ago

I glanced at this quickly, here are some thoughts:

edit: looks like @kousu proposed something in line with what I was thinking (https://github.com/neuropoly/spinalcordtoolbox/issues/3154#issuecomment-758058973) as I was writing this.

joshuacwnewton commented 3 years ago

Since we are gearing up to release a (patch? minor?) quite soon, could we start with a non-breaking patch, then consider adding subcommands/mutually exclusive groups for the next major?

kousu commented 3 years ago

I found this old thread: https://bugs.python.org/issue9253. It looks like they tried to let argparse handle having default subcommands (problem #2) but never finished it? There might be some tricks buried in that thread we could use to get there though.

kousu commented 3 years ago

I can't get argparse to accept a default subcommand. Every way I've tried so far to hack it in has had sct_deepseg -i a b c try to parse a as the subcommand.

Sidestepping argparse for the subcommand argument works, kind of:

# testing subcommands with argparse

import sys
import argparse

def segment(args):
    print(args)
    pass

def list_tasks(args):
    print(args)
    pass

def install_task(args):
    print(args)
    pass

def main():
    subcommands = {}

    segment_options = subcommands[''] = argparse.ArgumentParser(description="Segment the images") 
    segment_options.add_argument("-i", nargs="+", required=True)
    segment_options.set_defaults(func=segment)

    list_tasks_options = subcommands['-list-tasks'] = argparse.ArgumentParser(description="List available segmenation tasks") 
    list_tasks_options.set_defaults(func=list_tasks)

    install_task_options = subcommands['-install-task'] = argparse.ArgumentParser(description="Do some stuff") 
    install_task_options.set_defaults(func=install_task)

    cmd = sys.argv[1] if len(sys.argv) > 1 else '' 
    parser = subcommands.get(cmd, segment_options)

    # drop cmd := sys.argv[1], since we're going to be using the subparsers now
    if cmd not in subcommands:
        # handle default case
        cmd = ''
    else:
        sys.argv.pop(1)

    args = parser.parse_args()
    args.func(args)

main()

but the help is generated wrongly:

$ python3 args.py -list-tasks -h
usage: args.py [-h]

List available segmenation tasks

optional arguments:
  -h, --help  show this help message and exit
$ python3 args.py -h
usage: args.py [-h] -i I [I ...]

Segment the images

optional arguments:
  -h, --help    show this help message and exit
  -i I [I ...]
kousu commented 3 years ago

https://stackoverflow.com/questions/46667843/how-to-set-a-default-subparser-using-argparse-module-with-python-2-7 suggests something pretty similar to what I have above, but using argparse in place of my looking directly at sys.argv. It, too, kinda works, but the help is wrong:

import argparse

def cmd1(args):
    print('cmd1', args)
def cmd2(args):
    print('cmd2', args)

parser1 = argparse.ArgumentParser()

parser1.add_argument("-i", "--info",  help="Display more information")

parser2 = argparse.ArgumentParser()
subparsers = parser2.add_subparsers(dest='cmd')

parserCmd1 = subparsers.add_parser("cmd1", help="First Command")
parserCmd1.set_defaults(func=cmd1)

parserCmd2 = subparsers.add_parser("cmd2", help="Second Command")
parserCmd2.add_argument("-o", "--output", help="Redirect Output")
parserCmd2.set_defaults(func=cmd2)

args, extras = parser1.parse_known_args()
if len(extras)>0 and extras[0] in ['cmd1','cmd2']:
    args = parser2.parse_args(extras, namespace=args)
    args.func(args)
else:
    print('doing system with', args, extras)
$ python3 args2.py -h
usage: args2.py [-h] [-i INFO]

optional arguments:
  -h, --help            show this help message and exit
  -i INFO, --info INFO  Display more information
$ python3 args2.py cmd1 -h
usage: args2.py [-h] [-i INFO]

optional arguments:
  -h, --help            show this help message and exit
  -i INFO, --info INFO  Display more information
kousu commented 3 years ago

This version is closer:

# testing subcommands with argparse

import sys
import argparse

def segment(args):
    print(f"segment({args})")

def list_tasks(args):
    print(f"list_task({args})")
    pass

def install_task(args):
    print(f"install_task({args})")
    pass

def main():
    parser = argparse.ArgumentParser()
    subcommands = parser.add_subparsers()
    subcommands.required=False

    segment_options = subcommands.add_parser('segment')
    segment_options.add_argument("-i", nargs="+", required=True)
    segment_options.set_defaults(func=segment)

    list_tasks_options = subcommands.add_parser('list-tasks')
    list_tasks_options.set_defaults(func=list_tasks)

    install_task_options = subcommands.add_parser('install-task')
    install_task_options.set_defaults(func=install_task)

    if len(sys.argv)>1:
        if sys.argv[1] in ["-h", "--help"]:
            pass
        elif sys.argv[1] not in subcommands.choices:
            # default to 'segment'
            sys.argv.insert(1, 'segment')
    else:
        parser.error("missing required arguments")

    args = parser.parse_args()
    args.func(args)

main()
$ python3 args3.py # this doesn't print the help here, but when combined with our fix in https://github.com/neuropoly/spinalcordtoolbox/pull/3139 it'll do the right thing
usage: args3.py [-h] {segment,list-tasks,install-task} ...
args3.py: error: missing required arguments

$ python3 args3.py -h
usage: args3.py [-h] {segment,list-tasks,install-task} ...

positional arguments:
  {segment,list-tasks,install-task}

optional arguments:
  -h, --help            show this help message and exit

$ python3 args3.py segment -h
usage: args3.py segment [-h] -i I [I ...]

optional arguments:
  -h, --help    show this help message and exit
  -i I [I ...]

$ python3 args3.py list-tasks -h
usage: args3.py list-tasks [-h]

optional arguments:
  -h, --help  show this help message and exit
$ python3 args3.py install-task -h
usage: args3.py install-task [-h]

optional arguments:
  -h, --help  show this help message and exit

It still can't do -install-task, because argparse sees the - and jumps to parsing it as an option:

$ python3 args3.py -install-task -h
usage: args3.py segment [-h] -i I [I ...]

optional arguments:
  -h, --help    show this help message and exit
  -i I [I ...]

Even if I change the things:

@@ -20,14 +20,14 @@
     subcommands = parser.add_subparsers()
     subcommands.required=False

-    segment_options = subcommands.add_parser('segment')
+    segment_options = subcommands.add_parser('-segment')
     segment_options.add_argument("-i", nargs="+", required=True)
     segment_options.set_defaults(func=segment)

-    list_tasks_options = subcommands.add_parser('list-tasks')
+    list_tasks_options = subcommands.add_parser('-list-tasks')
     list_tasks_options.set_defaults(func=list_tasks)

-    install_task_options = subcommands.add_parser('install-task')
+    install_task_options = subcommands.add_parser('-install-task')
     install_task_options.set_defaults(func=install_task)

     if len(sys.argv)>1:
@@ -35,7 +35,7 @@
             pass
         elif sys.argv[1] not in subcommands.choices:
             # default to 'segment'
-            sys.argv.insert(1, 'segment')
+            sys.argv.insert(1, '-segment')
     else:
         parser.error("missing required arguments")     

it still doesn't work:

$ python3 args3.py -install-task
usage: args3.py [-h] {-segment,-list-tasks,-install-task} ...
args3.py: error: unrecognized arguments: -install-task
Drulex commented 3 years ago

I have a piece of code that does this. Give me a few minutes to adapt to this example. (but you're right, we would need to drop the - however..)

edit: see below

A minimal piece of code that could do what we want:

import sys
import argparse
import logging

def main():

    parser = argparse.ArgumentParser(
     description="sct_deepseg",
    )

    parser.add_argument("--log-level",
     default="INFO",
     help="Logging level (eg. INFO, see Python logging docs)",
    )

    subparsers = parser.add_subparsers(
     help='the function; type "%s FUNCTION -h" for function-specific help' % sys.argv[0],
    )

    def do_a(args):
        print("doing a")
        print(args)

    def do_b(args):
        print("doing b")
        print(args)

    a_subp = subparsers.add_parser(
     "a",
     help="Help for function A",
     description='This is a description for function a',
    )
    a_subp.set_defaults(func=do_a, help="help for a")

    a_subp.add_argument('--some-option-a',
     help="option for A only"
    )

    a_subp.add_argument('--some-other-option-with-default',
     choices=(1, 2, 3),
     default=1,
    )

    b_subp = subparsers.add_parser(
     "b",
     help="Help for function B",
     description='This is a description for function b',
    )

    b_subp.set_defaults(func=do_b)

    b_subp.add_argument('--some-option-b',
     help="option for B only"
    )

    b_subp.add_argument('--some-other-option-with-default',
     choices=(1, 2, 3),
     default=2,
    )

    args = parser.parse_args()

    logging.basicConfig(
     datefmt="%Y%m%dT%H%M%S",
     level=getattr(logging, args.log_level),
     format="%(asctime)-15s %(name)s %(levelname)s %(message)s"
    )

    if getattr(args, 'func', None) is None:
        parser.print_help()
        return 1
    else:
        return args.func(args)

if __name__ == "__main__":
    ret = main()
    raise SystemExit(ret)

example usage

[drulex@gentoo-p53 tmp]  $  python subparsers_example.py
usage: subparsers_example.py [-h] [--log-level LOG_LEVEL]
                             [--common-option {sample1,sample2}]
                             {a,b} ...

sct_deepseg

positional arguments:
  {a,b}                 the function; type "subparsers_example.py FUNCTION -h"
                        for function-specific help
    a                   Help for function A
    b                   Help for function B

optional arguments:
  -h, --help            show this help message and exit
  --log-level LOG_LEVEL
                        Logging level (eg. INFO, see Python logging docs)
  --common-option {sample1,sample2}

[drulex@gentoo-p53 tmp]  $  echo $?
1

[drulex@gentoo-p53 tmp]  $  python subparsers_example.py -h
usage: subparsers_example.py [-h] [--log-level LOG_LEVEL]
                             [--common-option {sample1,sample2}]
                             {a,b} ...

sct_deepseg

positional arguments:
  {a,b}                 the function; type "subparsers_example.py FUNCTION -h"
                        for function-specific help
    a                   Help for function A
    b                   Help for function B

optional arguments:
  -h, --help            show this help message and exit
  --log-level LOG_LEVEL
                        Logging level (eg. INFO, see Python logging docs)
  --common-option {sample1,sample2}

[drulex@gentoo-p53 tmp]  $  echo $?
0

[drulex@gentoo-p53 tmp]  $  python subparsers_example.py a
doing a
Namespace(common_option=None, func=<function main.<locals>.do_a at 0x7fe38b5c7830>, help='help for a', log_level='INFO', some_other_option_with_default=1)

[drulex@gentoo-p53 tmp]  $  python subparsers_example.py a -h
usage: subparsers_example.py a [-h] [--some-other-option-with-default {1,2,3}]

This is a description for function a

optional arguments:
  -h, --help            show this help message and exit
  --some-other-option-with-default {1,2,3}

[drulex@gentoo-p53 tmp]  $  python subparsers_example.py b -h
usage: subparsers_example.py b [-h] [--some-option-b SOME_OPTION_B]
                               [--some-other-option-with-default {1,2,3}]

This is a description for function b

optional arguments:
  -h, --help            show this help message and exit
  --some-option-b SOME_OPTION_B
                        option for B only
  --some-other-option-with-default {1,2,3}

[drulex@gentoo-p53 tmp]  $  python subparsers_example.py --log-level DEBUG a
doing a
Namespace(common_option=None, func=<function main.<locals>.do_a at 0x7f1193883830>, help='help for a', log_level='DEBUG', some_other_option_with_default=1)

But you're right @kousu, we would need to break the API due to the current non-standard usage of options in the CLI.

joshuacwnewton commented 3 years ago

Update: Only 2 new scripts needed a fix. So, the total number of scripts that can't use required=True is 3:

I've updated #3156 to include a quick patch + a test.

kousu commented 3 years ago

Great, thanks @Drulex.

joshuacwnewton commented 3 years ago

Thoughts on creating a "move away from single dash" open issue that we potentially address when we're planning for a major release?

Since subcommands would necessarily involve that larger API break, it might be better to shelve it for now.

Also, using single dash has had a number of other consequences. Some workarounds are listed in this Wiki section that could also be fixed by moving away from single dash..

jcohenadad commented 3 years ago

Thoughts on creating a "move away from single dash" open issue that we potentially address when we're planning for a major release?

It's not the first time (and not the last) someone raises the question-- i have strong opinions about this-- could you pls open an issue so we can have a track record of the various opinions-- thanks 😊

joshuacwnewton commented 3 years ago

It's not the first time (and not the last) someone raises the question-- i have strong opinions about this-- could you pls open an issue so we can have a track record of the various opinions-- thanks blush

Done in https://github.com/neuropoly/spinalcordtoolbox/issues/3159. :slightly_smiling_face: