Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

number_format max_dig with integers #155

Open
dhan056 opened this issue Sep 17, 2021 · 3 comments
Open

number_format max_dig with integers #155

dhan056 opened this issue Sep 17, 2021 · 3 comments

Comments

@dhan056
Copy link

dhan056 commented Sep 17, 2021

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
Copy link
Member

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

@alesasse
Copy link

alesasse commented Nov 7, 2022

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
Copy link

alesasse commented Nov 9, 2022

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants