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

percent() with zero value yields NaN #50

Closed
vsalmendra opened this issue Sep 8, 2014 · 7 comments
Closed

percent() with zero value yields NaN #50

vsalmendra opened this issue Sep 8, 2014 · 7 comments

Comments

@vsalmendra
Copy link

@vsalmendra vsalmendra commented Sep 8, 2014

> library(scales)
> percent(0.1)
[1] "10%"
> percent(0)
[1] "NaN%"

> sessionInfo()
R version 3.1.0 (2014-04-10)
Platform: x86_64-pc-linux-gnu (64-bit)

locale:
[1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
[3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
[5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
[7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
[9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] scales_0.2.4

loaded via a namespace (and not attached):
[1] colorspace_1.2-4 munsell_0.4.2    plyr_1.8.1       Rcpp_0.11.2    
@BrianDiggs
Copy link
Contributor

@BrianDiggs BrianDiggs commented Sep 10, 2014

The problem ultimately arises because plyr:::round_any(0, 0) return NaN. (See https://github.com/hadley/plyr/blob/master/R/round-any.r#L28). I'm not sure what round_any should return when accuracy == 0; I'm thinking that it should just return x unaltered. Or a vector of 0 the same length as x.

# not real code because we are talking about a function in another package.
plyr:::round_any.numeric
function (x, accuracy, f = round) 
{
    if(accuracy == 0) {
        x
    } else {
        f(x/accuracy) * accuracy
    }
}
@dougmitarotonda
Copy link
Contributor

@dougmitarotonda dougmitarotonda commented Nov 25, 2014

I think that is a good solution. percent similarly does not work for negative numbers and I submitted a pull request for that here: #40. It would be great to see both these changes made so that percent works on all real numbers.

@hadley
Copy link
Member

@hadley hadley commented Jun 10, 2015

@BrianDiggs I suspect there's a better rounding algorithm that will also work with small x. Hopefully twitter can help: http://twitter.com/hadleywickham/status/608679186597695488

@mvpp
Copy link

@mvpp mvpp commented Nov 2, 2015

Any update on this?
The weird part is that it returns "0%" properly when passed in a vector with at least one no-zero value:

percent(c(0,2))
[1] "0%" "200%"
percent(0)
[1] "NaN%"
percent(c(0,0))
[1] "NaN%" "NaN%"

@Stefan2015-5
Copy link

@Stefan2015-5 Stefan2015-5 commented Nov 17, 2015

I must say it is embarrassing that this bug happened in the first place but it is more embarrassing that it is not fixed. I nearly had a report going out with "NaN%" in there. Sorry, if this comes across angry, but it makes me sad when you can't rely on these easy things.

my.percent<- function(x) {
if(length(x)==1) if(x==0) return(paste0(0,"%") )
return(percent(x) )
}

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Dec 15, 2015

IMO, the precision() function (which is used only by percent()) should be improved:

> precision(c(0.1, 100))
[1] 10
> precision(c(-0.1, 100))
[1] 100
> precision(0)
[1] 0

precision(0) should return 1 (this would fix the issue here), but for the other two I'm not sure.

@hadley hadley closed this in e659100 Feb 2, 2016
@Braonan
Copy link

@Braonan Braonan commented Dec 22, 2016

Could use mvpp's observation for a fix by creating a vector with dummy value and using only first numeric e.g.,
percent(c(0,1))[1]
bit of a dirty fix; but simple and works

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

Successfully merging a pull request may close this issue.

None yet
8 participants