Negative numbers cannot be passed as arguments since 0.6

Issue #18 resolved
Panagiotis Cheilaris created an issue

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)

  1. David Shih repo owner

    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.

  2. Panagiotis Cheilaris reporter

    Great, thanks for the fix! You could also consider adding unit tests in a tests directory of your package to avoid regression.

  3. Paolo Ribeca

    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.

  4. Paolo Ribeca

    @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$"
    

  5. David Shih 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 what parse_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.

  6. Paolo Ribeca

    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…

  7. Log in to comment