Negative numbers cannot be passed as arguments since 0.6
Thank you for the very nice and useful package!
I have been using the package since version 0.4, also for parsing negative numeric arguments, but since version 0.6, negative numeric arguments cannot be passed through the command line.
For example, save the following script as negnum.R:
p <- argparser::arg_parser('negative numeric issue')
p <- argparser::add_argument(p, '--numarg',
help = 'number help',
default = -0.9)
parsed <- argparser::parse_args(p)
print(parsed[['numarg']])
Then, try:
Rscript negnum.R --numarg '-0.7'
You get the following error:
Error in (function (object, class, nargs) :
Invalid argument value: expecting numeric but got: (-0.7).
Calls: <Anonymous> -> mapply -> <Anonymous>
Execution halted
I believe that the issue arises from the restrictive regular expressions in argparser.R, e.g.,
"^(([0-9]+)|([0-9]*\\.[0-9]+)|-?Inf)$"
I think the regular expression should at least include an optional negative sign and maybe also optional spaces before the sign and/or number. What do you think?
Comments (10)
-
repo owner -
reporter Great, thanks for the fix! You could also consider adding unit tests in a tests directory of your package to avoid regression.
-
repo owner - changed status to resolved
-
Hi David, thank you for your nice package. I am afraid the bug is not entirely resolved yet - your parsing of numerical options is still too restrictive. I need to give some option the value 1e-6, and that cannot be parsed by your regexp, neither the old nor the new one. I have tried to write the default as 0.000001, but apparently this too gets automatically converted to 1e-6 by R while parsing… Also can I ask you why you need to check things with a regexp? Cannot you just trust R if it says the input is numeric? Anyway it would be great if you could please fix this case too - thank you.
-
@David Shih Just in case that is useful, I have fixed my local copy by changing the regexp to
"^([-+]?(([0-9]+)|([0-9]*\\.[0-9]+([eE][+-][0-9]+)?)|Inf))|NA|NaN$"
-
repo owner R -e "commandArgs(trailingOnly=TRUE)" --args 0.0000000001
yields
[1] "0.0000000001"
So, avoiding scientific notation at the command line should have worked, because R does not do any string conversion in
commandArgs
, which is whatparse_args
uses. Maybe your issue is unrelated?The reason for the regex is for input error detection (a previous feature request). I originally wanted to keep the code simple and rely on R’s automatic conversion, but that means user input gets turned into
NA
with a warning message.I think I’ll turn off input error detection by default, and allow an option to turn it on.
I don’t think you got the regex quite right, because the period and the sign should be optional in the scientific notation. I’ll try to find a better regex.
-
repo owner Try commit 1de8683b.
The updated regex seems to allow scientific notation.
-
I think the issue is related. If I say
default=0.000001
as an argument to your functions i still get
[default: 1e-06]
in the usage, which seems to indicate that there is some automatic conversion going on internally at some stage. So I guess
R -e "commandArgs(trailingOnly=TRUE)" --args 0.0000000001
is somehow not sufficient to test what is going on - a short program using your library would be more appropriate. And anyway if I modify the regexp everything works fine for me, so the problem seems to be there. You are right about the regexp though - the additional
([eE][+-][0-9]+)?)
block should be following the definition for the rest of the number, not be inside brackets as I wrote it. But you get the idea I guess…
-
Oh, crossing posts - yes, I agree, your latest commit should fix the issue. Thank you!
-
Tested with 1de8683b, and it works - appreciated.
- Log in to comment
Thank you for this bug report, and I apologize for my insufficient testing.
I have just fixed this bug in commit 04b1783.
I am not implementing the optional space between the number and the sign, because the command line arguments are tokenized before being passed into R, and any whitespace is consider a delimiter. I am reluctant to implement an exception/work-around to this simple and understandable parsing mechanism.
CRAN is currently not allowing any package updates due to the holidays. Pending additional testing, I will submit the next release to CRAN by the end of January. In the meanwhile, please install commit 04b1783 using `devtools`. Please be sure to follow the instructions in the `README`, because you will need to build the documentation. I strongly prefer not to commit any generated files into the git repo.