Feature request: test type of arguments

Issue #16 resolved
António Miguel de Jesus Domingues created an issue

First of all thank you so much for this very useful package @djhshih. I have been using for several years now on pretty much all my scripts.

When type is set in the args, argparser will try to force to character or alternatively make it an NA. Would it be possible to change this behaviour and test explicitly for type That is this code:

#!/usr/bin/env Rscript

# ==========================================================================
# Arguments
# ==========================================================================
library("argparser")
# Create a parser
p <- arg_parser("Can argparser and rend.R play nicely with each other?")

# Add command line arguments
p <- add_argument(
p,
"--arg1",
help = "Character argument",
type = "character"
)

p <- add_argument(
p,
"--arg2",
help = "Integer argument.",
type = "numeric"
)

# Parse the command line arguments
argv <- parse_args(p, argv = commandArgs())
arg1 <- argv$arg1

arg2 <- argv$arg2

Would return an error when called with Rscript test.R --arg1 1 --arg2 error.

Along those lines, it would be useful than when an argument is missing or the wrong type, the printed error message would mention the cause of the issue. For instance "arg1 is missing" or "arg1 is not type numeric".

Comments (6)

  1. David Shih repo owner

    So you are requesting two new features:

    1. Return error when optional/named argument is missing.
    2. Return error when argument does not match defined type.

    For 1, you really should be using a positional/required argument. POSIX conventions recommend that named arguments (those that begin with -) are optional, though I have not implemented the full POSIX specification. I know there are many argument parsers that throw an error for missing named argument. I’ll think more about this.

    For 2, this sounds a reasonable request albeit it would be backwards incompatible, so you’ll get the blame for this. I’ll have to think more about how best to implement this while minimizing the chance of breaking existing code.

  2. António Miguel de Jesus Domingues reporter

    Indeed.

    For 1, you really should be using a positional/required argument. POSIX conventions recommend that named arguments (those that begin with -) are optional, though I have not implemented the full POSIX specification. I know there are many argument parsers that throw an error for missing named argument. I’ll think more about this.

    You are absolutely right that missing optional/named arguments should not create an error, so nothing to see here. Regarding the positional/required arguments argparser does indeed prints and error message, but imho it could be improved. Currently it is something like:

    domingue@ThinkPad-X280:sandbox$ ./round.R -d 1
    usage: round.R [--] [--help] [--opts OPTS] [--digits DIGITS] number
    
    Round a floating point number
    
    positional arguments:
      number        number to round
    
    flags:
      -h, --help    show this help message and exit
    
    optional arguments:
      -x, --opts    RDS file containing argument values
      -d, --digits  number of decimal places [default: 0]
    
    Error in parse_args(p) : 
      Missing required arguments: expecting 1 values but got ().
    Execution halted
    

    and would suggest that:

    1. the line Missing required arguments: expecting 1 values but got (). moves to top, to make is clear to the user what is happening instead of having to parse all the usage information (in some of my scripts as many as 8 arguments).
    2. the error should be phrased along the lines of Missing required arguments: expecting 1 values but got 0. See usage.

    This is all cosmetic and probably a matter of opinion.

    For 2, this sounds a reasonable request albeit it would be backwards incompatible, so you’ll get the blame for this.

    I don't mind getting blamed for pushing for something useful :) Jokes aside, I can totally understand your point of view and it could be useful if other user chip in.

    I’ll have to think more about how best to implement this while minimizing the chance of breaking existing code.

    Maybe instead of

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

    Default to inspecting the type of the argument instead of the value? type should be valid class anyway. Just a thought based on my shallow reading of the code.

    Anyway, thank you for looking into this.

  3. David Shih repo owner

    Numeric and integer type checking is implemented, and error messages are slightly improved in commit 2c0ec59.

    More informative error message proved difficult, because it is hard to guess what the user intended, and much more code will be required to handle the assignment of values to optional arguments.

    The way the code currently works is kept simple: passed values are sequentially assigned to optional arguments as needed, and any remaining values are left for positional arguments. If there is a mismatch between the number of remaining values and the number of positional arguments, an error is thrown.

  4. Log in to comment