trevorld / r-optparse

command-line optional argument parser
http://trevorldavis.com/R/optparse/dev/
GNU General Public License v2.0
146 stars 11 forks source link

It affects the functionality of callback #46

Closed hs3434 closed 5 months ago

hs3434 commented 5 months ago

I think this is not necessary, the user should understand their default value type, rather than using the type to convert again, and more importantly, this affects the functionality of the callback, callback is best for raw character processing, but this will make the callback type not consistent with the default value. for example, I have a callback: str2bool <- function(option, flag, option_value, parser) { s <- tolower(as.character(option_value)) if (s %in% c("0", "false", "f", "no", "n")) { FALSE } else { TRUE } } It handles strings, and return logical, but if I set a logical default value, it will become character, not logical.

trevorld commented 5 months ago

We definitely don't want to change this in the general case of a non-callback function e.g. if we set a numeric default for a "store" action default we want values like "3.12" to be coerced to 3.12...

I'm not entirely persuaded we need to change this for callback functions. Can you provide a minimal reproducible example where this causes an issue?

It handles strings, and return logical, but if I set a logical default value, it will become character, not logical.

If you set a logical default value won't it coerce to logical, not character and if already logical not change it at all?

library("optparse")
parser <- OptionParser()
parser <- add_option(parser, "--bool", default=TRUE)
parse_args(parser, "--bool=F")
$help
[1] FALSE

$bool
[1] FALSE

Note also that normally when you'd want to store a boolean value you should probably be using the "store_true" and "store_false" actions...

hs3434 commented 5 months ago

Thank you for your reply!

If you set a logical default value won't it coerce to logical, not character and if already logical not change it at all?

yes,I think developers should understand the meaning of the default value, because this value is set in the code. My situation is as follows:

` $ radian R version 4.3.2 (2023-10-31) -- "Eye Holes" Platform: x86_64-conda-linux-gnu (64-bit)

r$> library("optparse")

r$> str2bool <- function(option, flag, option_value, parser) { s <- tolower(as.character(option_value)) if (s %in% c("0", "false", "f", "no", "n")) { FALSE } else { TRUE } }

r$> parser <- OptionParser(option_list = list(make_option("--test", type = "character", default = FALSE, callback = str2bool)))

r$> parse_args(parser, "--test=f") $test [1] FALSE

$help [1] FALSE

r$> parse_args(parser) $test [1] "FALSE"

$help [1] FALSE

r$> parser <- add_option(parser, "--bool", default = TRUE)

r$> parse_args(parser, "--bool=f") $test [1] "FALSE"

$help [1] FALSE

$bool [1] NA

Warning message: In getopt(spec = spec, opt = args) : long flag bool given a bad argument

r$> parser <- add_option(parser, "--bool2", default = TRUE, callback = str2bool) Error in storage.mode(default) <- type : cannot coerce type 'logical' to vector of type 'NULL'

r$> parser <- OptionParser()

r$> parser <- add_option(parser, "--bool2", type = "logical", default = TRUE)

r$> parse_args(parser, "--bool2=f") $help [1] FALSE

$bool2 [1] NA

Warning message: In getopt(spec = spec, opt = args) : long flag bool2 given a bad argument ` When parse_args(parser, "--test=f") is used, the parameter is processed by callback , but when parse_args(parser) is used, the parameter is not processed by callback. In addition, if I set the parameter to logical type, then some cases will not be handled correctly, such as parse_args(parser, "--bool2=f"), and for the users of the software, I don't think we should assume that they understand what logical types are, so I need to be as compatible as possible through callback.

trevorld commented 5 months ago
hs3434 commented 5 months ago

@trevorld it's good!