-
Notifications
You must be signed in to change notification settings - Fork 61
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
CLI default value appears to be ignored #498
Comments
re #498 It appears that defaults are ignored for boolean SEM parameters. Even though the parameter was set to true in https://github.com/QIICR/dcmqi/blob/2990dbd01d1fa61c16777950732ff19dc87318e3/apps/seg/itkimage2segimage.xml#L58-L65, the default reported by the executable was 0. Local tests show that integer parameters do not have this problem.
re #498 It appears that defaults are ignored for boolean SEM parameters. Even though the parameter was set to true in https://github.com/QIICR/dcmqi/blob/2990dbd01d1fa61c16777950732ff19dc87318e3/apps/seg/itkimage2segimage.xml#L58-L65, the default reported by the executable was 0. Local tests show that integer parameters do not have this problem. Also switch to the most recent has for SlicerExecutionModel.
It is not possible to set default value for a Boolean flag. If you don't specify it then it will be always false (how else would you be able to set it to false?). Default value is just a hint for GUI generation. I agree that the help text is confusing, it should be removed or expressed differently. For example, it could be |
I guess it may make sense, but it for sure confused me. I think ideally there should be some built-time check or validation that would alert the developer that |
I see what you mean. This is super confusing. This essentially means that this specific parameter is only meaningful for GUI use of SEM, while the developer may not even be aware about GUI and only care about CLI use of the module. I understand it is not possible/difficult to fix. |
The misleading help text is a bug, but that should be fixed (a simple change). The other thing that causes confusion is that Boolean values are specified by just adding the parameter name ( |
Despite "skip" flag assigned default value "true" in XML
https://github.com/QIICR/dcmqi/blame/master/apps/seg/itkimage2segimage.xml#L58-L65
the actual default value is set to 0
The text was updated successfully, but these errors were encountered: