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

bug in adorn_percentages(denominator = "col") #490

Closed
mgacc0 opened this issue Oct 17, 2022 · 7 comments · Fixed by #491
Closed

bug in adorn_percentages(denominator = "col") #490

mgacc0 opened this issue Oct 17, 2022 · 7 comments · Fixed by #491
Assignees
Labels

Comments

@mgacc0
Copy link

mgacc0 commented Oct 17, 2022

      library(janitor)      
      mtcars %>%
        tabyl(am, cyl) %>%
        adorn_totals(c("row", "col")) %>%
        adorn_percentages(denominator = "col")
#> Error in FUN(X[[i]], ...): only defined on a data frame with all numeric-alike variables
@jzadra
Copy link
Contributor

jzadra commented Oct 17, 2022

This code works fine for me. Double check that you don't have any other packages loaded that have conflicting function names?

 library(janitor)      
#> 
#> Attaching package: 'janitor'
#> The following objects are masked from 'package:stats':
#> 
#>     chisq.test, fisher.test
      mtcars %>%
        tabyl(am, cyl) %>%
        adorn_totals(c("row", "col")) %>%
        adorn_percentages(denominator = "col")
#>     am         4         6         8   Total
#>      0 0.2727273 0.5714286 0.8571429 0.59375
#>      1 0.7272727 0.4285714 0.1428571 0.40625
#>  Total 1.0000000 1.0000000 1.0000000 1.00000

@mgacc0
Copy link
Author

mgacc0 commented Oct 17, 2022

I have the current development version of dplyr
dplyr 1.0.99.9000 2022-10-05 [1] Github (tidyverse/dplyr@1a7061f)

Could it be related?
Will this be a bug in the near future (when the dplyr development version arrives to CRAN)?

@jzadra
Copy link
Contributor

jzadra commented Oct 17, 2022

@sfirke I am able to reproduce this error with dplyr 1.0.99.9000

Either adorn on it's own works fine. And putting adorn_percentages() before adorn_totals() works (but of course yields slightly different results for the totals column.

@sfirke
Copy link
Owner

sfirke commented Oct 17, 2022 via email

@sfirke sfirke self-assigned this Oct 18, 2022
@sfirke sfirke added the bug label Oct 18, 2022
@sfirke
Copy link
Owner

sfirke commented Oct 21, 2022

Past Sam (blame him) had relied on dplyr::last() returning the last column when called on a data.frame: https://github.com/sfirke/janitor/blob/main/R/adorn_percentages.R#L94-L96

That behavior has changed according to dplyr's NEWS.md:

first(), last(), and nth() have been rewritten to use vctrs. This comes with the following improvements (#6331):

When used on a data frame, these functions now return a single row rather than a single column. This is more consistent with the vctrs principle that a data frame is generally treated as a vector of rows.

Seems reasonable. When I got to this line of the current code, I was kind of surprised it even works as-is. I'll refactor to base R, which it should have been in the first place.

sfirke added a commit that referenced this issue Oct 21, 2022
Fix adorn_percentages() and close #490
@sfirke
Copy link
Owner

sfirke commented Oct 21, 2022

@mgacc0 feel free to install the latest commit here with remotes::install_github("sfirke/janitor") and see if your code works again

@mgacc0
Copy link
Author

mgacc0 commented Oct 21, 2022

@mgacc0 feel free to install the latest commit here with remotes::install_github("sfirke/janitor") and see if your code works again

Now it works as good as before. 👏

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

Successfully merging a pull request may close this issue.

3 participants