number_format max_dig with integers

[dhan056]

Hello,

Thank you for making this wonderful package.

number_format(x, format = NULL, min_dig = NULL, max_dig = NULL, dec = ".")

I use this expression to check if the input format is +/-[0-9].[0-9]{0,8}. I find that when max_dig (e.g. max_dig = 8) is specified it expects that all inputs /rows to actually have decimals.

In the manual it says " max_dig maximum number of digits after separator"

So I was expecting that when it encounters integers this max_dig would not be processed. But in reality if a record is integer it would restrict the number of digits in the integer portion,

Am I using it correctly or perhaps this can be made clearer in the manual?

[markvanderloo]

Hi there, that smells blije a bug, or an inconsistency in the documentation. Thanks for reporting!

[alesasse]

Yes, I think this is definitely a bug. A couple of failing examples:

> number_format(122.0, max_dig=2) # should be TRUE
[1] FALSE
> number_format(122, max_dig=2) # should be TRUE
[1] FALSE

[alesasse]

I think I've found a possible fix.

number_format <- function(x, format=NULL, min_dig=NULL, max_dig=NULL, dec="."){
  if ( !is.null(format) ){
    rx <- utils::glob2rx(format, trim.tail=FALSE)
    rx <- gsub("d",  "\\d", rx, fixed=TRUE)
    rx <- gsub(".*", "\\d*",   rx, fixed=TRUE)
    return( grepl(rx, as.character(x)) )
  }
  rx <- if (dec == ".") "\\." else sprintf("\\%s", dec)
  decimal_digits <- unlist(strsplit(as.character(x), rx))[2]
  n_decimal <- if(is.na(decimal_digits)) 0 else nchar(decimal_digits)
  min_dig <- if (is.null(min_dig)) 0 else min_dig
  max_dig <- if (is.null(max_dig)) 8  else max_dig # possible problem here with the hardcoded 8
  if (n_decimal >= min_dig & n_decimal <= max_dig) {
    return(TRUE)
  }
  FALSE
}

Working on my fork I added the two tests above for the function and this rewriting seems to pass everything. I know I have changed quite a bit the logic of the function but honestly I don't feel very comfortable with regex ;) There may be a problem as the max_dig variable is hardcoded to 8, though. What do you think?

I can (try to) provide a clean pull request too if it's helpful.

Thanks for the great work!