From 68b5ccca3f176cddb11a0b7959899b1b6afc3604 Mon Sep 17 00:00:00 2001 From: "Trevor L. Davis" Date: Mon, 15 Apr 2024 10:00:30 -0700 Subject: [PATCH 1/2] fix: Don't coerce default if callback action * We no longer coerce the type of an option `default` to match that of its `type` argument when `action = "callback"`. Thanks husheng (@hs3434) for bug report. closes #46, closes #47 --- .github/workflows/R-CMD-check.yaml | 2 +- DESCRIPTION | 4 ++-- NEWS.md | 7 +++++++ R/optparse-package.R | 5 ++--- R/optparse.R | 9 +++++++-- inst/COPYRIGHTS | 4 ++-- man/optparse-package.Rd | 2 +- tests/testthat/test-optparse.R | 20 ++++++++++++-------- 8 files changed, 34 insertions(+), 19 deletions(-) diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index a94d12a..a661ee3 100644 --- a/.github/workflows/R-CMD-check.yaml +++ b/.github/workflows/R-CMD-check.yaml @@ -29,7 +29,7 @@ jobs: R_KEEP_PKG_SOURCE: yes steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: r-lib/actions/setup-pandoc@v2 diff --git a/DESCRIPTION b/DESCRIPTION index 180b829..b4dacfa 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -2,7 +2,7 @@ Encoding: UTF-8 Package: optparse Type: Package Title: Command Line Option Parser -Version: 1.7.4 +Version: 1.7.5-0 Authors@R: c(person("Trevor L", "Davis", role=c("aut", "cre"), email="trevor.l.davis@gmail.com", comment = c(ORCID = "0000-0001-6341-4639")), @@ -33,4 +33,4 @@ Suggests: testthat VignetteBuilder: knitr Roxygen: list(markdown = TRUE) -RoxygenNote: 7.2.3 +RoxygenNote: 7.3.1 diff --git a/NEWS.md b/NEWS.md index 1978d13..3875a0f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,10 @@ +optparse 1.7.5 +============== + +* We no longer coerce the type of an option `default` to match that of + its `type` argument when `action = "callback"`. + Thanks husheng (@hs3434) for bug report (#47). + optparse 1.7.4 ============== diff --git a/R/optparse-package.R b/R/optparse-package.R index 7520c5d..46134f1 100644 --- a/R/optparse-package.R +++ b/R/optparse-package.R @@ -24,8 +24,7 @@ #' #'@name optparse-package #'@aliases optparse-package optparse -#'@docType package -#'@author Trevor Davis. +#'@author Trevor L. Davis. #' #'Some documentation and unit tests ported from Allen Day's getopt package. #' @@ -44,4 +43,4 @@ #' readLines(example_file_2) #' } #' -NULL +"_PACKAGE" diff --git a/R/optparse.R b/R/optparse.R index c9df197..0fc562c 100644 --- a/R/optparse.R +++ b/R/optparse.R @@ -1,4 +1,4 @@ -# Copyright (c) 2010-2022 Trevor L. Davis +# Copyright (c) 2010-2024 Trevor L. Davis # Copyright (c) 2015 Rick FitzJohn https://github.com/richfitz # Copyright (c) 2013 Kirill Müller https://github.com/krlmlr # Copyright (c) 2011 Jim Nikelski @@ -262,12 +262,17 @@ make_option <- function(opt_str, action = NULL, type = NULL, dest = NULL, defaul type <- infer_type(action, default) } if (type == "numeric") type <- "double" + # default - if ((type != typeof(default)) && !is.null(default)) { + if ((action != "callback") && + (type != typeof(default)) && + !is.null(default)) { storage.mode(default) <- type } + # dest if (is.null(dest)) dest <- sub("^--", "", long_flag) + # metavar if (is.null(metavar)) { if (option_needs_argument_helper(action, type)) { diff --git a/inst/COPYRIGHTS b/inst/COPYRIGHTS index 3bbb065..5ce47f2 100644 --- a/inst/COPYRIGHTS +++ b/inst/COPYRIGHTS @@ -1,7 +1,7 @@ Format: http://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ Files: * -Copyright: 2010-2022 Trevor L Davis +Copyright: 2010-2024 Trevor L Davis License: GPL-2+ File: exec/example.R @@ -33,7 +33,7 @@ License: GPL-2+ File: R/optparse.R Copyright: 1990-2009 Python Software Foundation - 2011-2022 Trevor L Davis + 2011-2024 Trevor L Davis 2010 Steve Lianoglou 2011 Jim Nikelski 2013 Kirill Müller diff --git a/man/optparse-package.Rd b/man/optparse-package.Rd index e06b0d7..3cc6af0 100644 --- a/man/optparse-package.Rd +++ b/man/optparse-package.Rd @@ -47,7 +47,7 @@ is described here: \url{https://docs.python.org/3/library/optparse.html} \code{\link[getopt]{getopt}} } \author{ -Trevor Davis. +Trevor L. Davis. Some documentation and unit tests ported from Allen Day's getopt package. diff --git a/tests/testthat/test-optparse.R b/tests/testthat/test-optparse.R index e61690e..9fbdda8 100644 --- a/tests/testthat/test-optparse.R +++ b/tests/testthat/test-optparse.R @@ -33,8 +33,7 @@ option_list <- list( parser_ol <- OptionParser(option_list = option_list) -context("Testing make_option") -test_that("make_option works as expected", { +test_that("`make_option()` works as expected", { expect_equal(make_option("--integer", type = "integer", default = 5), make_option("--integer", default = as.integer(5))) expect_equal(make_option("--logical", type = "logical", default = "TRUE"), @@ -47,8 +46,7 @@ test_that("make_option works as expected", { get_long_flags <- function(parser) { sort(sapply(parser@options, function(x) x@long_flag)) } -context("Test add_option") -test_that("add_option works as expected", { +test_that("`add_option()` works as expected", { parser1 <- OptionParser(option_list = list(make_option("--generator"), make_option("--count"))) parser2 <- OptionParser() parser2 <- add_option(parser2, "--generator") @@ -56,8 +54,7 @@ test_that("add_option works as expected", { expect_equal(get_long_flags(parser1), get_long_flags(parser2)) }) -context("Testing parse_args") -test_that("parse_args works as expected", { +test_that("`parse_args()` works as expected", { # option_list took outside test_that option_list2 <- list( make_option(c("-n", "--add-numbers"), action = "store_true", default = FALSE, @@ -241,7 +238,6 @@ test_that("test bug with a NA short flag option with positional_arguments = TRUE sort_list(list(options = list(help = TRUE), args = "foo"))) }) -context("print_help") test_that("description and epilogue work as expected", { parser <- OptionParser() expect_output(print_help(parser), "Usage:") @@ -318,7 +314,6 @@ test_that("Can parse empty string", { # nolint end # Use h flag for non-help (Reported by Jeff Bruce) -context("Use h option for non-help") test_that("Use h option for non-help", { option_list_neg <- list(make_option(c("-h", "--mean"), default = 0.0)) parser <- OptionParser(usage = "\\%prog [options] file", option_list = option_list_neg) @@ -329,3 +324,12 @@ test_that("Use h option for non-help", { args <- parse_args(parser, args = c("-h", "-5.0")) expect_equal(args, list(mean = -5.0)) }) + +# Bug found by @husheng (#47) +test_that("Don't coerce `default` of callback action match that of `type`", { + parser <- OptionParser() + str2bool <- function(option, flag, option_value, parser) as.logical(option_value) + parser <- add_option(parser, "--bool", type = "character", default = FALSE, callback = str2bool) + expect_equal(parse_args(parser, c())$bool, FALSE) + expect_equal(parse_args(parser, "--bool=T")$bool, TRUE) +}) From c2a368c61ea5d3c93a2bb69b385b23d1058b5751 Mon Sep 17 00:00:00 2001 From: "Trevor L. Davis" Date: Mon, 15 Apr 2024 17:25:10 -0700 Subject: [PATCH 2/2] chore(cran_comments.md): Update test environments and revdepcheck results --- DESCRIPTION | 4 ++-- cran-comments.rst | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index b4dacfa..4b13177 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -2,8 +2,8 @@ Encoding: UTF-8 Package: optparse Type: Package Title: Command Line Option Parser -Version: 1.7.5-0 -Authors@R: c(person("Trevor L", "Davis", role=c("aut", "cre"), +Version: 1.7.5 +Authors@R: c(person("Trevor L.", "Davis", role=c("aut", "cre"), email="trevor.l.davis@gmail.com", comment = c(ORCID = "0000-0001-6341-4639")), person("Allen", "Day", role="ctb", comment="Some documentation and examples ported from the getopt package."), diff --git a/cran-comments.rst b/cran-comments.rst index 5ce6f7e..25a393b 100644 --- a/cran-comments.rst +++ b/cran-comments.rst @@ -1,6 +1,6 @@ **Test environments** -* local (linux), R 4.3.2 +* local (linux), R 4.3.3 * win-builder (windows), R devel * Github Actions (linux), R devel and R release * Github Actions (OSX), R release @@ -14,7 +14,7 @@ Status: OK ## revdepcheck results -We checked 13 reverse dependencies (9 from CRAN + 4 from Bioconductor), comparing R CMD check results across CRAN and dev versions of this package. +We checked 15 reverse dependencies (7 from CRAN + 8 from Bioconductor), comparing R CMD check results across CRAN and dev versions of this package. * We saw 0 new problems * We failed to check 0 packages