-
Notifications
You must be signed in to change notification settings - Fork 26
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
LoosOptions can't print fullhelp and exit if there are required arguments #72
Comments
This is partially but not completely fixed by #76. I got rid of all of the "required" flags on options in LoosOptions, since their presence breaks everything and it doesn't make sense to invoke a specific kind of option and not supply an argument (I suppose I need to add manually checking). I'm not ready to close this issue yet though because at least a couple of other tools (rare-event-detection.py, perhaps others) still have some required options. |
Not exactly sure what you mean by ' it doesn't make sense to invoke a specific kind of option and not supply an argument (I suppose I need to add manually checking).' In the context of normal argparse, required positional arguments can be added to the parser, and it won't screw up your ability to call the tool with the built '-h' flag. Did you mean that the word option doesn't make sense for required args? Because I just thought that was the parlance for command-line parsers that had the capacity for optional args...certainly the C++ version uses that term even though it can represent positional args. Seems to me like having positionals makes a lot of sense. I've always advocated having positional arguments for required input for a tool. If one finds oneself in a situation with many positional arguments, I usually think it's time to use a config file (JSON is pretty easy for python) instead of having CLI. Or maybe write two tools...I think my limit for different required args is like four or five, but that's kind of a style thing. Could we just put the huge fullhelp string into the argparser as its help? If a user wants the 'short help', they can still get that by running the tool with no input... |
Required arguments break the "no arguments is equivalent to -h" behavior, so we need to root them out everywhere and either replace them with positionals or do some other checking. This will require some testing, which is why I haven't closed this issue yet. If you look at the commit, what I changed was some of the methods in LoosOptions, eg modelSelectionOptions, that defined options with the required flag. The idea was that if you were doing it that way, those options had to be supplied or the program should exit, but unfortunately the way argparse is implemented that breaks help. I think modelSelectionOptions should perhaps be deprecated in favor of modelSelectionPositionalOptions. The alternative is that the tool itself has to check, which defeats the goal of the pre-packaged options in the first place. |
Describe the bug
The tool
contact_distance.py
, while having apparently correct usage of the description and fullhelp strings used to initialize loos options, is unable to print the fullhelp. Instead, it prints:I think what's happening here is that it's doing its program options logic in a way that doesn't respect whether there's been a fullhelp requested, The parser realizes that a number of required args aren't present and throws the corresponding error, instead of printing full-help and exiting. I'm not exactly sure how to make sure that fullhelp gets printed first. For what it's worth, 'help' has a special status that avoids this issue, so the way argparse handles the
-h
flag might be the place to start on running this down.To Reproduce
Steps to reproduce the behavior:
contact_distance.py --fullhelp
in a functional loos environment with that tool in the path.Expected behavior
I expected it to print the fullhelp that is saved as that variable name in the source code.
LOOS version and platform
f647665 linux conda install x86
Additional context
none
The text was updated successfully, but these errors were encountered: