- edited description
Argument type not respected for multiple arguments.
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")'
Comments (6)
-
reporter -
reporter - edited description
-
reporter - edited description
-
reporter 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() }
-
repo owner Thanks for your detailed report and investigation. I’ll get back to you.
-
repo owner - changed status to resolved
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
- Log in to comment