Unexpected behaviour for optional arguments.

[Oliver Per Madsen]

Assume that a custom class has been created similar to this issue

setClass('myMethod')
setMethod('coerce', c(from = 'ANY', to = 'myMethod'), function(from, to){
  if(!from %in% c('hello', 'world'))
    stop('this should not happen')
  from
})
p <- arg_parser('test-parser')
p <- add_argument(p, arg = '--my-number', help = 'a number', type = 'numeric')
p <- add_argument(p, arg = c('--myMethod'), 
                  help = ('a test variable'),
                  type = 'myMethod')

Now as both arguments are optional, we’d assume that providing one or more of them, would do just fine. As such

parse_args(p)

works, but in the case where the first argument is provided, this fails

parse_args(p, c('--my-number', '4.2'))

The issue here, is that line line 521-523 within parse_args makes no assumption for partially missing arguments

    if (length(arg.idx) > 0) {
        # extract values following the optional argument label
        x[match(argv[arg.idx], arg.opt)] <- argv[arg.idx+1];
        # convert type of extraced values; x is now a list
        x <- mapply(convert_type,                               ##<===== Sinner 
            object=x, class=arg.opt.types, nargs=arg.opt.nargs, ##<===== Sinner
            SIMPLIFY=FALSE);                                    ##<===== Sinner
        # remove extracted arguments
        to.remove <- c(arg.idx, arg.idx+1);
        argv <- argv[-to.remove];
    }

Several fixes are possible here. The simplest is likely just to change mapply(convert_type, ... to

mapply(function(object, class, nargs){
  if(is.na(object))
    return(object)
  else 
    convert_type(object = object, class = class, nargs = nargs)
}, ...

or similarly updating the definition of convert_type.
A more complex (but faster) solution would be to continue the subsetting

    if (length(arg.idx) > 0) {
        # extract values following the optional argument label
        x[ind <- match(argv[arg.idx], arg.opt)] <- argv[arg.idx+1];
        # convert type of extraced values; x is now a list
        x[ind] <- mapply(convert_type,                                
                         object = x[ind], 
                         class = arg.opt.types[ind], 
                         nargs = arg.opt.nargs[ind], 
                         SIMPLIFY = FALSE);                                    
        # remove extracted arguments
        to.remove <- c(arg.idx, arg.idx+1);
        argv <- argv[-to.remove];
    }

‌

[David Shih]

Thanks for your detailed bug report. You might be venturing outside the intended use of this function, but I’ll look into it.

[Oliver Per Madsen]

Thank you David. This behaviour seems odd either way, and in the end, did not have anything to do with the user-defined type, but more that the function evaluates all optional arguments given any optional arguments. Although, I still think it is worth considering User-defined arguments. Often it is useful to be able to restrict numeric arguments to positive only values, certain intervals or in the case of files restrict to certain file extensions. Of course one could just check the arguments after they have been parsed, but then one looses the generality of the parser. Your package does seem to be the most well developed on cran from my perspective though.

[Sur]

I agree with Oliver. I have this same issue with just standard numeric type. You can reproduce with

library(argparser)

p <- arg_parser("Test")
p <- add_argument(p, "--arg1", help = "arg1", type = "integer")
p <- add_argument(p, "--arg2", help = "arg2", type = "integer")
# Get the same results if I put both arguments in one command
# p <- add_argument(p, c("--arg1", "--arg2"),
#                   help = c("arg1", "arg2"),
#                   type = c("integer", "integer"))

# Works
args <- parse_args(p, c("--arg1", "1", "--arg2", "2"))
print(args)

# Works
args <- parse_args(p)
print(args)

# Fails
args <- parse_args(p, c("--arg1", "1"))

The last line produces the error message

Error in (function (object, class, nargs)  :Invalid argument value: expecting integer but got: (NA).

I feel like it should be possible to include only a subset of of optional arguments and that should work.

It is a really useful packages anyway.

Here is my session info, but I get the same in other machines and with R 3.6.1 as well

> sessionInfo()
R version 4.0.3 (2020-10-10)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.1 LTS

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.9.0
LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.9.0

locale:
 [1] LC_CTYPE=es_US.UTF-8       LC_NUMERIC=C               LC_TIME=es_MX.UTF-8        LC_COLLATE=es_US.UTF-8     LC_MONETARY=es_MX.UTF-8   
 [6] LC_MESSAGES=es_US.UTF-8    LC_PAPER=es_MX.UTF-8       LC_NAME=C                  LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=es_MX.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] argparser_0.6

loaded via a namespace (and not attached):
 [1] colorspace_1.4-1 scales_1.1.1     compiler_4.0.3   plyr_1.8.6       R6_2.4.1         tools_4.0.3      Rcpp_1.0.5       AMOR_0.2-1      
 [9] lifecycle_0.2.0  munsell_0.5.0    rlang_0.4.8    

‌

[Oliver Per Madsen]

I am expecting that the problem currently is, that the package is only maintained to be functional rather than updated. I have considered extending the package and would love to hear from @David Shih whether this is the actual case?

‌

[David Shih]

The two issues described here were actually caused by a bug that mishandled NA in the convert_type function. This bug was introduced during a feature to allow checking of integer and numeric types.

This bug was already fixed in d75d49c6111fa13a15ecd5e6c470b9e4ae6acad6, but the CRAN version lagged behind.

Test cases are added in 31edb6dc10cc07405c2c7bdf85d15ffe75159da1

[David Shih]

d75d49c6111fa13a15ecd5e6c470b9e4ae6acad6