Command-line argument design document

[billsacks]

Description of changes

Start a design document for command-line arguments

Specific notes

Contributors other than yourself, if any: Various discussions at ctsm-software meeting and elsewhere

CTSM Issues Fixed (include github issue #): none

Are answers expected to change (and if so in what way)? No

Any User Interface Changes (namelist or namelist defaults changes)? No

Testing performed, if any: none

[billsacks]

You can view the rendered version here: https://github.com/billsacks/ctsm/blob/doc_arguments/doc/design/command_line_arguments.rst

I don't know why the final "Rationale" got bold & italics applied to it; that wasn't intentional.

Update (2022-01-26) This is now here: https://github.com/billsacks/ctsm/blob/doc_arguments/doc/design/python_script_user_interface.rst

[ekluzek]

I think this is good to have so that we can remind ourselves of what we've decided on these issues and have a place in the repository to look for it. I suspect we might need to check to see what scripts we have that don't completely follow these recommendations.

[negin513]

@billsacks Thanks for creating this docs from our discussions. My intention with creating and showing the Jupyter notebook at the CTSM SE meeting was exactly the same:

• improving the efficiency of code review process by avoiding conflicting reviewers' opinions.
• avoiding conflicting UI in the CTSM python codes.
• avoiding double standards in the python codes.

[ekluzek]

Yes, the way I see it @negin513's Jupyter notebook is how we were able to establish the standards that @billsacks listed in the document here. And it was really helpful to be able to see the notebook to see how this stuff worked.

That makes me wonder if saving the notebook in the repository would have value? I'm not sure that it does, but might be good to discuss. I also really like @negin513's articulation of the list of reasons for this. It's been difficult for us to get to this standardization, but I think it will really help for moving forward.

@billsacks should we just approve this to come to master right now?

[billsacks]

should we just approve this to come to master right now?

I invited (via email) anyone who is interested to review this. I figure we'll give people at least a few days, then it can come to master if that sounds okay to you.

[ekluzek]

I like the use of the POSIX standard that you pointed to. But, that leads to one question, and something that we should clarify.

Does python arg parsing already handle the ending of options with a "--"?

I think we should also give some clarifications on when you use a single dash "-v" and when you use double dashes "--verbose".

Do we also have a standard in regard to a required input option that still uses the option syntax with dashes "--require-option"? Or are we saying that any required option is in the argument list without a proceeding "--"? With long argument lists it's helpful to use the option argument syntax even if the argument is required.

[negin513]

One point that is missing here is we have decided to avoid using negative wording as much as possible. Basically, we have decided to convert flags to the positive wording instead of negative wording.
So instead of --no-feature if there is another translation with positive definition, we will use that.

[billsacks]

Does python arg parsing already handle the ending of options with a "--"?

Not sure

I think we should also give some clarifications on when you use a single dash "-v" and when you use double dashes "--verbose".

For that example, both are supported. I feel like the general convention in Unix utilities is: provide single-letter short options for the most common options, but usually/always also provide long options.

Do we also have a standard in regard to a required input option that still uses the option syntax with dashes "--require-option"? Or are we saying that any required option is in the argument list without a proceeding "--"? With long argument lists it's helpful to use the option argument syntax even if the argument is required.

In most situations, I'd say still use a named argument like --required-option VALUE. Is that what you're saying, too? The exception I can see is if there are one or two fundamental arguments to a script – e.g., a script that operates on a file can use that file as a positional argument. We can discuss more if you'd like.

[billsacks]

One point that is missing here is we have decided to avoid using negative wording as much as possible. Basically, we have decided to convert flags to the positive wording instead of negative wording.
So instead of --no-feature if there is another translation with positive definition, we will use that.

Thanks for your comments on this, @negin513 . I do say, "Prefer phrasing these as positives whenever possible (e.g., --allow-multiple-pfts instead of --no-single-pft)", but I'd welcome suggestions for more clear wording if you feel there's a better way to say this.

[ekluzek]

I took a look through all of the scripts in CTSM to see what would need to be updated. I think overall we don't need to make changes. There are some small working scripts that are probably OK as is (or maybe should be removed), like bld/queryDefaultNamelist.pl. It's going to be eventually replaced.

I think bld/build-namelist should be updated to allow "--silent" in addition to "-s". There is also a problem with the perl based scripts in that the long argument names still use a single dash rather than a double dash. So we might need to see if we can make that the same as the python standard. build-namelist is also a script where required options (using dash syntax).

I'd say that the cime scripts buildlib and buildnml follow the standard, so don't need to change. Although they also don't work to even give help if CIMEROOT is not set. I would like to see that "./buildnml --help" would work even if CIMEROOT wasn't set.

[ekluzek]

To address @negin513 point about preferring positive phrasing, I'd just add a line something like the following to the end.

* Whenever possible avoid the use of --no-feature arguments as they often are confusing. Try to express the same thing using positive wording.

[billsacks]

* Whenever possible avoid the use of --no-feature arguments as they often are confusing. Try to express the same thing using positive wording.

Can you please make whatever change you would suggest? Please note that it already says, "Prefer phrasing these as positives whenever possible (e.g., --allow-multiple-pfts instead of --no-single-pft)". I'm not clear on whether you're suggesting changing the wording of that sentence, adding a new bullet item, etc. I think it will be easiest if you just make the changes as you see fit.

[negin513]

I think this should be possibly organized into two sections:

1. Switch Flags:
   Flags that are not expecting any values which for turning on and off a feature. The discussions that we had regarding --feature and --no-feature will go under this section.
2. Value Flags:
   Here we will talk about the standards we defined for the flags that need a value for example ``--dompft [DOMPFT]`.

For both of them I think it might worth adding some examples for clarifying what we discussed.

[wwieder]

👍 @negin513, adding real examples from the script is very helpful for me. As an aside examples using foobar kind of make be frusterated for no logical reason :(

[wwieder]

@negin513 's notebook does have nice examples that were helpful in understanding the logical/technical challenges of this topic too (for non-SEs).

[adrifoster]

Hey all,

Thanks for working on this. As we write more documentation like this, and perhaps go over old documentation, I think it would be good if we all consider the topic of "the curse of knowledge". That is, the cognitive bias that occurs when experts assume that others have the same background knowledge as them, and how difficult it is for experts to remember what it was like to not have this knowledge.

As experts in various fields I think we all fall prey to this, and without adequate care and training it can be extremely difficult to overcome. Here is a nice video about it: https://www.youtube.com/watch?v=XQivffNIjQo.

[ekluzek]

Interesting video @adrifoster, useful for a ton of things that we do. Especially since we are so interdisciplinary in our work in CTSM. We constantly need to talk to people whose expertise is different than ours.

@negin513 @wwieder and @billsacks. Since @billsacks suggested I commit directly my suggestion I went ahead and did that and also took in suggestions from @negin513 and @wwieder. I also realized that two additional standard options are --silent (-s) and --help (-h). Help is always useful, and silent is sometimes. the infrastructure in ctsm_logging doesn't help you with either of those, but I wonder if it should be extended to handle --silent. But, that could also be an optional extension only for those scripts where it's useful.

To advocate for --silent it's sometimes useful if you want output completely restricted for example when you are capturing output (for example: setenv DION_LOC_ROOT=./xmlquery --silent DIN_LOC_ROOT). When you have both silent and verbose you implicitly then have three levels of logging: silent, normal, verbose. And then debug might add a fourth level on top of that debug-level. The debug option can be used both for adding a higher level of verbosity and also for doing a dry-run type scenario which is useful for debugging when some of the internal steps take a long time. We could be more clear and only use --debug to add another level of logging, and use --dry-run for the later type of use of --debug.

Is this better? Any comments about my suggestions on --silent, --debug, and --dry-run?

[wwieder]

@ekluzek thanks for just making the changes. I don't have much more to say on --silent, --debug, etc.
I don't know what --dry-run is for (or what we're even talking about now), but I don't think it's critical right now.

@adrifoster I also like the video, but wondered what you were trying to encode by sharing it? Was it something specific to the subset_data PR, this design document, our broader workflow and group dynamic, or a combination of these? This is more of a meta-cognition question that's likely not well suited to discuss on a git issue :). Regardless, the suggestion of emphasizing the main idea up front is helpful in communication among experts and non-experts.

[adrifoster]

@adrifoster I also like the video, but wondered what you were trying to encode by sharing it? Was it something specific to the subset_data PR, this design document, our broader workflow and group dynamic, or a combination of these? This is more of a meta-cognition question that's likely not well suited to discuss on a git issue :). Regardless, the suggestion of emphasizing the main idea up front is helpful in communication among experts and non-experts.

I thought of this while reading these comments and such, but I did mean this as more about our broader workflow and group dynamic, so agree that I should have brought it up on a different platform!

[billsacks]

@ekluzek thank you for your changes. I'm happy with them.

I also committed some new pieces following some very good suggestions from @negin513 that some additional python standards should be documented here.

--help/-h is included automatically when using argparse, which is why I didn't think it needed to be called out explicitly here, but it doesn't hurt to call it out. You're right that --silent could be a good addition; would you imagine that that sets the logging level to error, so that warnings are suppressed? I'm not sure I understand what you're thinking based on your xmlquery example, because I don't see any difference of adding --silent in that xmlquery example. For that use case, though, I'm thinking that it helps make clear the distinction between when to use print vs. logging – because in that example, you do still want some output from xmlquery when running with --silent, but you only want the fundamental output – i.e., the result of actually running xmlquery – and I guess you want to suppress anything else?

I agree that --dry-run should be separate from --debug: I use those in very different circumstances and wouldn't usually want them to be combined into a single option. Personally, I usually use --dry-run to make sure a command is going to do the right thing before running it for real, not generally in debugging scenarios.

[billsacks]

I have updated the logging section based on yesterday's discussion. The general points are the same as before, but I have added some clarifications. Edits to make this more clear or concise are welcome if anyone feels they are needed.

[adrifoster]

I have updated the logging section based on yesterday's discussion. The general points are the same as before, but I have added some clarifications. Edits to make this more clear or concise are welcome if anyone feels they are needed.

These updates look great to me. The only thing I might suggest is having some extra clarification for logging.info vs. logging.debug. We say it at the end, but maybe we could expand on it a bit, and add an example. From our discussion it seemed like info should be used for higher-level information about program state, information about user, machine, where files are coming from. And debug should be used for fine-grained information about program state, or other information that really only developers getting into the guts of the the code would want to know.

This, again, is still subjective, but it is at least good to keep in mind.

[billsacks]

Thanks for your review, @adrifoster . Good point about adding some clarification of debug vs. info – that's a good idea. Would you be willing to add some text on this? I think you should have permission to push to my branch; let me know if you cannot.

[adrifoster]

Thanks for your review, @adrifoster . Good point about adding some clarification of debug vs. info – that's a good idea. Would you be willing to add some text on this? I think you should have permission to push to my branch; let me know if you cannot.

Sure can do!