fix: non-NULL default value with 'action == "append"'

* Fixes bug when using an argument with `action == "append"`
  and a non-`NULL` `default` value (#35).

closes #35

.Rbuildignore

@@ -1,4 +1,5 @@
 CONTRIBUTING
+Rakefile
 README.rst
 ^README.html$
 cran-comments.rst

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: argparse
 Type: Package
 Title: Command Line Optional and Positional Argument Parser
-Version: 2.1.1
+Version: 2.1.2
 Authors@R: c(person("Trevor L", "Davis", role=c("aut", "cre"), email="[email protected]"),
     person("Allen", "Day", role="ctb", comment="Some documentation and examples ported from the getopt package."),
     person("Python Software Foundation", role="ctb", comment="Some documentation from the optparse Python module."),

NEWS.md

@@ -1,4 +1,11 @@
-argparse 2.1.0
+argparse 2.1.2
+==============
+
+* Fixes bug when using an argument with `action == "append"` 
+  and a non-`NULL` `default` value (#35).
+  Thanks @miker985 for bug report.
+
+argparse 2.1.1
 ==============
 
 * Parsers now support ``parse_known_args()`` (#34).

R/argparse.R

@@ -217,12 +217,14 @@ parse_args_output <- function(output) {
 }
 
 # @param argument argument to be converted from R to Python
-convert_argument <- function(argument) {
+convert_argument <- function(argument, as_list = FALSE) {
     if (is.character(argument)) argument <- shQuote(argument, type = "sh")
     if (is.numeric(argument)) argument <- as.character(argument)
     if (is.logical(argument)) argument <- ifelse(argument, "True", "False")
     if (is.null(argument)) argument <- "None"
-    if (length(argument) > 1) {
+    if (as_list) {
+        argument <- sprintf("[%s]", paste(argument, collapse = ", "))
+    } else if (length(argument) > 1) {
         argument <- sprintf("(%s)", paste(argument, collapse = ", "))
     }
     argument
@@ -243,6 +245,17 @@ get_python_type <- function(type, proposed_arguments) {
     sprintf("type=%s", python_type)
 }
 
+should_as_list <- function(name, argument_list) {
+    if (name == "default" &&
+        (argument_list[["action"]] %||% "store") == "append") {
+        TRUE
+    } else {
+        FALSE
+    }
+}
+
+`%||%` <- function(x, y) if (is.null(x)) y else x # nolint
+
 # @param mode Either "add_argument" or "ArgumentParser"
 convert_..._to_arguments <- function(mode, ...) { # nolint
 
@@ -255,7 +268,8 @@ convert_..._to_arguments <- function(mode, ...) { # nolint
     for (ii in seq_along(argument_list)) {
         name <- argument_names[ii]
         equal <- equals[ii]
-        argument <- convert_argument(argument_list[[ii]])
+        as_list <- should_as_list(name, argument_list)
+        argument <- convert_argument(argument_list[[ii]], as_list)
         proposed_arguments <- append(proposed_arguments,
                                      paste0(name, equal, argument))
     }

tests/testthat/test-argparse.R

@@ -148,6 +148,8 @@ test_that("parse_known_args() works as expected", {
 
 })
 
+
+
 context("ArgumentParser")
 test_that("ArgumentParser works as expected", {
     skip_if_not(detects_python())
@@ -165,7 +167,7 @@ test_that("ArgumentParser works as expected", {
     expect_error(ArgumentParser(add_help = FALSE)$parse_args("-h"), "unrecognized arguments")
 })
 
-test_that("parse_args works as expected", {
+test_that("parse_args() works as expected", {
     skip_if_not(detects_python())
     parser <- ArgumentParser("foobar", usage = "%(prog)s arg1 arg2")
     parser$add_argument("--hello", dest = "saying", action = "store", default = "foo",
@@ -204,6 +206,16 @@ test_that("parse_args works as expected", {
     parser$add_argument("--lotsofstuff", type = "character", nargs = "+")
     args <- parser$parse_args(c("--lotsofstuff", rep("stuff", 1000)))
     expect_equal(args$lotsofstuff, rep("stuff", 1000))
+
+    # Bug found by @miker985
+    test_that("we can action = 'append' with a default list", {
+        parser <- argparse::ArgumentParser()
+        parser$add_argument("--test-dim", dest = "dims", action = "append",
+                            default = c("year", "sex", "age"))
+        args <- parser$parse_args(c("--test-dim", "race"))
+
+        expect_equal(args$dims, c("year", "sex", "age", "race"))
+    })
 })
 
 # Bug found by Erick Rocha Fonseca