Argument type not respected for multiple arguments.

[Oliver Per Madsen]

While using the package I noticed that in certain scenarios when multiple arguments are provided simultaneously, the parser does not seem to keep argument types correctly.

Minimal reproducible example:

parser <- arg_parser('MWE')
parser <- add_argument(parser,
                       arg = c('--method', '--grid-length', '--problem-type'),
                       help = c('arg1.',
                                'arg2.',
                                'arg3.'),
                       default = list(NA, 400, 'search'),
                       type = c('character', 'integer', 'character'),
                       nargs = c(1, 1, 1),
                       flag = c(FALSE, FALSE, FALSE),
                       short = c('-m', '-l', '-t'))
#Warning messages:
#1: In type[!is.na(default)] <- unlist(lapply(default, class)) :
#  number of items to replace is not a multiple of replacement length
#2: In nargs[!is.na(default)] <- unlist(lapply(default, length)) :
#  number of items to replace is not a multiple of replacement length
parse_args(parser, c('-m', 'hello world', '-l', '400', '-t', 'binary'))
#Error in (function (object, class, nargs)  : 
#  Invalid argument value: expecting numeric but got: (binary).

sessionInfo()
R version 4.0.0 (2020-04-24)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04 LTS

Matrix products: default
BLAS/LAPACK: /usr/lib/x86_64-linux-gnu/openblas-pthread/libopenblasp-r0.3.8.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8     LC_MONETARY=en_US.UTF-8    LC_MESSAGES=C             
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                  LC_ADDRESS=C               LC_TELEPHONE=C             LC_MEASUREMENT=en_US.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] compiler_4.0.0 tools_4.0.0  

Executed from rocker/tidyverse:4.0.0, so if one is used to docker, a reproducible image can be build using docker build . with the following specification

FROM rocker:tidyverse:4.0.0
RUN R -e 'install.packages(pkgs = "checkpoint");library(checkpoint);checkpoint("2020-06-22");y;install.packagages("argparser")'

‌

[Oliver Per Madsen]

Looking more into the source code, I spotted the sinner at line 181 - 185 in add_argument, where the function tries to infer the type based on default values.

    # infer type based on the default values (original default variable), 
    # whenever available
    type[!is.na(default)] <- unlist(lapply(default, class));
    # infer number of arguments based on default values
    nargs[!is.na(default)] <- unlist(lapply(default, length));

This is will overwrite any user-given input, and likely lead to unintended behaviour. It would likely be safer to take a “the user is right” approach and use

# infer type based on the default values (original default variable), 
# whenever available
if(any(ind <- (!is.na(default) & is.na(type))))
  type[!is.na(ind)] <- unlist(lapply(default[ind], class));
# infer number of arguments based on default values
if(any(ind <- (!is.na(default) & is.na(nargs))))
  nargs[ind] <- unlist(lapply(default[ind], length)); 

The problem here I guess would be checking consistency of default arguments. Here it seems rather sensible to try to convert the default arguments during add_argument

# infer type based on the default values (original default variable), 
# whenever available
if(any(ind <- (!is.na(default) & is.na(type))))
  type[!is.na(ind)] <- unlist(lapply(default[ind], class));
# infer number of arguments based on default values
if(any(ind <- (!is.na(default) & is.na(nargs))))
  nargs[ind] <- unlist(lapply(default[ind], length)); 
check_default_consistency(arg, narg, type, default)

with a possible definition such as (Note the example within the documentation, which show both good and bad arguments.)

#' Check the consistency of nargs and default according to the type argument in add_argument
#' 
#' @param args argument names (use no prefix for positional arguments, -- or - prefix for optional arguments or flags)
#' @param nargs number of argument values (which can be inferred from default); set to Inf for an indefinite number; an optional argument with an indefinite number of values may need to be followed by another optional argument or flag (e.g. --) to separate the indefinite optional argument from possible position arguments
#' @param type    variable types of the arguments (which can be inferred 
#'                from \code{default}); assumed to be character otherwise. 
#'                See details for more information
#' @param default default values for the arguments 
#' 
#' @examples 
#' 
#' arg = c('--perfect-square')
#' default = list(400)
#' type = c('integer')
#' nargs = c(1)
#' check_default_consistency(arg, nargs, type, default)
#' 
#' # Try with custom class definition
#' setClass('perfectSquare')
#' setMethod('coerce', c(from = 'ANY', to = 'perfectSquare'), function(from, to){
#'   from <- as.numeric(from)
#'   if(!all.equal(from, as.integer(from)))
#'     stop(paste0(from, ' is not a perfect integer!'))
#'   sqt <- sqrt(from)
#'   if(sqt != as.integer(sqt))
#'     stop('from is not a perfect integer!')
#'   from
#' })
#' 
#' type <- 'perfectSquare'
#' check_default_consistency(arg, nargs, type, default)
#' 
#' \dontrun{
#' # Errors are thrown when nargs or default can't be converted to 'type' using as(default, type)
#' default <- 401
#' check_default_consistency(arg, nargs, type, default)
#' 
#' #Also throws error when nargs < length(default)
#' default <- c(400, 400)
#' check_default_consistency(arg, nargs, type, default)
#' }
check_default_consistency <- function(args, nargs, type, default){
  # Check that each element of default can be converted to "type" using as
  #  Note that this is more sure-fired than using the canCoerce function (which simply checks if the method exists)
  if(!is.list(default))
    default <- list(default)
  type_check <- mapply(function(type, default){
    sapply(default, function(x){
      x <- try(as(x, type), silent = TRUE)
      if(inherits(x, 'try-error'))
        FALSE
      else 
        TRUE
    })
  }, type = type, default = default)
  ind <- !sapply(type_check, all)
  if(any(ind)){
    whi <- mapply(function(x, y){
      paste(y[!x], collapse = ', ', sep = ', ')
    }, type_check[ind], default[ind])
    errMessage <- paste0('Unable to convert default arguments in \n', paste0(' ', args[ind], ' (', whi, ')' , collapse = '\n'))
    stop(errMessage)
  }
  # A safe assumption for nargs seems to be, that if default is supplied, 
  # it has to be at least equal to the length of defaults
  ll <- lengths(default)
  nargs_check <- !nargs >= ll
  if(any(nargs_check)){
    errMessage <- paste0('Inconsistent length of argument and default found for \n', paste0(' ', args[nargs_check], collapse = '\n'))
    stop(errMessage)
  }
  #return nothing, as error messaging is handled internally in the function
  invisible()
}

‌

[David Shih]

Thanks for your detailed report and investigation. I’ll get back to you.

[David Shih]

Thanks for the suggested fix, though the bug was actually much simpler, as hinted by the warning message.

I forgot to subset only elements of default that are not NA.

Fixed in 59035104