trevorld / r-argparse

command-line optional and positional argument parser
GNU General Public License v2.0
103 stars 11 forks source link

nargs='+' seems to fail to short-circuit when missing #31

Closed vreuter closed 3 years ago

vreuter commented 3 years ago

Unlike nargs='*', + should cause an error at time of CLI opt/args parsing, but this appears not to be so. Instead, a program with an option defined this way appears to continue running even if the option's omitted when run from command-line, at least via Rscript.

vreuter commented 3 years ago
> sessionInfo()
R version 4.0.3 (2020-10-10)

...

other attached packages:
[1] argparse_2.0.3
trevorld commented 3 years ago

Please give a minimal reproducible example. Not reproducible on my end with R 3.6.3 nor R 4.0.5

trevorld commented 3 years ago

Disregard, I can reproduce it.

trevorld commented 3 years ago

Can you please give a minimal reproducible example of argparse not throwing an error in that case? I can't reproduce that on my end with R 3.6.3 nor R 4.0.5.

I can reproduce the fact that the script keeps running on but this depends on the setting of the "error" option. In particular my script will stop running if options(error=NULL) (the default) but will keep on running if options(error=quote(utils::recover()).

Are you using Rscript without the --vanilla flag and have set up the error option in a .Rprofile file?

vreuter commented 3 years ago

Maybe mixing the implicit "required" of nargs='+' with options that make it explicit with the keyword is causing the quirk?

$ cat tinytest.R 
library(argparse)

parser <- ArgumentParser()
parser$add_argument("--outdata", required=TRUE, help="Path to output data file")
parser$add_argument("--outplot", required=TRUE, help="Path to output plot file")
parser$add_argument("--plotTimes", nargs='+', help="Times to plot")
opts <- parser$parse_args()

message("Parsed!")
$ Rscript tinytest.R 
Error in .stop(output, "parse error:") : parse error:
usage: tinytest.R [-h] --outdata OUTDATA --outplot OUTPLOT
                  [--plotTimes PLOTTIMES [PLOTTIMES ...]]
tinytest.R: error: the following arguments are required: --outdata, --outplot
Calls: <Anonymous> -> .stop
Execution halted

$ Rscript tinytest.R --outdata tmp.txt 
Error in .stop(output, "parse error:") : parse error:
usage: tinytest.R [-h] --outdata OUTDATA --outplot OUTPLOT
                  [--plotTimes PLOTTIMES [PLOTTIMES ...]]
tinytest.R: error: the following arguments are required: --outplot
Calls: <Anonymous> -> .stop
Execution halted

$ Rscript tinytest.R --outdata tmp.txt --outplot tmp.png
Parsed!
vreuter commented 3 years ago

I expected the first two usages above to fail, but the third one is where I expected a parse error but execution got past it.

vreuter commented 3 years ago

Hmm, I haven't modified a .Rprofile. Sorry, just read the rest of your reply:

I can reproduce the fact that the script keeps running on but this depends on the setting of the "error" option. In particular my script will stop running if options(error=NULL) (the default) but will keep on running if options(error=quote(utils::recover()).

Are you using Rscript without the --vanilla flag and have set up the error option in a .Rprofile file?

I hadn't been using --vanilla, but I tested just now the same behavior using that option to Rscript.

vreuter commented 3 years ago

And from my home directory, find $PWD -type f -name ".Rprofile" yields no hits

vreuter commented 3 years ago

After adding a message call to the end to the tinytest script to check out the option setting:

$ Rscript --vanilla tinytest.R --outdata tmp.txt --outplot tmp.png
Parsed!
is.null(getOption('error')): TRUE

$ Rscript tinytest.R --outdata tmp.txt --outplot tmp.png
Parsed!
is.null(getOption('error')): TRUE
vreuter commented 3 years ago

FWIW diagnostics-wise, explicitly specifying required=TRUE alongside nargs='+' does make the option required, i.e. halting execution at time of parse.

trevorld commented 3 years ago

The issue is that argparse assumes that optional arguments are optional (unless required is set to TRUE). So no error is thrown if an optional argument is not present. Note this is exactly the same behavior as the python version of argparse.

In your case you are telling argparse that --plotTimes is an optional argument that is not required to be present, but if it is present it must be passed a non-zero number of elements. Note that argparse does throw an error if the optional argument --plotTimes is present and you pass it zero elements. This seems to be the expected/desired behaviour so I am closing this issue.

Your options seem to be:

vreuter commented 3 years ago

Interesting, you're right, thanks! Sorry for the hassle. I'll submit a ticket for Python argparse docs to change, as the current docs are misleading about + vs. *, since the error is conditional on the option name prefix as you noted.

'+'. Just like '*', all command-line args present are gathered into a list. Additionally, an error message will be generated if there wasn’t at least one command-line argument present. For example

vreuter commented 3 years ago

For future reference, here's the Python issue