dirac-institute / sorcha

An open-source community LSST Solar System Simulator
Other
18 stars 17 forks source link

add in extra message to tell the user how to get help when trying to use the CLI #1061

Open mschwamb opened 2 weeks ago

mschwamb commented 2 weeks ago

Fixes #1019

This PR If a user is new to linux/unix they might not be sure what to do to get the detailed help content from the CLI adds in SorchaArgumentParser class a subclass of the argparse.ArgumentParser that adds in an extra message to tell the user to do a sorcha verb -h .

I also modified the prog keyword so that it reflects the CLI commands since the user won't use sorcha-run they will be typing sorcha run (for example)

Review Checklist for Source Code Changes

mschwamb commented 2 weeks ago

Since I'm mucking with the CLI @mjuric can you take a look? Should take you 2 minutes. Some people using the CLI thought it wasn't clear what to do if you ran sorcha-run with no arguments. So I added an extra message to STDERR if there's no user provided arguments.

mschwamb commented 1 week ago

I even know what you're talking about. So maybe I did learn something python class stuff. I agree, your way is more elegant. I was thinking we didn't need to do this for all the times that someone screwed up the arguments, but I see your way is classier. I'll see where I get to and request a review when I've implemented this. Though it's gonna take me a bit to look at the python class tutorials again for me to do this/ double check what you've written will work (it probably will)

mjuric commented 1 week ago

FWIW, your current solution is also OK, if you want to quickly merge this PR & move the generalization to "future enhancements".

I'd offer to implement this myself but I'm underwater with return travel & other things for a few more days :(.

mschwamb commented 1 week ago

Nah. This is good for my soul. I'm back to having some energy, so I'm gonna try to get to this tonight/this week. I need to do something useful rather than emails, telecons, and commenting on paper drafts. So this is a good small task for me.

mschwamb commented 5 days ago

I did some reading implemented this and I'm getting this behavior that works but the output is a bit odd:

usage: ArgumentParser(prog='sorcha-run', usage=None, description='Run a simulation.', formatter_class=<class 'argparse.ArgumentDefaultsHelpFormatter'>, conflict_handler='error', add_help=True) [-h] -c C -o O -ob OB -p P -pd PD [-er ER] [-ew EW] [-ar AR] [-cp CP] [-f] [-s S] [-t T] [-v] [-st ST] For more detailed help, use the -h flag ArgumentParser(prog='sorcha-run', usage=None, description='Run a simulation.', formatter_class=<class 'argparse.ArgumentDefaultsHelpFormatter'>, conflict_handler='error', add_help=True): error: the following arguments are required: -c/--config, -o/--outfile, -ob/--orbit, -p/--params, -pd/--pointing_database Error: Command 'sorcha-run' failed with exit code 2.

instead of

usage: sorcha-run [-h] -c C -o O -ob OB -p P -pd PD [-er ER] [-ew EW] [-ar AR] [-cp CP] [-f] [-s S] [-t T] [-v] [-st ST] For more detailed help, use the -h flag sorcha-run: error: the following arguments are required: -c/--config, -o/--outfile, -ob/--orbit, -p/--params, -pd/--pointing_database Error: Command 'sorcha-run' failed with exit code 2.

I implemented


class sargparse(argparse.ArgumentParser):
    def __init__(self, *argv, **kwargs):
       super().__init__(*argv, **kwargs)

    def print_usage(self,file=None):
        super().print_usage(file)
        print("For more detailed help, use the -h flag",file=sys.stderr)

    def main():
    parser = argparse.ArgumentParser(
        formatter_class=argparse.ArgumentDefaultsHelpFormatter, description="Run a simulation."
    )

    parser=sargparse(parser)
    .......

I think this change makes it more confusing with the "usage: ArgumentParser(prog='sorcha-run', usage=None, description='Run a simulation.', formatter_class=<class 'argparse.ArgumentDefaultsHelpFormatter'>, conflict_handler='error', add_help=True)" bit. If you have ideas @mjuric how to get around this let me know. Otherwise I think it's easier to do what I implemented though I'll short the message to what you've written.

mschwamb commented 5 days ago

I figured it out. I'll get the PR in, in about half an hour.

mschwamb commented 4 days ago

I finally actually figured it out. Thanks to help from @bernardinelli and also the guidance of the code above. I'll also tag @bernardinelli in the review